Skip to content

Commit

Permalink
merge internal development externally
Browse files Browse the repository at this point in the history
  • Loading branch information
searlmc1 committed May 15, 2024
2 parents 0590572 + 7527926 commit 9878c40
Show file tree
Hide file tree
Showing 123 changed files with 2,937 additions and 1,024 deletions.
24 changes: 3 additions & 21 deletions amd/comgr/cmake/opencl_pch.cmake
Original file line number Diff line number Diff line change
@@ -1,26 +1,8 @@
if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
find_package(Clang REQUIRED CONFIG)

# FIXME: CLANG_CMAKE_DIR seems like the most stable way to find this, but
# really there is no way to reliably discover this header.
#
# We effectively back up to the Clang output directory (for the case of a build
# tree) or install prefix (for the case of an installed copy), and then search
# for a file named opencl-c.h anywhere below that. We take the first result in
# the case where there are multiple (e.g. if there is an installed copy nested
# in a build directory). This is a bit imprecise, but it covers cases like MSVC
# adding some additional configuration-specific subdirectories to the build
# tree but not to an installed copy.
file(GLOB_RECURSE OPENCL_C_H_LIST "${CLANG_CMAKE_DIR}/../../../*/opencl-c.h")

list(GET OPENCL_C_H_LIST 0 OPENCL_C_H)

if (NOT EXISTS "${OPENCL_C_H}" OR IS_DIRECTORY "${OPENCL_C_H}")
message(FATAL_ERROR "Unable to locate opencl-c.h from the supplied Clang. The path '${CLANG_CMAKE_DIR}/../../../*' was searched.")
endif()
else()
get_target_property(clang_build_header_dir clang-resource-headers RUNTIME_OUTPUT_DIRECTORY)
set(OPENCL_C_H "${clang_build_header_dir}/opencl-c.h")
execute_process(COMMAND "${CLANG_CMAKE_DIR}/../../../bin/clang" -print-resource-dir OUTPUT_VARIABLE CLANG_RESOURCE_DIR)
string(STRIP ${CLANG_RESOURCE_DIR} CLANG_RESOURCE_DIR)
set(OPENCL_C_H "${CLANG_RESOURCE_DIR}/include/opencl-c.h")
endif()

# Macro to create and install a custom target for generating PCH for given
Expand Down
18 changes: 16 additions & 2 deletions amd/comgr/src/comgr-compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,8 @@ AMDGPUCompiler::processFiles(amd_comgr_data_kind_t OutputKind,
if (auto Status = amd_comgr_create_data(OutputKind, &OutputT)) {
return Status;
}

// OutputT can be released after addition to the data_set
ScopedDataObjectReleaser SDOR(OutputT);

DataObject *Output = DataObject::convert(OutputT);
Expand Down Expand Up @@ -1365,6 +1367,9 @@ amd_comgr_status_t AMDGPUCompiler::unbundle() {
&ResultT))
return Status;

// ResultT can be released after addition to the data_set
ScopedDataObjectReleaser SDOR(ResultT);

