Skip to content

Commit ad6f6be

Browse files
shiltianshiltian
committed
[OffloadBundler] Rework the ctor of OffloadTargetInfo to support AMDGPU's generic target (llvm#122629)
The current parsing logic for the target string assumes it follows the format `<kind>-<triple>-<target id>:<feature>`, such as `hipv4-amdgcn-amd-amdhsa-gfx1030:+xnack`. Specifically, it assumes that `<target id>` does not contain any `-`, relying on `rsplit` for parsing. However, this assumption breaks for AMDGPU's generic targets, which may contain one or more `-`, such as `gfx10-3-generic` or `gfx12-generic`. As a result, the existing approach using `rstrip` is no longer reliable. This patch reworks the parsing logic to handle target strings more robustly, including support for generic targets. The bundler now strictly requires a 4-field target triple. Additionally, a new Python helper function has been added to `config.py` to normalize the target triple into the 4-field format when it is not, ensuring tests pass reliably. (cherry picked from commit d85a81b)
1 parent bf51372 commit ad6f6be

17 files changed

+133
-85
lines changed

‎clang/docs/ClangOffloadBundler.rst

Lines changed: 4 additions & 5 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

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ class CompressedOffloadBundle {
131131
decompress(const llvm::MemoryBuffer &Input, bool Verbose = false);
132132
};
133133

134+
/// Check whether the bundle id is in the following format:
135+
/// <kind>-<triple>[-<target id>[:target features]]
136+
/// <triple> := <arch>-<vendor>-<os>-<env>
137+
bool checkOffloadBundleID(const llvm::StringRef Str);
134138
} // namespace clang
135139

