Skip to content

Commit 744d4ad

Browse files
shiltianshiltian
committed
Reapply "[OffloadBundler] Rework the ctor of OffloadTargetInfo to support AMDGPU's generic target (llvm#122629)"
This reverts commit 9c870cb.
1 parent a56f802 commit 744d4ad

17 files changed

+135
-85
lines changed

‎clang/docs/ClangOffloadBundler.rst

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -266,15 +266,14 @@ without differentiation based on offload kind.
266266
The target triple of the code object. See `Target Triple
267267
<https://clang.llvm.org/docs/CrossCompilation.html#target-triple>`_.
268268

269-
The bundler accepts target triples with or without the optional environment
270-
field:
269+
LLVM target triples can be with or without the optional environment field:
271270

272271
``<arch><sub>-<vendor>-<sys>``, or
273272
``<arch><sub>-<vendor>-<sys>-<env>``
274273

275-
However, in order to standardize outputs for tools that consume bitcode
276-
bundles, bundles written by the bundler internally use only the 4-field
277-
target triple:
274+
However, in order to standardize outputs for tools that consume bitcode bundles
275+
and to parse target ID containing dashes, the bundler only accepts target
276+
triples in the 4-field format:
278277

279278
``<arch><sub>-<vendor>-<sys>-<env>``
280279

@@ -543,4 +542,4 @@ The compressed offload bundle begins with a header followed by the compressed bi
543542
- **Compressed Data**:
544543
The actual compressed binary data follows the header. Its size can be inferred from the total size of the file minus the header size.
545544

546-
> **Note**: Version 3 of the format is under development. It uses 64-bit fields for Total File Size and Uncompressed Binary Size to support files larger than 4GB. To experiment with version 3, set the environment variable `COMPRESSED_BUNDLE_FORMAT_VERSION=3`. This support is experimental and not recommended for production use.
545+
> **Note**: Version 3 of the format is under development. It uses 64-bit fields for Total File Size and Uncompressed Binary Size to support files larger than 4GB. To experiment with version 3, set the environment variable `COMPRESSED_BUNDLE_FORMAT_VERSION=3`. This support is experimental and not recommended for production use.

‎clang/include/clang/Driver/OffloadBundler.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,11 @@ class CompressedOffloadBundle {
128128
static llvm::Expected<std::unique_ptr<llvm::MemoryBuffer>>
129129
decompress(const llvm::MemoryBuffer &Input, bool Verbose = false);
130130
};
131+
132+
/// Check whether the bundle id is in the following format:
133+
/// <kind>-<triple>[-<target id>[:target features]]
134+
/// <triple> := <arch>-<vendor>-<os>-<env>
135+
bool checkOffloadBundleID(const llvm::StringRef Str);
131136
} // namespace clang
132137

133138
#endif // LLVM_CLANG_DRIVER_OFFLOADBUNDLER_H

‎clang/lib/Driver/OffloadBundler.cpp

