Skip to content

Commit 0c8b435

Browse files
linehilllinehill
authored
[Clang][retry 2] Lift HIPSPV onto the new offload driver (#179902)
Update HIPSPV toolchain to support `--offload-new-driver`. Additionally, tailor llvm-spirv invocation for [chipStar](github.com/CHIP-SPV/chipStar) via `spirv64-*-chipstar` offload triple. AFAICT, all the relevant test failures in the previous PR (#178664) came from tests involving `-Xoffload-compiler ‘-###’` in their RUN directives. I have reworked those tests in this PR.
1 parent 4d27530 commit 0c8b435

File tree

15 files changed

+441
-114
lines changed

15 files changed

+441
-114
lines changed

‎clang/docs/ReleaseNotes.rst‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@ AST Dumping Potentially Breaking Changes
5252
Clang Frontend Potentially Breaking Changes
5353
-------------------------------------------
5454

55+
- HIPSPV toolchain: `--offload-targets=spirv{32,64}` option is
56+
deprecated and will be removed when the new offload driver becomes
57+
default. The replacement for the option is
58+
`--offload-targets=spirv{32,64}-unknown-chipstar` when using the new
59+
offload driver (`--offload-new-driver`).
60+
61+
5562
Clang Python Bindings Potentially Breaking Changes
5663
--------------------------------------------------
5764
- Remove ``CompletionString.Availability``. No libclang interfaces returned instances of it.

‎clang/lib/Basic/Targets.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -682,13 +682,13 @@ std::unique_ptr<TargetInfo> AllocateTarget(const llvm::Triple &Triple,
682682
return std::make_unique<SPIRVTargetInfo>(Triple, Opts);
683683
}
684684
case llvm::Triple::spirv32: {
685-
if (os != llvm::Triple::UnknownOS ||
685+
if ((os != llvm::Triple::UnknownOS && os != llvm::Triple::ChipStar) ||
686686
Triple.getEnvironment() != llvm::Triple::UnknownEnvironment)
687687
return nullptr;
688688
return std::make_unique<SPIRV32TargetInfo>(Triple, Opts);
689689
}
690690
case llvm::Triple::spirv64: {
691-
if (os != llvm::Triple::UnknownOS ||
691+
if ((os != llvm::Triple::UnknownOS && os != llvm::Triple::ChipStar) ||
692692
Triple.getEnvironment() != llvm::Triple::UnknownEnvironment) {
693693
if (os == llvm::Triple::OSType::AMDHSA)
694694
return std::make_unique<SPIRV64AMDGCNTargetInfo>(Triple, Opts);

‎clang/lib/Basic/Targets/SPIR.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -362,8 +362,9 @@ class LLVM_LIBRARY_VISIBILITY SPIRV32TargetInfo : public BaseSPIRVTargetInfo {
362362
: BaseSPIRVTargetInfo(Triple, Opts) {
363363
assert(Triple.getArch() == llvm::Triple::spirv32 &&
364364
"Invalid architecture for 32-bit SPIR-V.");
365-
assert(getTriple().getOS() == llvm::Triple::UnknownOS &&
366-
"32-bit SPIR-V target must use unknown OS");
365+
assert((getTriple().getOS() == llvm::Triple::UnknownOS ||
366+
getTriple().getOS() == llvm::Triple::ChipStar) &&
367+
"32-bit SPIR-V target must use unknown or chipstar OS");
367368
assert(getTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
368369
"32-bit SPIR-V target must use unknown environment type");
369370
PointerWidth = PointerAlign = 32;
@@ -385,8 +386,9 @@ class LLVM_LIBRARY_VISIBILITY SPIRV64TargetInfo : public BaseSPIRVTargetInfo {
385386
: BaseSPIRVTargetInfo(Triple, Opts) {
386387
assert(Triple.getArch() == llvm::Triple::spirv64 &&
387388
"Invalid architecture for 64-bit SPIR-V.");
388-
assert(getTriple().getOS() == llvm::Triple::UnknownOS &&
389-
"64-bit SPIR-V target must use unknown OS");
389+
assert((getTriple().getOS() == llvm::Triple::UnknownOS ||
390+
getTriple().getOS() == llvm::Triple::ChipStar) &&
391+
"64-bit SPIR-V target must use unknown or chipstar OS");
390392
assert(getTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
391393
"64-bit SPIR-V target must use unknown environment type");
392394
PointerWidth = PointerAlign = 64;

‎clang/lib/Driver/Driver.cpp‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4954,21 +4954,26 @@ Action *Driver::BuildOffloadingActions(Compilation &C,
49544954
// Compiling HIP in device-only non-RDC mode requires linking each action
49554955
// individually.
49564956
for (Action *&A : DeviceActions) {
4957-
bool IsAMDGCNSPIRV = A->getOffloadingToolChain() &&
4958-
A->getOffloadingToolChain()->getTriple().getOS() ==
4959-
llvm::Triple::OSType::AMDHSA &&
4960-
A->getOffloadingToolChain()->getTriple().isSPIRV();
4957+
auto *OffloadTriple = A->getOffloadingToolChain()
4958+
? &A->getOffloadingToolChain()->getTriple()
4959+
: nullptr;
4960+
bool IsHIPSPV =
4961+
OffloadTriple && OffloadTriple->isSPIRV() &&
4962+
(OffloadTriple->getOS() == llvm::Triple::OSType::AMDHSA ||
4963+
OffloadTriple->getOS() == llvm::Triple::OSType::ChipStar);
49614964
bool UseSPIRVBackend = Args.hasFlag(options::OPT_use_spirv_backend,
49624965
options::OPT_no_use_spirv_backend,
49634966
/*Default=*/false);
49644967

4965-
// Special handling for the HIP SPIR-V toolchain in device-only.
4968+
// Special handling for the HIP SPIR-V toolchains in device-only.
49664969
// The translator path has a linking step, whereas the SPIR-V backend path
49674970
// does not to avoid any external dependency such as spirv-link. The
49684971
// linking step is skipped for the SPIR-V backend path.
4969-
bool IsAMDGCNSPIRVWithBackend = IsAMDGCNSPIRV && UseSPIRVBackend;
4972+
bool IsAMDGCNSPIRVWithBackend =
4973+
IsHIPSPV && OffloadTriple->getOS() == llvm::Triple::OSType::AMDHSA &&
4974+
UseSPIRVBackend;
49704975

4971-
if ((A->getType() != types::TY_Object && !IsAMDGCNSPIRV &&
4976+
if ((A->getType() != types::TY_Object && !IsHIPSPV &&
49724977
A->getType() != types::TY_LTO_BC) ||
49734978
HIPRelocatableObj || !HIPNoRDC || !offloadDeviceOnly() ||
49744979
(IsAMDGCNSPIRVWithBackend && offloadDeviceOnly()))
@@ -6977,6 +6982,9 @@ const ToolChain &Driver::getToolChain(const ArgList &Args,
69776982
case llvm::Triple::ShaderModel:
69786983
TC = std::make_unique<toolchains::HLSLToolChain>(*this, Target, Args);
69796984
break;
6985+
case llvm::Triple::ChipStar:
6986+
TC = std::make_unique<toolchains::HIPSPVToolChain>(*this, Target, Args);
6987+
break;
69806988
default:
69816989
// Of these targets, Hexagon is the only one that might have
69826990
// an OS of Linux, in which case it got handled above already.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9208,6 +9208,7 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
92089208
OPT_v,
92099209
OPT_cuda_path_EQ,
92109210
OPT_rocm_path_EQ,
9211+
OPT_hip_path_EQ,
92119212
OPT_O_Group,
92129213
OPT_g_Group,
92139214
OPT_g_flags_Group,

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

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,27 @@ void HIPSPV::Linker::constructLinkAndEmitSpirvCommand(
9090
}
9191

9292
// Emit SPIR-V binary.
93+
llvm::opt::ArgStringList TrArgs;
94+
auto T = getToolChain().getTriple();
95+
bool HasNoSubArch = T.getSubArch() == llvm::Triple::NoSubArch;
96+
if (T.getOS() == llvm::Triple::ChipStar) {
97+
// chipStar needs 1.2 for supporting warp-level primitivies via sub-group
98+
// extensions. Strictly put we'd need 1.3 for the standard non-extension
99+
// shuffle operations, but it's not supported by any backend driver of the
100+
// chipStar.
101+
if (HasNoSubArch)
102+
TrArgs.push_back("--spirv-max-version=1.2");
103+
TrArgs.push_back("--spirv-ext=-all"
104+
// Needed for experimental indirect call support.
105+
",+SPV_INTEL_function_pointers"
106+
// Needed for shuffles below SPIR-V 1.3
107+
",+SPV_INTEL_subgroups");
108+
} else {
109+
if (HasNoSubArch)
110+
TrArgs.push_back("--spirv-max-version=1.1");
111+
TrArgs.push_back("--spirv-ext=+all");
112+
}
93113

94-
llvm::opt::ArgStringList TrArgs{"--spirv-max-version=1.1",
95-
"--spirv-ext=+all"};
96114
InputInfo TrInput = InputInfo(types::TY_LLVM_BC, TempFile, "");
97115
SPIRV::constructTranslateCommand(C, *this, JA, Output, TrInput, TrArgs);
98116
}
@@ -116,7 +134,16 @@ void HIPSPV::Linker::ConstructJob(Compilation &C, const JobAction &JA,
116134

117135
HIPSPVToolChain::HIPSPVToolChain(const Driver &D, const llvm::Triple &Triple,
118136
const ToolChain &HostTC, const ArgList &Args)
119-
: ToolChain(D, Triple, Args), HostTC(HostTC) {
137+
: ToolChain(D, Triple, Args), HostTC(&HostTC) {
138+
// Lookup binaries into the driver directory, this is used to
139+
// discover the clang-offload-bundler executable.
140+
getProgramPaths().push_back(getDriver().Dir);
141+
}
142+
143+
// Non-offloading toolchain. Primaly used by clang-offload-linker.
144+
HIPSPVToolChain::HIPSPVToolChain(const Driver &D, const llvm::Triple &Triple,
145+
const ArgList &Args)
146+
: ToolChain(D, Triple, Args), HostTC(nullptr) {
120147
// Lookup binaries into the driver directory, this is used to
121148
// discover the clang-offload-bundler executable.
122149
getProgramPaths().push_back(getDriver().Dir);
@@ -125,7 +152,14 @@ HIPSPVToolChain::HIPSPVToolChain(const Driver &D, const llvm::Triple &Triple,
125152
void HIPSPVToolChain::addClangTargetOptions(
126153
const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args,
127154
Action::OffloadKind DeviceOffloadingKind) const {
128-
HostTC.addClangTargetOptions(DriverArgs, CC1Args, DeviceOffloadingKind);
155+
156+
if (!HostTC) {
157+
assert(DeviceOffloadingKind == Action::OFK_None &&
158+
"Need host toolchain for offloading!");
159+
return;
160+
}
161+
162+
HostTC->addClangTargetOptions(DriverArgs, CC1Args, DeviceOffloadingKind);
129163

130164
assert(DeviceOffloadingKind == Action::OFK_HIP &&
131165
"Only HIP offloading kinds are supported for GPUs.");
@@ -156,27 +190,37 @@ Tool *HIPSPVToolChain::buildLinker() const {
156190
}
157191

158192
void HIPSPVToolChain::addClangWarningOptions(ArgStringList &CC1Args) const {
159-
HostTC.addClangWarningOptions(CC1Args);
193+
if (HostTC)
194+
HostTC->addClangWarningOptions(CC1Args);
195+
ToolChain::addClangWarningOptions(CC1Args);
160196
}
161197

162198
ToolChain::CXXStdlibType
163199
HIPSPVToolChain::GetCXXStdlibType(const ArgList &Args) const {
164-
return HostTC.GetCXXStdlibType(Args);
200+
if (HostTC)
201+
return HostTC->GetCXXStdlibType(Args);
202+
return ToolChain::GetCXXStdlibType(Args);
165203
}
166204

167205
void HIPSPVToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
168206
ArgStringList &CC1Args) const {
169-
HostTC.AddClangSystemIncludeArgs(DriverArgs, CC1Args);
207+
if (HostTC)
208+
HostTC->AddClangSystemIncludeArgs(DriverArgs, CC1Args);
209+
ToolChain::AddClangSystemIncludeArgs(DriverArgs, CC1Args);
170210
}
171211

172212
void HIPSPVToolChain::AddClangCXXStdlibIncludeArgs(
173213
const ArgList &Args, ArgStringList &CC1Args) const {
174-
HostTC.AddClangCXXStdlibIncludeArgs(Args, CC1Args);
214+
if (HostTC)
215+
HostTC->AddClangCXXStdlibIncludeArgs(Args, CC1Args);
216+
ToolChain::AddClangCXXStdlibIncludeArgs(Args, CC1Args);
175217
}
176218

177219
void HIPSPVToolChain::AddIAMCUIncludeArgs(const ArgList &Args,
178220
ArgStringList &CC1Args) const {
179-
HostTC.AddIAMCUIncludeArgs(Args, CC1Args);
221+
if (HostTC)
222+
HostTC->AddIAMCUIncludeArgs(Args, CC1Args);
223+
ToolChain::AddIAMCUIncludeArgs(Args, CC1Args);
180224
}
181225

182226
void HIPSPVToolChain::AddHIPIncludeArgs(const ArgList &DriverArgs,
@@ -270,12 +314,16 @@ SanitizerMask HIPSPVToolChain::getSupportedSanitizers() const {
270314
// This behavior is necessary because the host and device toolchains
271315
// invocations often share the command line, so the device toolchain must
272316
// tolerate flags meant only for the host toolchain.
273-
return HostTC.getSupportedSanitizers();
317+
if (HostTC)
318+
return HostTC->getSupportedSanitizers();
319+
return ToolChain::getSupportedSanitizers();
274320
}
275321

276322
VersionTuple HIPSPVToolChain::computeMSVCVersion(const Driver *D,
277323
const ArgList &Args) const {
278-
return HostTC.computeMSVCVersion(D, Args);
324+
if (HostTC)
325+
return HostTC->computeMSVCVersion(D, Args);
326+
return ToolChain::computeMSVCVersion(D, Args);
279327
}
280328

281329
void HIPSPVToolChain::adjustDebugInfoKind(

‎clang/lib/Driver/ToolChains/HIPSPV.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,12 @@ class LLVM_LIBRARY_VISIBILITY HIPSPVToolChain final : public ToolChain {
4747
public:
4848
HIPSPVToolChain(const Driver &D, const llvm::Triple &Triple,
4949
const ToolChain &HostTC, const llvm::opt::ArgList &Args);
50+
HIPSPVToolChain(const Driver &D, const llvm::Triple &Triple,
51+
const llvm::opt::ArgList &Args);
5052

5153
const llvm::Triple *getAuxTriple() const override {
52-
return &HostTC.getTriple();
54+
assert(HostTC);
55+
return &HostTC->getTriple();
5356
}
5457

5558
void
@@ -90,7 +93,7 @@ class LLVM_LIBRARY_VISIBILITY HIPSPVToolChain final : public ToolChain {
9093
bool isPICDefaultForced() const override { return false; }
9194
bool SupportsProfiling() const override { return false; }
9295

93-
const ToolChain &HostTC;
96+
const ToolChain *HostTC = nullptr;
9497

9598
protected:
9699
Tool *buildLinker() const override;

‎clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64-unknown-chipstar.bc‎

Whitespace-only changes.

‎clang/test/Driver/hipspv-device-libs.hip‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
// Test --hip-device-lib-path
88
// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
9-
// RUN: --hip-path=%S/Inputs/hipspv \
9+
// RUN: --hip-path=%S/Inputs/hipspv \
1010
// RUN: --hip-device-lib-path=%S/Inputs/hipspv-dev-lib %s \
1111
// RUN: 2>&1 | FileCheck --check-prefixes=ALL,HIP-DEV-LIB-PATH %s
1212

‎clang/test/Driver/hipspv-link-static-library.hip‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,57 @@
11
// Test HIPSPV static device library linking
22
// REQUIRES: system-linux
3+
// REQUIRES: x86-registered-target
4+
// REQUIRES: spirv-registered-target
35
// UNSUPPORTED: system-windows
46

57
// Create a dummy archive to test SDL linking
68
// RUN: rm -rf %t && mkdir %t
7-
// RUN: touch %t/dummy.bc
9+
// RUN: touch %t/dummy.bc
810
// RUN: llvm-ar cr %t/libSDL.a %t/dummy.bc
911

1012
// Test that -l options are passed to llvm-link for --offload=spirv64
1113
// RUN: %clang -### --target=x86_64-linux-gnu --offload=spirv64 \
1214
// RUN: --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
13-
// RUN: -L%t -lSDL \
14-
// RUN: 2>&1 | FileCheck -check-prefix=SDL-LINK %s
15+
// RUN: --no-offload-new-driver -L%t -lSDL \
16+
// RUN: 2>&1 | FileCheck -check-prefixes=SDL %s
1517

16-
// Test that .a files are properly unbundled and passed to llvm-link
18+
// Test that .a files are properly unbundled and passed to llvm-link
1719
// RUN: %clang -### --target=x86_64-linux-gnu --offload=spirv64 \
1820
// RUN: --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
19-
// RUN: %t/libSDL.a \
20-
// RUN: 2>&1 | FileCheck -check-prefix=SDL-ARCHIVE %s
21+
// RUN: --no-offload-new-driver %t/libSDL.a \
22+
// RUN: 2>&1 | FileCheck -check-prefixes=SDL %s
23+
24+
// RUN: %clang -cc1 %s -triple spirv64-unknown-chipstar -emit-llvm-bc -o %t/dev.bc
25+
// RUN: llvm-offload-binary -o %t/dev.out \
26+
// RUN: --image=file=%t/dev.bc,kind=hip,triple=spirv64-unknown-chipstar,arch=generic
27+
// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t/tu0.o \
28+
// RUN: -fembed-offload-object=%t/dev.out
29+
// RUN: cp %t/tu0.o %t/tu1.o
30+
// RUN: llvm-ar cr %t/libSDL2.a %t/tu1.o
31+
32+
// RUN: %clang -### --target=x86_64-linux-gnu --offload-new-driver -fgpu-rdc \
33+
// RUN: -Xoffload-compiler-spirv64-unknown-chipstar \
34+
// RUN: --hip-path=%S/Inputs/hipspv -no-hip-rt %t/tu0.o %t/libSDL2.a \
35+
// RUN: 2>&1 | FileCheck -check-prefixes=SDL-NEW %s -DHIP_PATH=%S/Inputs/hipspv
36+
37+
// RUN: clang-linker-wrapper --dry-run --host-triple=x86_64-unknown-linux-gnu \
38+
// RUN: --device-compiler=spirv64-unknown-chipstar=--hip-path=%S/Inputs/hipspv \
39+
// RUN: --linker-path=/usr/bin/ld -o a.out %t/tu0.o %t/libSDL2.a \
40+
// RUN: 2>&1 | FileCheck -check-prefixes=SDL-NEW-WRAPPER %s -DHIP_PATH=%S/Inputs/hipspv
2141

2242
// Verify that the input files are added before the SDL files in llvm-link command
2343
// This tests the ordering fix to match HIPAMD behavior
24-
// SDL-LINK: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libSDL.a" "-targets=hip-spirv64-unknown-unknown-unknown-generic" "-output=[[SDL_A:.*\.a]]" "-allow-missing-bundles"
25-
// SDL-LINK: "{{.*}}llvm-link" "-o" "{{.*}}.bc" "{{.*}}.bc" "[[SDL_A]]"
44+
// SDL: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libSDL.a" "-targets=hip-spirv64-unknown-unknown-unknown-generic" "-output=[[SDL_A:.*\.a]]" "-allow-missing-bundles"
45+
// SDL: "{{.*}}llvm-link" "-o" "{{.*}}.bc" "{{.*}}.bc" "[[SDL_A]]"
46+
// SDL-NEW: "{{.*}}clang-linker-wrapper"
47+
// SDL-NEW-SAME: "--device-compiler=spirv64-unknown-chipstar=--hip-path=[[HIP_PATH]]"
48+
// SDL-NEW-SAME: "{{.*}}/tu0.o" "{{.*}}/libSDL2.a"
49+
// DELETE-SDL-NEW: "{{.*}}llvm-link" "-o" "{{.*}}.bc" "{{.*}}.o" "{{.*}}.o"
50+
51+
// SDL-NEW-WRAPPER: clang{{.*}}" --no-default-config -o {{[^ ]*.img}}
52+
// SDL-NEW-WRAPPER-SAME: {{[^ ]*.o}} {{[^ ]*.o}}
53+
// SDL-NEW-WRAPPER-SAME: --hip-path=[[HIP_PATH]]
2654

27-
// SDL-ARCHIVE: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libSDL.a" "-targets=hip-spirv64-unknown-unknown-unknown-generic" "-output=[[SDL_A:.*\.a]]" "-allow-missing-bundles"
28-
// SDL-ARCHIVE: "{{.*}}llvm-link" "-o" "{{.*}}.bc" "{{.*}}.bc" "[[SDL_A]]"
55+
// SDL: "{{.*}}opt"
56+
// SDL-SAME: "-load-pass-plugin" {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
57+
// SDL-SAME: "-passes=hip-post-link-passes"

0 commit comments

Comments
 (0)