136140
#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
@@ -75,32 +75,27 @@ OffloadTargetInfo::OffloadTargetInfo(const StringRef Target,
7575
const OffloadBundlerConfig &BC)
7676
: BundlerConfig(BC) {
7777

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

106101
bool OffloadTargetInfo::hasHostKind() const {
@@ -140,7 +135,18 @@ bool OffloadTargetInfo::operator==(const OffloadTargetInfo &Target) const {
140135
}
141136

142137
std::string OffloadTargetInfo::str() const {
143-
return Twine(OffloadKind + "-" + Triple.str() + "-" + TargetID).str();
138+
std::string NormalizedTriple;
139+
// Unfortunately we need some special sauce for AMDGPU because all the runtime
140+
// assumes the triple to be "amdgcn-amd-amdhsa-" (empty environment) instead
141+
// of "amdgcn-amd-amdhsa-unknown". It's gonna be very tricky to patch
142+
// different layers of runtime.
143+
if (Triple.isAMDGPU()) {
144+
NormalizedTriple = Triple.normalize(Triple::CanonicalForm::THREE_IDENT);
145+
NormalizedTriple.push_back('-');
146+
} else {
147+
NormalizedTriple = Triple.normalize(Triple::CanonicalForm::FOUR_IDENT);
148+
}
149+
return Twine(OffloadKind + "-" + NormalizedTriple + "-" + TargetID).str();
144150
}
145151

146152
static StringRef getDeviceFileExtension(StringRef Device,
@@ -1424,6 +1430,9 @@ Error OffloadBundler::UnbundleFiles() {
14241430
StringMap<StringRef> Worklist;
14251431
auto Output = BundlerConfig.OutputFileNames.begin();
14261432
for (auto &Triple : BundlerConfig.TargetNames) {
1433+
if (!checkOffloadBundleID(Triple))
1434+
return createStringError(errc::invalid_argument,
1435+
"invalid bundle id from bundle config");
14271436
Worklist[Triple] = *Output;
14281437
++Output;
14291438
}
@@ -1443,6 +1452,9 @@ Error OffloadBundler::UnbundleFiles() {
14431452

14441453
StringRef CurTriple = **CurTripleOrErr;
14451454
assert(!CurTriple.empty());
1455+
if (!checkOffloadBundleID(CurTriple))
1456+
return createStringError(errc::invalid_argument,
1457+
"invalid bundle id read from the bundle");
14461458

14471459
auto Output = Worklist.begin();
14481460
for (auto E = Worklist.end(); Output != E; Output++) {
@@ -1501,6 +1513,8 @@ Error OffloadBundler::UnbundleFiles() {
15011513
return createFileError(E.second, EC);
15021514

15031515
// If this entry has a host kind, copy the input file to the output file.
1516+
// We don't need to check E.getKey() here through checkOffloadBundleID
1517+
// because the entire WorkList has been checked above.
15041518
auto OffloadInfo = OffloadTargetInfo(E.getKey(), BundlerConfig);
15051519
if (OffloadInfo.hasHostKind())
15061520
OutputFile.write(Input.getBufferStart(), Input.getBufferSize());
@@ -1730,6 +1744,10 @@ Error OffloadBundler::UnbundleArchive() {
17301744
// archive.
17311745
while (!CodeObject.empty()) {
17321746
SmallVector<StringRef> CompatibleTargets;
1747+
if (!checkOffloadBundleID(CodeObject)) {
1748+
return createStringError(errc::invalid_argument,
1749+
"Invalid bundle id read from code object");
1750+
}
17331751
auto CodeObjectInfo = OffloadTargetInfo(CodeObject, BundlerConfig);
17341752
if (getCompatibleOffloadTargets(CodeObjectInfo, CompatibleTargets,
17351753
BundlerConfig)) {
@@ -1819,3 +1837,11 @@ Error OffloadBundler::UnbundleArchive() {
18191837

18201838
return Error::success();
18211839
}
1840+
1841+
bool clang::checkOffloadBundleID(const llvm::StringRef Str) {
1842+
// <kind>-<triple>[-<target id>[:target features]]
1843+
// <triple> := <arch>-<vendor>-<os>-<env>
1844+
SmallVector<StringRef, 6> Components;
1845+
Str.split(Components, '-', /*MaxSplit=*/5);
1846+
return Components.size() == 5 || Components.size() == 6;
1847+
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8969,7 +8969,8 @@ void OffloadBundler::ConstructJob(Compilation &C, const JobAction &JA,
89698969
}
89708970
Triples += Action::GetOffloadKindName(CurKind);
89718971
Triples += '-';
8972-
Triples += CurTC->getTriple().normalize();
8972+
Triples +=
8973+
CurTC->getTriple().normalize(llvm::Triple::CanonicalForm::FOUR_IDENT);
89738974
if ((CurKind == Action::OFK_HIP || CurKind == Action::OFK_Cuda) &&
89748975
!StringRef(CurDep->getOffloadingArch()).empty()) {
89758976
Triples += '-';
@@ -9072,7 +9073,8 @@ void OffloadBundler::ConstructJobMultipleOutputs(
90729073
auto OffloadKind = Dep.DependentOffloadKind;
90739074
Triples += Action::GetOffloadKindName(OffloadKind);
90749075
Triples += '-';
9075-
Triples += Dep.DependentToolChain->getTriple().normalize();
9076+
Triples += Dep.DependentToolChain->getTriple().normalize(
9077+
llvm::Triple::CanonicalForm::FOUR_IDENT);
90769078
if ((Dep.DependentOffloadKind == Action::OFK_HIP ||
90779079
Dep.DependentOffloadKind == Action::OFK_Cuda) &&
90789080
!Dep.DependentBoundArch.empty()) {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2660,7 +2660,8 @@ static void GetSDLFromOffloadArchive(
26602660
SmallString<128> DeviceTriple;
26612661
DeviceTriple += Action::GetOffloadKindName(JA.getOffloadingDeviceKind());
26622662
DeviceTriple += '-';
2663-
std::string NormalizedTriple = T.getToolChain().getTriple().normalize();
2663+
std::string NormalizedTriple = T.getToolChain().getTriple().normalize(
2664+
llvm::Triple::CanonicalForm::FOUR_IDENT);
26642665
DeviceTriple += NormalizedTriple;
26652666
if (!Target.empty()) {
26662667
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)