Skip to content

Commit

Permalink
[OffloadBundler] Rework the ctor of OffloadTargetInfo to support ge…
Browse files Browse the repository at this point in the history 
…neric target

The current parsing of target string assumes to be in a form of
`kind-triple-targetid:feature`, such as
`hipv4-amdgcn-amd-amdhsa-gfx1030:+xnack`. Specifically, the target id does not
contain any `-`, which is not the case for generic target. Also, a generic
target may contain one or more `-`, such as `gfx10-3-generic` and
`gfx12-generic`. As a result, we can no longer depend on `rstrip` to get things
work right. This patch reworks the logic to parse the target string to make it
more robust, as well as supporting generic target.
  • Loading branch information
@shiltian
shiltian committed Feb 10, 2025
1 parent f3cd223 commit 0075687
Show file tree
Hide file tree
Showing 17 changed files with 116 additions and 113 deletions.
7 changes: 3 additions & 4 deletions clang/docs/ClangOffloadBundler.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,15 +266,14 @@ without differentiation based on offload kind.
The target triple of the code object. See `Target Triple
<https://clang.llvm.org/docs/CrossCompilation.html#target-triple>`_.

The bundler accepts target triples with or without the optional environment
field:
LLVM target triples can be with or without the optional environment field:

``<arch><sub>-<vendor>-<sys>``, or
``<arch><sub>-<vendor>-<sys>-<env>``

However, in order to standardize outputs for tools that consume bitcode
bundles, bundles written by the bundler internally use only the 4-field
target triple:
bundles, the bundler only accepts target triples with the 4-field target
triple:

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