Lines changed: 52 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -84,32 +84,27 @@ OffloadTargetInfo::OffloadTargetInfo(const StringRef Target,
8484
const OffloadBundlerConfig &BC)
8585
: BundlerConfig(BC) {
8686

87-
// TODO: Add error checking from ClangOffloadBundler.cpp
88-
auto TargetFeatures = Target.split(':');
89-
auto TripleOrGPU = TargetFeatures.first.rsplit('-');
90-
91-
if (clang::StringToOffloadArch(TripleOrGPU.second) !=
92-
clang::OffloadArch::UNKNOWN) {
93-
auto KindTriple = TripleOrGPU.first.split('-');
94-
this->OffloadKind = KindTriple.first;
95-
96-
// Enforce optional env field to standardize bundles
97-
llvm::Triple t = llvm::Triple(KindTriple.second);
98-
this->Triple = llvm::Triple(t.getArchName(), t.getVendorName(),
99-
t.getOSName(), t.getEnvironmentName());
100-
101-
this->TargetID = Target.substr(Target.find(TripleOrGPU.second));
102-
} else {
103-
auto KindTriple = TargetFeatures.first.split('-');
104-
this->OffloadKind = KindTriple.first;
105-
106-
// Enforce optional env field to standardize bundles
107-
llvm::Triple t = llvm::Triple(KindTriple.second);
108-
this->Triple = llvm::Triple(t.getArchName(), t.getVendorName(),
109-
t.getOSName(), t.getEnvironmentName());
110-
87+
// <kind>-<triple>[-<target id>[:target features]]
88+
// <triple> := <arch>-<vendor>-<os>-<env>
89+
SmallVector<StringRef, 6> Components;
90+
Target.split(Components, '-', /*MaxSplit=*/5);
91+
assert((Components.size() == 5 || Components.size() == 6) &&
92+
"malformed target string");
93+
94+
StringRef TargetIdWithFeature =
95+
Components.size() == 6 ? Components.back() : "";
96+
StringRef TargetId = TargetIdWithFeature.split(':').first;
97+
if (!TargetId.empty() &&
98+
clang::StringToOffloadArch(TargetId) != clang::OffloadArch::UNKNOWN)
99+
this->TargetID = TargetIdWithFeature;
100+
else
111101
this->TargetID = "";
112-
}
102+
103+
this->OffloadKind = Components.front();
104+
ArrayRef<StringRef> TripleSlice{&Components[1], /*length=*/4};
105+
llvm::Triple T = llvm::Triple(llvm::join(TripleSlice, "-"));
106+
this->Triple = llvm::Triple(T.getArchName(), T.getVendorName(), T.getOSName(),
107+
T.getEnvironmentName());
113108
}
114109