DataObject *Result = DataObject::convert(ResultT);
if (auto Status = inputFromFile(Result, StringRef(output_file_path)))
return Status;
Expand Down Expand Up @@ -1520,6 +1525,9 @@ amd_comgr_status_t AMDGPUCompiler::linkBitcodeToBitcode() {
if (auto Status = amd_comgr_create_data(AMD_COMGR_DATA_KIND_BC, &ResultT))
return Status;

// ResultT can be released after addition to the data_set
ScopedDataObjectReleaser SDOR(ResultT);

DataObject *Result = DataObject::convert(ResultT);
if (auto Status = inputFromFile(Result, StringRef(output_file_path)))
return Status;
Expand All @@ -1539,8 +1547,6 @@ amd_comgr_status_t AMDGPUCompiler::linkBitcodeToBitcode() {
return AMD_COMGR_STATUS_ERROR;
if (L.linkInModule(std::move(Mod), ApplicableFlags))
return AMD_COMGR_STATUS_ERROR;

Result->release();
}
// Unbundle bitcode archive
else if (Input->DataKind == AMD_COMGR_DATA_KIND_AR_BUNDLE) {
Expand Down Expand Up @@ -1620,6 +1626,9 @@ amd_comgr_status_t AMDGPUCompiler::linkBitcodeToBitcode() {
if (auto Status = amd_comgr_create_data(AMD_COMGR_DATA_KIND_AR, &ResultT))
return Status;

// ResultT can be released after addition to the data_set
ScopedDataObjectReleaser SDOR(ResultT);

DataObject *Result = DataObject::convert(ResultT);
if (auto Status = inputFromFile(Result, StringRef(output_file_path)))
return Status;
Expand Down Expand Up @@ -1699,6 +1708,8 @@ amd_comgr_status_t AMDGPUCompiler::linkBitcodeToBitcode() {
if (auto Status = amd_comgr_create_data(AMD_COMGR_DATA_KIND_BC, &OutputT)) {
return Status;
}

// OutputT can be released after addition to the data_set
ScopedDataObjectReleaser SDOR(OutputT);

DataObject *Output = DataObject::convert(OutputT);
Expand Down Expand Up @@ -1795,6 +1806,8 @@ amd_comgr_status_t AMDGPUCompiler::linkToRelocatable() {
amd_comgr_create_data(AMD_COMGR_DATA_KIND_RELOCATABLE, &OutputT)) {
return Status;
}

// OutputT can be released after addition to the data_set
ScopedDataObjectReleaser SDOR(OutputT);

DataObject *Output = DataObject::convert(OutputT);
Expand Down Expand Up @@ -1849,6 +1862,7 @@ amd_comgr_status_t AMDGPUCompiler::linkToExecutable() {
amd_comgr_create_data(AMD_COMGR_DATA_KIND_EXECUTABLE, &OutputT)) {
return Status;
}
// OutputT can be released after addition to the data_set
ScopedDataObjectReleaser SDOR(OutputT);

DataObject *Output = DataObject::convert(OutputT);
Expand Down
38 changes: 38 additions & 0 deletions clang/docs/ClangLinkerWrapper.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ only for the linker wrapper will be forwarded to the wrapped linker job.
-l <libname> Search for library <libname>
--opt-level=<O0, O1, O2, or O3>
Optimization level for LTO
--override-image=<kind=file>
Uses the provided file as if it were the output of the device link step
-o <path> Path to file to write output
--pass-remarks-analysis=<value>
Pass remarks for LTO
Expand Down Expand Up @@ -87,6 +89,42 @@ other. Generally, this requires that the target triple and architecture match.
An exception is made when the architecture is listed as ``generic``, which will
cause it be linked with any other device code with the same target triple.

Debugging
=========

The linker wrapper performs a lot of steps internally, such as input matching,
symbol resolution, and image registration. This makes it difficult to debug in
some scenarios. The behavior of the linker-wrapper is controlled mostly through
metadata, described in `clang documentation
<https://clang.llvm.org/docs/OffloadingDesign.html>`_. Intermediate output can
be obtained from the linker-wrapper using the ``--save-temps`` flag. These files
can then be modified.

.. code-block:: sh
$> clang openmp.c -fopenmp --offload-arch=gfx90a -c
$> clang openmp.o -fopenmp --offload-arch=gfx90a -Wl,--save-temps
$> ; Modify temp files.
$> llvm-objcopy --update-section=.llvm.offloading=out.bc openmp.o
Doing this will allow you to override one of the input files by replacing its
embedded offloading metadata with a user-modified version. However, this will be
more difficult when there are multiple input files. For a very large hammer, the
``--override-image=<kind>=<file>`` flag can be used.

In the following example, we use the ``--save-temps`` to obtain the LLVM-IR just
before running the backend. We then modify it to test altered behavior, and then
compile it to a binary. This can then be passed to the linker-wrapper which will
then ignore all embedded metadata and use the provided image as if it were the
result of the device linking phase.

.. code-block:: sh
$> clang openmp.c -fopenmp --offload-arch=gfx90a -Wl,--save-temps
$> ; Modify temp files.
$> clang --target=amdgcn-amd-amdhsa -mcpu=gfx90a -nogpulib out.bc -o a.out
$> clang openmp.c -fopenmp --offload-arch=gfx90a -Wl,--override-image=openmp=a.out
Example
=======

Expand Down
10 changes: 10 additions & 0 deletions clang/lib/AST/Interp/ByteCodeExprGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2488,6 +2488,16 @@ bool ByteCodeExprGen<Emitter>::VisitRecoveryExpr(const RecoveryExpr *E) {
return this->emitError(E);
}

template <class Emitter>
bool ByteCodeExprGen<Emitter>::VisitAddrLabelExpr(const AddrLabelExpr *E) {
assert(E->getType()->isVoidPointerType());

unsigned Offset = allocateLocalPrimitive(
E->getLabel(), PT_Ptr, /*IsConst=*/true, /*IsExtended=*/false);

return this->emitGetLocal(PT_Ptr, Offset, E);
}

template <class Emitter> bool ByteCodeExprGen<Emitter>::discard(const Expr *E) {
OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/true,
/*NewInitializing=*/false);
Expand Down
1 change: 1 addition & 0 deletions clang/lib/AST/Interp/ByteCodeExprGen.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
bool VisitPseudoObjectExpr(const PseudoObjectExpr *E);
bool VisitPackIndexingExpr(const PackIndexingExpr *E);
bool VisitRecoveryExpr(const RecoveryExpr *E);
bool VisitAddrLabelExpr(const AddrLabelExpr *E);

protected:
bool visitExpr(const Expr *E) override;
Expand Down
14 changes: 7 additions & 7 deletions clang/lib/AST/Interp/Interp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,9 @@ bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {

QualType T = VD->getType();
if (S.getLangOpts().CPlusPlus && !S.getLangOpts().CPlusPlus11)
return T->isSignedIntegerOrEnumerationType() || T->isUnsignedIntegerOrEnumerationType();
return (T->isSignedIntegerOrEnumerationType() ||
T->isUnsignedIntegerOrEnumerationType()) &&
T.isConstQualified();

if (T.isConstQualified())
return true;
Expand All @@ -316,12 +318,10 @@ bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
return false;
};

if (const auto *D = Desc->asValueDecl()) {
if (const auto *VD = dyn_cast<VarDecl>(D);
VD && VD->hasGlobalStorage() && !IsConstType(VD)) {
diagnoseNonConstVariable(S, OpPC, VD);
return S.inConstantContext();
}
if (const auto *D = Desc->asVarDecl();
D && D->hasGlobalStorage() && !IsConstType(D)) {
diagnoseNonConstVariable(S, OpPC, D);
return S.inConstantContext();
}

return true;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/AST/Interp/Pointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ void Pointer::print(llvm::raw_ostream &OS) const {
else
OS << Offset << ", ";

if (isBlockPointer() && PointeeStorage.BS.Pointee)
if (PointeeStorage.BS.Pointee)
OS << PointeeStorage.BS.Pointee->getSize();
else
OS << "nullptr";
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/AST/Interp/Program.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ std::optional<unsigned> Program::getOrCreateDummy(const ValueDecl *VD) {
if (std::optional<PrimType> T = Ctx.classify(QT))
Desc = createDescriptor(VD, *T, std::nullopt, true, false);
else
Desc = createDescriptor(VD, VD->getType().getTypePtr(), std::nullopt, true,
false);
Desc = createDescriptor(VD, QT.getTypePtr(), std::nullopt, true, false);
if (!Desc)
Desc = allocateDescriptor(VD);

Expand Down
2 changes: 1 addition & 1 deletion clang/lib/CodeGen/Targets/AArch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ AArch64ABIInfo::classifyArgumentType(QualType Ty, bool IsVariadic,

if (const auto *EIT = Ty->getAs<BitIntType>())
if (EIT->getNumBits() > 128)
return getNaturalAlignIndirect(Ty);
return getNaturalAlignIndirect(Ty, false);

return (isPromotableIntegerTypeForABI(Ty) && isDarwinPCS()
? ABIArgInfo::getExtend(Ty)
Expand Down
35 changes: 18 additions & 17 deletions clang/lib/Format/TokenAnnotator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5405,6 +5405,9 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
return true;
}

const auto *BeforeLeft = Left.Previous;
const auto *AfterRight = Right.Next;

if (Style.isCSharp()) {
if (Left.is(TT_FatArrow) && Right.is(tok::l_brace) &&
Style.BraceWrapping.AfterFunction) {
Expand All @@ -5416,7 +5419,7 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
}
if (Right.is(TT_CSharpGenericTypeConstraint))
return true;
if (Right.Next && Right.Next->is(TT_FatArrow) &&
if (AfterRight && AfterRight->is(TT_FatArrow) &&
(Right.is(tok::numeric_constant) ||
(Right.is(tok::identifier) && Right.TokenText == "_"))) {
return true;
Expand All @@ -5433,15 +5436,14 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
Left.is(tok::r_square) && Right.is(tok::l_square)) {
return true;
}

} else if (Style.isJavaScript()) {
// FIXME: This might apply to other languages and token kinds.
if (Right.is(tok::string_literal) && Left.is(tok::plus) && Left.Previous &&
Left.Previous->is(tok::string_literal)) {
if (Right.is(tok::string_literal) && Left.is(tok::plus) && BeforeLeft &&
BeforeLeft->is(tok::string_literal)) {
return true;
}
if (Left.is(TT_DictLiteral) && Left.is(tok::l_brace) && Line.Level == 0 &&
Left.Previous && Left.Previous->is(tok::equal) &&
BeforeLeft && BeforeLeft->is(tok::equal) &&
Line.First->isOneOf(tok::identifier, Keywords.kw_import, tok::kw_export,
tok::kw_const) &&
// kw_var/kw_let are pseudo-tokens that are tok::identifier, so match
Expand All @@ -5460,8 +5462,8 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
// instead of bin-packing.
return true;
}
if (Right.is(tok::r_brace) && Left.is(tok::l_brace) && Left.Previous &&
Left.Previous->is(TT_FatArrow)) {
if (Right.is(tok::r_brace) && Left.is(tok::l_brace) && BeforeLeft &&
BeforeLeft->is(TT_FatArrow)) {
// JS arrow function (=> {...}).
switch (Style.AllowShortLambdasOnASingleLine) {
case FormatStyle::SLS_All:
Expand Down Expand Up @@ -5489,8 +5491,8 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
FormatStyle::SFS_InlineOnly);
}
} else if (Style.Language == FormatStyle::LK_Java) {
if (Right.is(tok::plus) && Left.is(tok::string_literal) && Right.Next &&
Right.Next->is(tok::string_literal)) {
if (Right.is(tok::plus) && Left.is(tok::string_literal) && AfterRight &&
AfterRight->is(tok::string_literal)) {
return true;
}
} else if (Style.isVerilog()) {
Expand Down Expand Up @@ -5543,8 +5545,7 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
}
return Style.BreakArrays;
}
}
if (Style.isTableGen()) {
} else if (Style.isTableGen()) {
// Break the comma in side cond operators.
// !cond(case1:1,
// case2:0);
Expand Down Expand Up @@ -5600,8 +5601,8 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
return true;
if (Left.IsUnterminatedLiteral)
return true;
if (Right.is(tok::lessless) && Right.Next && Left.is(tok::string_literal) &&
Right.Next->is(tok::string_literal)) {
if (Right.is(tok::lessless) && AfterRight && Left.is(tok::string_literal) &&
AfterRight->is(tok::string_literal)) {
return true;
}
if (Right.is(TT_RequiresClause)) {
Expand Down Expand Up @@ -5678,8 +5679,8 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
// string literal accordingly. Thus, we try keep existing line breaks.
return Right.IsMultiline && Right.NewlinesBefore > 0;
}
if ((Left.is(tok::l_brace) || (Left.is(tok::less) && Left.Previous &&
Left.Previous->is(tok::equal))) &&
if ((Left.is(tok::l_brace) ||
(Left.is(tok::less) && BeforeLeft && BeforeLeft->is(tok::equal))) &&
Right.NestingLevel == 1 && Style.Language == FormatStyle::LK_Proto) {
// Don't put enums or option definitions onto single lines in protocol
// buffers.
Expand Down Expand Up @@ -5793,7 +5794,7 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
//
// We ensure elsewhere that extensions are always on their own line.
if (Style.isProto() && Right.is(TT_SelectorName) &&
Right.isNot(tok::r_square) && Right.Next) {
Right.isNot(tok::r_square) && AfterRight) {
// Keep `@submessage` together in:
// @submessage { key: value }
if (Left.is(tok::at))
Expand All @@ -5802,7 +5803,7 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
// selector { ...
// selector: { ...
// selector: @base { ...
FormatToken *LBrace = Right.Next;
const auto *LBrace = AfterRight;
if (LBrace && LBrace->is(tok::colon)) {
LBrace = LBrace->Next;
if (LBrace && LBrace->is(tok::at)) {
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ void ErrnoChecker::checkPreCall(const CallEvent &Call,
// Probably 'strerror'?
if (CallF->isExternC() && CallF->isGlobal() &&
C.getSourceManager().isInSystemHeader(CallF->getLocation()) &&
!isErrno(CallF)) {
!isErrnoLocationCall(Call)) {
if (getErrnoState(C.getState()) == MustBeChecked) {
std::optional<ento::Loc> ErrnoLoc = getErrnoLoc(C.getState());
assert(ErrnoLoc && "ErrnoLoc should exist if an errno state is set.");
Expand Down
Loading

0 comments on commit 9878c40

Please sign in to comment.