Expand Down
49 changes: 24 additions & 25 deletions clang/lib/Driver/OffloadBundler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,31 +84,27 @@ OffloadTargetInfo::OffloadTargetInfo(const StringRef Target,
: BundlerConfig(BC) {

// TODO: Add error checking from ClangOffloadBundler.cpp
auto TargetFeatures = Target.split(':');
auto TripleOrGPU = TargetFeatures.first.rsplit('-');

if (clang::StringToOffloadArch(TripleOrGPU.second) !=
clang::OffloadArch::UNKNOWN) {
auto KindTriple = TripleOrGPU.first.split('-');
this->OffloadKind = KindTriple.first;

// Enforce optional env field to standardize bundles
llvm::Triple t = llvm::Triple(KindTriple.second);
this->Triple = llvm::Triple(t.getArchName(), t.getVendorName(),
t.getOSName(), t.getEnvironmentName());

this->TargetID = Target.substr(Target.find(TripleOrGPU.second));
} else {
auto KindTriple = TargetFeatures.first.split('-');
this->OffloadKind = KindTriple.first;

// Enforce optional env field to standardize bundles
llvm::Triple t = llvm::Triple(KindTriple.second);
this->Triple = llvm::Triple(t.getArchName(), t.getVendorName(),
t.getOSName(), t.getEnvironmentName());

// <kind>-<triple>[-<target id>[:target features]]
// <triple> := <arch>-<vendor>-<os>-<env>
SmallVector<StringRef, 6> Components;
Target.split(Components, '-', /*MaxSplit=*/5);
assert((Components.size() == 5 || Components.size() == 6) &&
"malformed target string");

StringRef TargetIdWithFeature =
Components.size() == 6 ? Components.back() : "";
StringRef TargetId = TargetIdWithFeature.split(':').first;
if (!TargetId.empty() &&
clang::StringToOffloadArch(TargetId) != clang::OffloadArch::UNKNOWN)
this->TargetID = TargetIdWithFeature;
else
this->TargetID = "";
}

this->OffloadKind = Components.front();
ArrayRef<StringRef> TripleSlice{&Components[1], /*length=*/4};
llvm::Triple T = llvm::Triple(llvm::join(TripleSlice, "-"));
this->Triple = llvm::Triple(T.getArchName(), T.getVendorName(), T.getOSName(),
T.getEnvironmentName());
}

bool OffloadTargetInfo::hasHostKind() const {
Expand Down Expand Up @@ -148,7 +144,10 @@ bool OffloadTargetInfo::operator==(const OffloadTargetInfo &Target) const {
}

std::string OffloadTargetInfo::str() const {
return Twine(OffloadKind + "-" + Triple.str() + "-" + TargetID).str();
return Twine(OffloadKind + "-" +
Triple.normalize(Triple::CanonicalForm::FOUR_IDENT) + "-" +
TargetID)
.str();
}

static StringRef getDeviceFileExtension(StringRef Device,
Expand Down
6 changes: 4 additions & 2 deletions clang/lib/Driver/ToolChains/Clang.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8981,7 +8981,8 @@ void OffloadBundler::ConstructJob(Compilation &C, const JobAction &JA,
}
Triples += Action::GetOffloadKindName(CurKind);
Triples += '-';
Triples += CurTC->getTriple().normalize();
Triples +=
CurTC->getTriple().normalize(llvm::Triple::CanonicalForm::FOUR_IDENT);
if ((CurKind == Action::OFK_HIP || CurKind == Action::OFK_Cuda) &&
!StringRef(CurDep->getOffloadingArch()).empty()) {
Triples += '-';
Expand Down Expand Up @@ -9075,7 +9076,8 @@ void OffloadBundler::ConstructJobMultipleOutputs(
auto &Dep = DepInfo[I];
Triples += Action::GetOffloadKindName(Dep.DependentOffloadKind);
Triples += '-';
Triples += Dep.DependentToolChain->getTriple().normalize();
Triples += Dep.DependentToolChain->getTriple().normalize(
llvm::Triple::CanonicalForm::FOUR_IDENT);
if ((Dep.DependentOffloadKind == Action::OFK_HIP ||
Dep.DependentOffloadKind == Action::OFK_Cuda) &&
!Dep.DependentBoundArch.empty()) {
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Driver/ToolChains/CommonArgs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2569,7 +2569,8 @@ static void GetSDLFromOffloadArchive(
SmallString<128> DeviceTriple;
DeviceTriple += Action::GetOffloadKindName(JA.getOffloadingDeviceKind());
DeviceTriple += '-';
std::string NormalizedTriple = T.getToolChain().getTriple().normalize();
std::string NormalizedTriple = T.getToolChain().getTriple().normalize(
llvm::Triple::CanonicalForm::FOUR_IDENT);
DeviceTriple += NormalizedTriple;
if (!Target.empty()) {
DeviceTriple += '-';
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Driver/ToolChains/HIPUtility.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ static std::string normalizeForBundler(const llvm::Triple &T,
return HasTargetID ? (T.getArchName() + "-" + T.getVendorName() + "-" +
T.getOSName() + "-" + T.getEnvironmentName())
.str()
: T.normalize();
: T.normalize(llvm::Triple::CanonicalForm::FOUR_IDENT);
}

// Collect undefined __hip_fatbin* and __hip_gpubin_handle* symbols from all
Expand Down
14 changes: 7 additions & 7 deletions clang/test/Driver/clang-offload-bundler-asserts-on.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,20 @@
// Check code object compatibility for 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 -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.simple.bundle
// 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
// 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
// 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
// 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
// RUN: llvm-ar cr %t.input-archive.a %t.simple.bundle %t.targetID1.bundle %t.targetID2.bundle %t.targetID3.bundle

// 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 -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
// 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]
// 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
// BUNDLECOMPATIBILITY: Compatible: Exact match: [CodeObject: openmp-amdgcn-amd-amdhsa-unknown-gfx906] : [Target: openmp-amdgcn-amd-amdhsa-unknown-gfx906]
// BUNDLECOMPATIBILITY: Compatible: Exact match: [CodeObject: openmp-amdgcn-amd-amdhsa-unknown-gfx908] : [Target: openmp-amdgcn-amd-amdhsa-unknown-gfx908]

// 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
// HIPOpenMPCOMPATIBILITY: Compatible: Target IDs are compatible [CodeObject: openmp-amdgcn-amd-amdhsa--gfx906] : [Target: hip-amdgcn-amd-amdhsa--gfx906]
// HIPOpenMPCOMPATIBILITY: Compatible: Target IDs are compatible [CodeObject: openmp-amdgcn-amd-amdhsa--gfx908] : [Target: hipv4-amdgcn-amd-amdhsa--gfx908]
// 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
// HIPOpenMPCOMPATIBILITY: Compatible: Target IDs are compatible [CodeObject: openmp-amdgcn-amd-amdhsa-unknown-gfx906] : [Target: hip-amdgcn-amd-amdhsa-unknown-gfx906]
// HIPOpenMPCOMPATIBILITY: Compatible: Target IDs are compatible [CodeObject: openmp-amdgcn-amd-amdhsa-unknown-gfx908] : [Target: hipv4-amdgcn-amd-amdhsa-unknown-gfx908]

// Some code so that we can create a binary out of this file.
int A = 0;
Expand Down
18 changes: 5 additions & 13 deletions clang/test/Driver/clang-offload-bundler-standardize.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,12 @@
//
// Check code object compatibility for archive unbundling
//
// Create an object bundle with and without env fields
// 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
// 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
// Create an object bundle
// 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


// Unbundle bundle.no.env while providing targets with env
// 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
// BUNDLE-NO-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx906] : [Target: hip-amdgcn-amd-amdhsa--gfx906]
// BUNDLE-NO-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx908] : [Target: hip-amdgcn-amd-amdhsa--gfx908]

// Unbundle bundle.env while providing targets with no env
// 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
// BUNDLE-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx906] : [Target: hip-amdgcn-amd-amdhsa--gfx906]
// BUNDLE-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx908] : [Target: hip-amdgcn-amd-amdhsa--gfx908]
// 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
// BUNDLE: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa-unknown-gfx906] : [Target: hip-amdgcn-amd-amdhsa-unknown-gfx906]
// BUNDLE: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa-unknown-gfx908] : [Target: hip-amdgcn-amd-amdhsa-unknown-gfx908]

// Some code so that we can create a binary out of this file.
int A = 0;
Expand Down
12 changes: 6 additions & 6 deletions clang/test/Driver/clang-offload-bundler-zlib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
// DECOMPRESS: Decompression method: zlib
// DECOMPRESS: Hashes match: Yes
// NOHOST-NOT: host-
// NOHOST-DAG: hip-amdgcn-amd-amdhsa--gfx900
// NOHOST-DAG: hip-amdgcn-amd-amdhsa--gfx906
// NOHOST-DAG: hip-amdgcn-amd-amdhsa-unknown-gfx900
// NOHOST-DAG: hip-amdgcn-amd-amdhsa-unknown-gfx906
//

// Check compression/decompression of offload bundle using version 3 format.
Expand Down Expand Up @@ -102,10 +102,10 @@
// RUN: -output=%t.hip_900.a -output=%t.hip_906.a -input=%t.hip_archive.a
// RUN: llvm-ar t %t.hip_900.a | FileCheck -check-prefix=HIP-AR-900 %s
// RUN: llvm-ar t %t.hip_906.a | FileCheck -check-prefix=HIP-AR-906 %s
// HIP-AR-900-DAG: hip_bundle1-hip-amdgcn-amd-amdhsa--gfx900
// HIP-AR-900-DAG: hip_bundle2-hip-amdgcn-amd-amdhsa--gfx900
// HIP-AR-906-DAG: hip_bundle1-hip-amdgcn-amd-amdhsa--gfx906
// HIP-AR-906-DAG: hip_bundle2-hip-amdgcn-amd-amdhsa--gfx906
// HIP-AR-900-DAG: hip_bundle1-hip-amdgcn-amd-amdhsa-unknown-gfx900
// HIP-AR-900-DAG: hip_bundle2-hip-amdgcn-amd-amdhsa-unknown-gfx900
// HIP-AR-906-DAG: hip_bundle1-hip-amdgcn-amd-amdhsa-unknown-gfx906
// HIP-AR-906-DAG: hip_bundle2-hip-amdgcn-amd-amdhsa-unknown-gfx906

// Some code so that we can create a binary out of this file.
int A = 0;
Expand Down
12 changes: 6 additions & 6 deletions clang/test/Driver/clang-offload-bundler-zstd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
// CHECK: Decompression method: zstd
// CHECK: Hashes match: Yes
// NOHOST-NOT: host-
// NOHOST-DAG: hip-amdgcn-amd-amdhsa--gfx900
// NOHOST-DAG: hip-amdgcn-amd-amdhsa--gfx906
// NOHOST-DAG: hip-amdgcn-amd-amdhsa-unknown-gfx900
// NOHOST-DAG: hip-amdgcn-amd-amdhsa-unknown-gfx906
//

// Check -compression-level= option
Expand Down Expand Up @@ -77,10 +77,10 @@
// RUN: -output=%t.hip_900.a -output=%t.hip_906.a -input=%t.hip_archive.a
// RUN: llvm-ar t %t.hip_900.a | FileCheck -check-prefix=HIP-AR-900 %s
// RUN: llvm-ar t %t.hip_906.a | FileCheck -check-prefix=HIP-AR-906 %s
// HIP-AR-900-DAG: hip_bundle1-hip-amdgcn-amd-amdhsa--gfx900
// HIP-AR-900-DAG: hip_bundle2-hip-amdgcn-amd-amdhsa--gfx900
// HIP-AR-906-DAG: hip_bundle1-hip-amdgcn-amd-amdhsa--gfx906
// HIP-AR-906-DAG: hip_bundle2-hip-amdgcn-amd-amdhsa--gfx906
// HIP-AR-900-DAG: hip_bundle1-hip-amdgcn-amd-amdhsa-unknown-gfx900
// HIP-AR-900-DAG: hip_bundle2-hip-amdgcn-amd-amdhsa-unknown-gfx900
// HIP-AR-906-DAG: hip_bundle1-hip-amdgcn-amd-amdhsa-unknown-gfx906
// HIP-AR-906-DAG: hip_bundle2-hip-amdgcn-amd-amdhsa-unknown-gfx906

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

0 comments on commit 0075687

Please sign in to comment.