115110
bool OffloadTargetInfo::hasHostKind() const {
@@ -149,7 +144,18 @@ bool OffloadTargetInfo::operator==(const OffloadTargetInfo &Target) const {
149144
}
150145

151146
std::string OffloadTargetInfo::str() const {
152-
return Twine(OffloadKind + "-" + Triple.str() + "-" + TargetID).str();
147+
std::string NormalizedTriple;
148+
// Unfortunately we need some special sauce for AMDGPU because all the runtime
149+
// assumes the triple to be "amdgcn-amd-amdhsa-" (empty environment) instead
150+
// of "amdgcn-amd-amdhsa-unknown". It's gonna be very tricky to patch
151+
// different layers of runtime.
152+
if (Triple.isAMDGPU()) {
153+
NormalizedTriple = Triple.normalize(Triple::CanonicalForm::THREE_IDENT);
154+
NormalizedTriple.push_back('-');
155+
} else {
156+
NormalizedTriple = Triple.normalize(Triple::CanonicalForm::FOUR_IDENT);
157+
}
158+
return Twine(OffloadKind + "-" + NormalizedTriple + "-" + TargetID).str();
153159
}
154160

155161
static StringRef getDeviceFileExtension(StringRef Device,
@@ -1563,6 +1569,9 @@ Error OffloadBundler::UnbundleFiles() {
15631569
StringMap<StringRef> Worklist;
15641570
auto Output = BundlerConfig.OutputFileNames.begin();
15651571
for (auto &Triple : BundlerConfig.TargetNames) {
1572+
if (!checkOffloadBundleID(Triple))
1573+
return createStringError(errc::invalid_argument,
1574+
"invalid bundle id from bundle config");
15661575
Worklist[Triple] = *Output;
15671576
++Output;
15681577
}
@@ -1582,6 +1591,9 @@ Error OffloadBundler::UnbundleFiles() {
15821591

15831592
StringRef CurTriple = **CurTripleOrErr;
15841593
assert(!CurTriple.empty());
1594+
if (!checkOffloadBundleID(CurTriple))
1595+
return createStringError(errc::invalid_argument,
1596+
"invalid bundle id read from the bundle");
15851597

15861598
auto Output = Worklist.begin();
15871599
for (auto E = Worklist.end(); Output != E; Output++) {
@@ -1640,6 +1652,8 @@ Error OffloadBundler::UnbundleFiles() {
16401652
return createFileError(E.second, EC);
16411653

16421654
// If this entry has a host kind, copy the input file to the output file.
1655+
// We don't need to check E.getKey() here through checkOffloadBundleID
1656+
// because the entire WorkList has been checked above.
16431657
auto OffloadInfo = OffloadTargetInfo(E.getKey(), BundlerConfig);
16441658
if (OffloadInfo.hasHostKind())
16451659
OutputFile.write(Input.getBufferStart(), Input.getBufferSize());
@@ -1869,6 +1883,10 @@ Error OffloadBundler::UnbundleArchive() {
18691883
// archive.
18701884
while (!CodeObject.empty()) {
18711885
SmallVector<StringRef> CompatibleTargets;
1886+
if (!checkOffloadBundleID(CodeObject)) {
1887+
return createStringError(errc::invalid_argument,
1888+
"Invalid bundle id read from code object");
1889+
}
18721890
auto CodeObjectInfo = OffloadTargetInfo(CodeObject, BundlerConfig);
18731891
if (getCompatibleOffloadTargets(CodeObjectInfo, CompatibleTargets,
18741892
BundlerConfig)) {
@@ -1950,3 +1968,11 @@ Error OffloadBundler::UnbundleArchive() {
19501968

19511969
return Error::success();
19521970
}
1971+
1972+
bool clang::checkOffloadBundleID(const llvm::StringRef Str) {
1973+
// <kind>-<triple>[-<target id>[:target features]]
1974+
// <triple> := <arch>-<vendor>-<os>-<env>
1975+
SmallVector<StringRef, 6> Components;
1976+
Str.split(Components, '-', /*MaxSplit=*/5);
1977+
return Components.size() == 5 || Components.size() == 6;
1978+
}

‎clang/lib/Driver/ToolChains/Clang.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9166,7 +9166,8 @@ void OffloadBundler::ConstructJob(Compilation &C, const JobAction &JA,
91669166
}
91679167
Triples += Action::GetOffloadKindName(CurKind);
91689168
Triples += '-';
9169-
Triples += CurTC->getTriple().normalize();
9169+
Triples +=
9170+
CurTC->getTriple().normalize(llvm::Triple::CanonicalForm::FOUR_IDENT);
91709171
if ((CurKind == Action::OFK_HIP || CurKind == Action::OFK_Cuda) &&
91719172
!StringRef(CurDep->getOffloadingArch()).empty()) {
91729173
Triples += '-';
@@ -9269,7 +9270,8 @@ void OffloadBundler::ConstructJobMultipleOutputs(
92699270
auto OffloadKind = Dep.DependentOffloadKind;
92709271
Triples += Action::GetOffloadKindName(OffloadKind);
92719272
Triples += '-';
9272-
Triples += Dep.DependentToolChain->getTriple().normalize();
9273+
Triples += Dep.DependentToolChain->getTriple().normalize(
9274+
llvm::Triple::CanonicalForm::FOUR_IDENT);
92739275
if ((Dep.DependentOffloadKind == Action::OFK_HIP ||
92749276
Dep.DependentOffloadKind == Action::OFK_Cuda) &&
92759277
!Dep.DependentBoundArch.empty()) {

‎clang/lib/Driver/ToolChains/CommonArgs.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2766,7 +2766,8 @@ static void GetSDLFromOffloadArchive(
27662766
SmallString<128> DeviceTriple;
27672767
DeviceTriple += Action::GetOffloadKindName(JA.getOffloadingDeviceKind());
27682768
DeviceTriple += '-';
2769-
std::string NormalizedTriple = T.getToolChain().getTriple().normalize();
2769+
std::string NormalizedTriple = T.getToolChain().getTriple().normalize(
2770+
llvm::Triple::CanonicalForm::FOUR_IDENT);
27702771
DeviceTriple += NormalizedTriple;
27712772
if (!Target.empty()) {
27722773
DeviceTriple += '-';

‎clang/lib/Driver/ToolChains/HIPUtility.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ static std::string normalizeForBundler(const llvm::Triple &T,
4545
return HasTargetID ? (T.getArchName() + "-" + T.getVendorName() + "-" +
4646
T.getOSName() + "-" + T.getEnvironmentName())
4747
.str()
48-
: T.normalize();
48+
: T.normalize(llvm::Triple::CanonicalForm::FOUR_IDENT);
4949
}
5050

5151
// Collect undefined __hip_fatbin* and __hip_gpubin_handle* symbols from all

‎clang/test/Driver/clang-offload-bundler-asserts-on.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,18 @@
1515
// Check code object compatibility for archive unbundling
1616
//
1717
// Create few code object bundles and archive them to create an input archive
18-
// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-amdgcn-amd-amdhsa-gfx906,openmp-amdgcn-amd-amdhsa--gfx908 -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.simple.bundle
18+
// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-amdgcn-amd-amdhsa--gfx906,openmp-amdgcn-amd-amdhsa--gfx908 -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.simple.bundle
1919
// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-amdgcn-amd-amdhsa--gfx906:sramecc+:xnack+,openmp-amdgcn-amd-amdhsa--gfx908:sramecc+:xnack+ -inputs=%t.o,%t.tgt1,%t.tgt1 -outputs=%t.targetID1.bundle
2020
// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-amdgcn-amd-amdhsa--gfx906:sramecc+:xnack-,openmp-amdgcn-amd-amdhsa--gfx908:sramecc+:xnack- -inputs=%t.o,%t.tgt1,%t.tgt1 -outputs=%t.targetID2.bundle
2121
// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-amdgcn-amd-amdhsa--gfx906:xnack-,openmp-amdgcn-amd-amdhsa--gfx908:xnack- -inputs=%t.o,%t.tgt1,%t.tgt1 -outputs=%t.targetID3.bundle
2222
// RUN: llvm-ar cr %t.input-archive.a %t.simple.bundle %t.targetID1.bundle %t.targetID2.bundle %t.targetID3.bundle
2323

2424
// Tests to check compatibility between Bundle Entry ID formats i.e. between presence/absence of extra hyphen in case of missing environment field
25-
// RUN: clang-offload-bundler -unbundle -type=a -targets=openmp-amdgcn-amd-amdhsa--gfx906,openmp-amdgcn-amd-amdhsa-gfx908 -input=%t.input-archive.a -output=%t-archive-gfx906-simple.a -output=%t-archive-gfx908-simple.a -debug-only=CodeObjectCompatibility 2>&1 | FileCheck %s -check-prefix=BUNDLECOMPATIBILITY
25+
// RUN: clang-offload-bundler -unbundle -type=a -targets=openmp-amdgcn-amd-amdhsa--gfx906,openmp-amdgcn-amd-amdhsa--gfx908 -input=%t.input-archive.a -output=%t-archive-gfx906-simple.a -output=%t-archive-gfx908-simple.a -debug-only=CodeObjectCompatibility 2>&1 | FileCheck %s -check-prefix=BUNDLECOMPATIBILITY
2626
// BUNDLECOMPATIBILITY: Compatible: Exact match: [CodeObject: openmp-amdgcn-amd-amdhsa--gfx906] : [Target: openmp-amdgcn-amd-amdhsa--gfx906]
2727
// BUNDLECOMPATIBILITY: Compatible: Exact match: [CodeObject: openmp-amdgcn-amd-amdhsa--gfx908] : [Target: openmp-amdgcn-amd-amdhsa--gfx908]
2828

29-
// RUN: clang-offload-bundler -unbundle -type=a -targets=hip-amdgcn-amd-amdhsa--gfx906,hipv4-amdgcn-amd-amdhsa-gfx908 -input=%t.input-archive.a -output=%t-hip-archive-gfx906-simple.a -output=%t-hipv4-archive-gfx908-simple.a -hip-openmp-compatible -debug-only=CodeObjectCompatibility 2>&1 | FileCheck %s -check-prefix=HIPOpenMPCOMPATIBILITY
29+
// RUN: clang-offload-bundler -unbundle -type=a -targets=hip-amdgcn-amd-amdhsa--gfx906,hipv4-amdgcn-amd-amdhsa--gfx908 -input=%t.input-archive.a -output=%t-hip-archive-gfx906-simple.a -output=%t-hipv4-archive-gfx908-simple.a -hip-openmp-compatible -debug-only=CodeObjectCompatibility 2>&1 | FileCheck %s -check-prefix=HIPOpenMPCOMPATIBILITY
3030
// HIPOpenMPCOMPATIBILITY: Compatible: Target IDs are compatible [CodeObject: openmp-amdgcn-amd-amdhsa--gfx906] : [Target: hip-amdgcn-amd-amdhsa--gfx906]
3131
// HIPOpenMPCOMPATIBILITY: Compatible: Target IDs are compatible [CodeObject: openmp-amdgcn-amd-amdhsa--gfx908] : [Target: hipv4-amdgcn-amd-amdhsa--gfx908]
3232

‎clang/test/Driver/clang-offload-bundler-standardize.c

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,12 @@
1515
//
1616
// Check code object compatibility for archive unbundling
1717
//
18-
// Create an object bundle with and without env fields
19-
// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,hip-amdgcn-amd-amdhsa-gfx906,hip-amdgcn-amd-amdhsa-gfx908 -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle.no.env
20-
// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple-,hip-amdgcn-amd-amdhsa--gfx906,hip-amdgcn-amd-amdhsa--gfx908 -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle.env
18+
// Create an object bundle
19+
// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,hip-amdgcn-amd-amdhsa--gfx906,hip-amdgcn-amd-amdhsa--gfx908 -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle
2120

22-
23-
// Unbundle bundle.no.env while providing targets with env
24-
// RUN: clang-offload-bundler -unbundle -type=o -targets=hip-amdgcn-amd-amdhsa--gfx906,hip-amdgcn-amd-amdhsa--gfx908 -input=%t.bundle.no.env -output=%t-hip-amdgcn-amd-amdhsa--gfx906.bc -output=%t-hip-amdgcn-amd-amdhsa--gfx908.bc -debug-only=CodeObjectCompatibility 2>&1 | FileCheck %s -check-prefix=BUNDLE-NO-ENV
25-
// BUNDLE-NO-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx906] : [Target: hip-amdgcn-amd-amdhsa--gfx906]
26-
// BUNDLE-NO-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx908] : [Target: hip-amdgcn-amd-amdhsa--gfx908]
27-
28-
// Unbundle bundle.env while providing targets with no env
29-
// RUN: clang-offload-bundler -unbundle -type=o -targets=hip-amdgcn-amd-amdhsa-gfx906,hip-amdgcn-amd-amdhsa-gfx908 -input=%t.bundle.env -output=%t-hip-amdgcn-amd-amdhsa-gfx906.bc -output=%t-hip-amdgcn-amd-amdhsa-gfx908.bc -debug-only=CodeObjectCompatibility 2>&1 | FileCheck %s -check-prefix=BUNDLE-ENV
30-
// BUNDLE-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx906] : [Target: hip-amdgcn-amd-amdhsa--gfx906]
31-
// BUNDLE-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx908] : [Target: hip-amdgcn-amd-amdhsa--gfx908]
21+
// RUN: clang-offload-bundler -unbundle -type=o -targets=hip-amdgcn-amd-amdhsa--gfx906,hip-amdgcn-amd-amdhsa--gfx908 -input=%t.bundle -output=%t-hip-amdgcn-amd-amdhsa--gfx906.bc -output=%t-hip-amdgcn-amd-amdhsa--gfx908.bc -debug-only=CodeObjectCompatibility 2>&1 | FileCheck %s -check-prefix=BUNDLE
22+
// BUNDLE: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx906] : [Target: hip-amdgcn-amd-amdhsa--gfx906]
23+
// BUNDLE: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx908] : [Target: hip-amdgcn-amd-amdhsa--gfx908]
3224

3325
// Some code so that we can create a binary out of this file.
3426
int A = 0;

0 commit comments

Comments
 (0)