From 98380762c3b734c23d206182605ab9e035c93caa Mon Sep 17 00:00:00 2001 From: Saiyedul Islam Date: Mon, 26 Jul 2021 22:09:41 +0530 Subject: [PATCH] [clang-offload-bundler] Make Bundle Entry ID backward compatible Earlier BundleEntryID used to be --. This used to work because the clang-offload-bundler didn't need GPUArch explicitly for any bundling/unbundling action. With unbundleArchive it needs GPUArch to ensure compatibility between device specific code objects. D93525 enforced triples to have separators for all 4 components irrespective of number of components, like "amdgcn-amd-amdhsa--". It was required to to correctly parse a possible 4th environment component or a GPU. But, this condition is breaking backward compatibility with archive libraries compiled with compilers older than D93525. This patch allows triples to have any number of components with and without extra separator for empty environment field. Thus, both the following bundle entry IDs are same: openmp-amdgcn-amd-amdhsa--gfx906 openmp-amdgcn-amd-amdhsa-gfx906 Reviewed By: yaxunl, grokos Differential Revision: https://reviews.llvm.org/D106809 --- clang/docs/ClangOffloadBundler.rst | 10 +---- clang/lib/Driver/ToolChains/Clang.cpp | 34 ++++++-------- clang/test/Driver/clang-offload-bundler.c | 20 +++++++-- clang/test/Driver/hip-rdc-device-only.hip | 8 ++-- .../Driver/hip-toolchain-rdc-separate.hip | 12 ++--- .../ClangOffloadBundler.cpp | 45 +++++++++++++------ 6 files changed, 73 insertions(+), 56 deletions(-) diff --git a/clang/docs/ClangOffloadBundler.rst b/clang/docs/ClangOffloadBundler.rst index c92d8a94cfb54..a0e446f766eea 100644 --- a/clang/docs/ClangOffloadBundler.rst +++ b/clang/docs/ClangOffloadBundler.rst @@ -121,15 +121,7 @@ Where: ============= ============================================================== **target-triple** - The target triple of the code object: - -.. code:: - - --- - -It is required to have all four components present, if target-id is present. -Components are hyphen separated. If a component is not specified then the -empty string must be used in its place. + The target triple of the code object. **target-id** The canonical target ID of the code object. Present only if the target diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 65e377f6a7f7a..5d817aa480bf4 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -7686,16 +7686,12 @@ void OffloadBundler::ConstructJob(Compilation &C, const JobAction &JA, }); } Triples += Action::GetOffloadKindName(CurKind); - Triples += "-"; - std::string NormalizedTriple = CurTC->getTriple().normalize(); - Triples += NormalizedTriple; - - if (CurDep->getOffloadingArch() != nullptr) { - // If OffloadArch is present it can only appear as the 6th hypen - // sepearated field of Bundle Entry ID. So, pad required number of - // hyphens in Triple. - for (int i = 4 - StringRef(NormalizedTriple).count("-"); i > 0; i--) - Triples += "-"; + Triples += '-'; + Triples += CurTC->getTriple().normalize(); + if ((CurKind == Action::OFK_HIP || CurKind == Action::OFK_OpenMP || + CurKind == Action::OFK_Cuda) && + CurDep->getOffloadingArch()) { + Triples += '-'; Triples += CurDep->getOffloadingArch(); } } @@ -7768,17 +7764,13 @@ void OffloadBundler::ConstructJobMultipleOutputs( auto &Dep = DepInfo[I]; Triples += Action::GetOffloadKindName(Dep.DependentOffloadKind); - Triples += "-"; - std::string NormalizedTriple = - Dep.DependentToolChain->getTriple().normalize(); - Triples += NormalizedTriple; - - if (!Dep.DependentBoundArch.empty()) { - // If OffloadArch is present it can only appear as the 6th hypen - // sepearated field of Bundle Entry ID. So, pad required number of - // hyphens in Triple. - for (int i = 4 - StringRef(NormalizedTriple).count("-"); i > 0; i--) - Triples += "-"; + Triples += '-'; + Triples += Dep.DependentToolChain->getTriple().normalize(); + if ((Dep.DependentOffloadKind == Action::OFK_HIP || + Dep.DependentOffloadKind == Action::OFK_OpenMP || + Dep.DependentOffloadKind == Action::OFK_Cuda) && + !Dep.DependentBoundArch.empty()) { + Triples += '-'; Triples += Dep.DependentBoundArch; } } diff --git a/clang/test/Driver/clang-offload-bundler.c b/clang/test/Driver/clang-offload-bundler.c index e1afa19570ec3..d201dd4103892 100644 --- a/clang/test/Driver/clang-offload-bundler.c +++ b/clang/test/Driver/clang-offload-bundler.c @@ -382,16 +382,30 @@ // Check archive unbundling // // Create few code object bundles and archive them to create an input archive -// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-amdgcn-amd-amdhsa--gfx906,openmp-amdgcn-amd-amdhsa--gfx908 -inputs=%t.o,%t.tgt1,%t.tgt2 -outputs=%t.simple.bundle +// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-amdgcn-amd-amdhsa-gfx906,openmp-amdgcn-amd-amdhsa--gfx908 -inputs=%t.o,%t.tgt1,%t.tgt2 -outputs=%t.simple.bundle // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-amdgcn-amd-amdhsa--gfx903 -inputs=%t.o,%t.tgt1 -outputs=%t.simple1.bundle // RUN: llvm-ar cr %t.input-archive.a %t.simple.bundle %t.simple1.bundle -// RUN: clang-offload-bundler -unbundle -type=a -targets=openmp-amdgcn-amd-amdhsa--gfx906,openmp-amdgcn-amd-amdhsa--gfx908 -inputs=%t.input-archive.a -outputs=%t-archive-gfx906-simple.a,%t-archive-gfx908-simple.a +// RUN: clang-offload-bundler -unbundle -type=a -targets=openmp-amdgcn-amd-amdhsa-gfx906,openmp-amdgcn-amd-amdhsa-gfx908 -inputs=%t.input-archive.a -outputs=%t-archive-gfx906-simple.a,%t-archive-gfx908-simple.a // RUN: llvm-ar t %t-archive-gfx906-simple.a | FileCheck %s -check-prefix=GFX906 -// GFX906: simple-openmp-amdgcn-amd-amdhsa--gfx906 +// GFX906: simple-openmp-amdgcn-amd-amdhsa-gfx906 // RUN: llvm-ar t %t-archive-gfx908-simple.a | FileCheck %s -check-prefix=GFX908 // GFX908-NOT: {{gfx906}} +// Check for error if no compatible code object is found in the heterogeneous archive library +// RUN: not clang-offload-bundler -unbundle -type=a -targets=openmp-amdgcn-amd-amdhsa-gfx803 -inputs=%t.input-archive.a -outputs=%t-archive-gfx803-incompatible.a 2>&1 | FileCheck %s -check-prefix=INCOMPATIBLEARCHIVE +// INCOMPATIBLEARCHIVE: error: no compatible code object found for the target 'openmp-amdgcn-amd-amdhsa-gfx803' in heterogeneous archive library + +// Check creation of empty archive if allow-missing-bundles is present and no compatible code object is found in the heterogeneous archive library +// RUN: clang-offload-bundler -unbundle -type=a -targets=openmp-amdgcn-amd-amdhsa-gfx803 -inputs=%t.input-archive.a -outputs=%t-archive-gfx803-empty.a -allow-missing-bundles +// RUN: cat %t-archive-gfx803-empty.a | FileCheck %s -check-prefix=EMPTYARCHIVE +// EMPTYARCHIVE: ! + +// Tests to check compatibility between Bundle Entry ID formats i.e. between presence/absence of extra hyphen in case of missing environment field +// RUN: clang-offload-bundler -unbundle -type=a -targets=openmp-amdgcn-amd-amdhsa--gfx906,openmp-amdgcn-amd-amdhsa-gfx908 -inputs=%t.input-archive.a -outputs=%t-archive-gfx906-simple.a,%t-archive-gfx908-simple.a -debug-only=CodeObjectCompatibility 2>&1 | FileCheck %s -check-prefix=BUNDLECOMPATIBILITY +// BUNDLECOMPATIBILITY: Compatible: Exact match: [CodeObject: openmp-amdgcn-amd-amdhsa-gfx906] : [Target: openmp-amdgcn-amd-amdhsa--gfx906] +// BUNDLECOMPATIBILITY: Compatible: Exact match: [CodeObject: openmp-amdgcn-amd-amdhsa--gfx908] : [Target: openmp-amdgcn-amd-amdhsa-gfx908] + // Some code so that we can create a binary out of this file. int A = 0; void test_func(void) { diff --git a/clang/test/Driver/hip-rdc-device-only.hip b/clang/test/Driver/hip-rdc-device-only.hip index a95f636d777c1..ca8d54ea633e2 100644 --- a/clang/test/Driver/hip-rdc-device-only.hip +++ b/clang/test/Driver/hip-rdc-device-only.hip @@ -82,7 +82,7 @@ // COMMON-SAME: {{.*}} {{".*a.cu"}} // COMMON: "{{.*}}clang-offload-bundler" "-type={{(bc|ll)}}" -// COMMON-SAME: "-targets=hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900" +// COMMON-SAME: "-targets=hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900" // COMMON-SAME: "-outputs=a-hip-amdgcn-amd-amdhsa.{{(bc|ll)}}" // COMMON: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa" @@ -112,7 +112,7 @@ // COMMON-SAME: {{.*}} {{".*b.hip"}} // COMMON: "{{.*}}clang-offload-bundler" "-type={{(bc|ll)}}" -// COMMON-SAME: "-targets=hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900" +// COMMON-SAME: "-targets=hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900" // COMMON-SAME: "-outputs=b-hip-amdgcn-amd-amdhsa.{{(bc|ll)}}" // SAVETEMP: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu" @@ -142,7 +142,7 @@ // SAVETEMP-SAME: {{.*}} "-o" {{"a.*.ll"}} "-x" "ir" [[A_GFX900_TMP_BC]] // SAVETEMP: "{{.*}}clang-offload-bundler" "-type=ll" -// SAVETEMP-SAME: "-targets=hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900" +// SAVETEMP-SAME: "-targets=hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900" // SAVETEMP-SAME: "-outputs=a-hip-amdgcn-amd-amdhsa.ll" // SAVETEMP: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu" @@ -172,7 +172,7 @@ // SAVETEMP-SAME: {{.*}} "-o" {{"b.*.ll"}} "-x" "ir" [[B_GFX900_TMP_BC]] // SAVETEMP: "{{.*}}clang-offload-bundler" "-type=ll" -// SAVETEMP-SAME: "-targets=hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900" +// SAVETEMP-SAME: "-targets=hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900" // SAVETEMP-SAME: "-outputs=b-hip-amdgcn-amd-amdhsa.ll" // FAIL: error: cannot specify -o when generating multiple output files diff --git a/clang/test/Driver/hip-toolchain-rdc-separate.hip b/clang/test/Driver/hip-toolchain-rdc-separate.hip index cdddbcc8fd216..698ee14e74dc9 100644 --- a/clang/test/Driver/hip-toolchain-rdc-separate.hip +++ b/clang/test/Driver/hip-toolchain-rdc-separate.hip @@ -44,7 +44,7 @@ // CHECK-SAME: {{.*}} [[A_SRC]] // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o" -// CHECK-SAME: "-targets=hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900,host-x86_64-unknown-linux-gnu" +// CHECK-SAME: "-targets=hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900,host-x86_64-unknown-linux-gnu" // CHECK-SAME: "-outputs=[[A_O:.*a.o]]" "-inputs=[[A_BC1]],[[A_BC2]],[[A_OBJ_HOST]]" // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa" @@ -79,7 +79,7 @@ // CHECK-SAME: {{.*}} [[B_SRC]] // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o" -// CHECK-SAME: "-targets=hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900,host-x86_64-unknown-linux-gnu" +// CHECK-SAME: "-targets=hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900,host-x86_64-unknown-linux-gnu" // CHECK-SAME: "-outputs=[[B_O:.*b.o]]" "-inputs=[[B_BC1]],[[B_BC2]],[[B_OBJ_HOST]]" // RUN: touch %T/a.o @@ -91,22 +91,22 @@ // RUN: 2>&1 | FileCheck -check-prefix=LINK %s // LINK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o" -// LINK-SAME: "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900" +// LINK-SAME: "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900" // LINK-SAME: "-inputs=[[A_O:.*a.o]]" "-outputs=[[A_OBJ_HOST:.*o]],{{.*o}},{{.*o}}" // LINK: "-unbundle" "-allow-missing-bundles" // LINK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o" -// LINK-SAME: "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900" +// LINK-SAME: "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900" // LINK-SAME: "-inputs=[[B_O:.*b.o]]" "-outputs=[[B_OBJ_HOST:.*o]],{{.*o}},{{.*o}}" // LINK: "-unbundle" "-allow-missing-bundles" // LINK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o" -// LINK-SAME: "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900" +// LINK-SAME: "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900" // LINK-SAME: "-inputs=[[A_O]]" "-outputs={{.*o}},[[A_BC1:.*o]],[[A_BC2:.*o]]" // LINK: "-unbundle" "-allow-missing-bundles" // LINK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o" -// LINK-SAME: "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa--gfx803,hip-amdgcn-amd-amdhsa--gfx900" +// LINK-SAME: "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900" // LINK-SAME: "-inputs=[[B_O]]" "-outputs={{.*o}},[[B_BC1:.*o]],[[B_BC2:.*o]]" // LINK: "-unbundle" "-allow-missing-bundles" diff --git a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp index 49275ec198c2a..522b5d33341fb 100644 --- a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp +++ b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp @@ -14,6 +14,7 @@ /// //===----------------------------------------------------------------------===// +#include "clang/Basic/Cuda.h" #include "clang/Basic/Version.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallString.h" @@ -134,21 +135,28 @@ static std::string BundlerExecutable; /// * Offload Kind - Host, OpenMP, or HIP /// * Triple - Standard LLVM Triple /// * GPUArch (Optional) - Processor name, like gfx906 or sm_30 -/// In presence of Proc, the Triple should contain separator "-" for all -/// standard four components, even if they are empty. + struct OffloadTargetInfo { StringRef OffloadKind; llvm::Triple Triple; StringRef GPUArch; OffloadTargetInfo(const StringRef Target) { - SmallVector Components; - Target.split(Components, '-', 5); - Components.resize(6); - this->OffloadKind = Components[0]; - this->Triple = llvm::Triple(Components[1], Components[2], Components[3], - Components[4]); - this->GPUArch = Components[5]; + auto TargetFeatures = Target.split(':'); + auto TripleOrGPU = TargetFeatures.first.rsplit('-'); + + if (clang::StringToCudaArch(TripleOrGPU.second) != + clang::CudaArch::UNKNOWN) { + auto KindTriple = TripleOrGPU.first.split('-'); + this->OffloadKind = KindTriple.first; + this->Triple = llvm::Triple(KindTriple.second); + this->GPUArch = Target.substr(Target.find(TripleOrGPU.second)); + } else { + auto KindTriple = TargetFeatures.first.split('-'); + this->OffloadKind = KindTriple.first; + this->Triple = llvm::Triple(KindTriple.second); + this->GPUArch = ""; + } } bool hasHostKind() const { return this->OffloadKind == "host"; } @@ -1063,9 +1071,10 @@ bool isCodeObjectCompatible(OffloadTargetInfo &CodeObjectInfo, // Compatible in case of exact match. if (CodeObjectInfo == TargetInfo) { - DEBUG_WITH_TYPE( - "CodeObjectCompatibility", - dbgs() << "Compatible: Exact match: " << CodeObjectInfo.str() << "\n"); + DEBUG_WITH_TYPE("CodeObjectCompatibility", + dbgs() << "Compatible: Exact match: \t[CodeObject: " + << CodeObjectInfo.str() + << "]\t:\t[Target: " << TargetInfo.str() << "]\n"); return true; } @@ -1276,9 +1285,19 @@ static Error UnbundleArchive() { } else if (!AllowMissingBundles) { std::string ErrMsg = Twine("no compatible code object found for the target '" + Target + - "' in heterogenous archive library: " + IFName) + "' in heterogeneous archive library: " + IFName) .str(); return createStringError(inconvertibleErrorCode(), ErrMsg); + } else { // Create an empty archive file if no compatible code object is + // found and "allow-missing-bundles" is enabled. It ensures that + // the linker using output of this step doesn't complain about + // the missing input file. + std::vector EmptyArchive; + EmptyArchive.clear(); + if (Error WriteErr = writeArchive(FileName, EmptyArchive, true, + getDefaultArchiveKindForHost(), true, + false, nullptr)) + return WriteErr; } }