Skip to content

[clang][SPIR-V] Add support for AMDGCN flavoured SPIRV #89796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 37 commits into from
Jun 7, 2024
Merged

[clang][SPIR-V] Add support for AMDGCN flavoured SPIRV #89796

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
662f160
Add initial support for AMDGCN flavoured SPIRV.
AlexVlx Apr 23, 2024
393ce66
Fix formatting.
AlexVlx Apr 23, 2024
2a10ad0
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx Apr 25, 2024
98db8f7
Use `fillAMDGPUFeatureMap` instead of copy-pasta.
AlexVlx Apr 25, 2024
c359e0a
Add `__has_builtin` test.
AlexVlx Apr 25, 2024
e98f3f5
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx Apr 28, 2024
c41726d
Re-use `AMDGPUTargetInfo`, where feasible, instead of copypasta-ing.
AlexVlx Apr 28, 2024
4698b58
Incorporate review suggestions.
AlexVlx Apr 28, 2024
aa1cd7c
Fix header ordering.
AlexVlx Apr 28, 2024
f9729ef
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 8, 2024
8257cb1
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 9, 2024
900cd69
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 11, 2024
3307f17
Handle `wavefrontsize` (we need both 32 and 64); add more tests.
AlexVlx May 12, 2024
eee6063
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 13, 2024
d2f4244
Add an additional test.
AlexVlx May 14, 2024
4cb4026
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 14, 2024
84a621d
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 14, 2024
1841385
AMDGCN SPIRV should allow both AMDGCN and SPIRV builtins.
AlexVlx May 15, 2024
0ce2da3
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 15, 2024
120b73c
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 15, 2024
f3942bd
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 15, 2024
31ac77d
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 15, 2024
0e9b1a1
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 16, 2024
83cd5e0
Enable AMDGCN flavoured SPIRV in the experimental SPIRV BE.
AlexVlx May 16, 2024
e9158b0
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 16, 2024
05074e7
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 19, 2024
e1fb93f
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 20, 2024
36c4bf6
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 27, 2024
5ffa186
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 27, 2024
cf1880c
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx May 29, 2024
4d85a1b
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx Jun 5, 2024
516e14c
Revert spurios testing noise, AMDGCN SPIRV is still SPIRV.
AlexVlx Jun 5, 2024
bdc3eb5
First pass at updating SPIR-V docs to reflect the addition of AMDGCN …
AlexVlx Jun 6, 2024
361d47b
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx Jun 6, 2024
b088c72
Fix erroneous versioning claim.
AlexVlx Jun 6, 2024
e85b557
Remove function pointer tests.
AlexVlx Jun 6, 2024
1d41787
Merge branch 'main' of https://github.com/llvm/llvm-project into amdg…
AlexVlx Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion clang/lib/Basic/Targets.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,8 +673,12 @@ std::unique_ptr<TargetInfo> AllocateTarget(const llvm::Triple &Triple,
}
case llvm::Triple::spirv64: {
if (os != llvm::Triple::UnknownOS ||
Triple.getEnvironment() != llvm::Triple::UnknownEnvironment)
Triple.getEnvironment() != llvm::Triple::UnknownEnvironment) {
if (os == llvm::Triple::OSType::AMDHSA)
return std::make_unique<SPIRV64AMDGCNTargetInfo>(Triple, Opts);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

return nullptr;
}
return std::make_unique<SPIRV64TargetInfo>(Triple, Opts);
}
case llvm::Triple::wasm32:
Expand Down
288 changes: 288 additions & 0 deletions clang/lib/Basic/Targets/SPIR.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

#include "SPIR.h"
#include "Targets.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/TargetBuiltins.h"

using namespace clang;
using namespace clang::targets;
Expand Down Expand Up @@ -54,3 +56,289 @@ void SPIRV64TargetInfo::getTargetDefines(const LangOptions &Opts,
BaseSPIRVTargetInfo::getTargetDefines(Opts, Builder);
DefineStd(Builder, "SPIRV64", Opts);
}

static constexpr Builtin::Info BuiltinInfo[] = {
#define BUILTIN(ID, TYPE, ATTRS) \
{#ID, TYPE, ATTRS, nullptr, HeaderDesc::NO_HEADER, ALL_LANGUAGES},
#define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE) \
{#ID, TYPE, ATTRS, FEATURE, HeaderDesc::NO_HEADER, ALL_LANGUAGES},
#include "clang/Basic/BuiltinsAMDGPU.def"
};

namespace {
const char *AMDGPUGCCRegNames[] = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could create a common base class for amdgpu target info and spirv64 amdgcn target info and not duplicate stuff. we can define a static member for reg names.

"v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8",
"v9", "v10", "v11", "v12", "v13", "v14", "v15", "v16", "v17",
"v18", "v19", "v20", "v21", "v22", "v23", "v24", "v25", "v26",
"v27", "v28", "v29", "v30", "v31", "v32", "v33", "v34", "v35",
"v36", "v37", "v38", "v39", "v40", "v41", "v42", "v43", "v44",
"v45", "v46", "v47", "v48", "v49", "v50", "v51", "v52", "v53",
"v54", "v55", "v56", "v57", "v58", "v59", "v60", "v61", "v62",
"v63", "v64", "v65", "v66", "v67", "v68", "v69", "v70", "v71",
"v72", "v73", "v74", "v75", "v76", "v77", "v78", "v79", "v80",
"v81", "v82", "v83", "v84", "v85", "v86", "v87", "v88", "v89",
"v90", "v91", "v92", "v93", "v94", "v95", "v96", "v97", "v98",
"v99", "v100", "v101", "v102", "v103", "v104", "v105", "v106", "v107",
"v108", "v109", "v110", "v111", "v112", "v113", "v114", "v115", "v116",
"v117", "v118", "v119", "v120", "v121", "v122", "v123", "v124", "v125",
"v126", "v127", "v128", "v129", "v130", "v131", "v132", "v133", "v134",
"v135", "v136", "v137", "v138", "v139", "v140", "v141", "v142", "v143",
"v144", "v145", "v146", "v147", "v148", "v149", "v150", "v151", "v152",
"v153", "v154", "v155", "v156", "v157", "v158", "v159", "v160", "v161",
"v162", "v163", "v164", "v165", "v166", "v167", "v168", "v169", "v170",
"v171", "v172", "v173", "v174", "v175", "v176", "v177", "v178", "v179",
"v180", "v181", "v182", "v183", "v184", "v185", "v186", "v187", "v188",
"v189", "v190", "v191", "v192", "v193", "v194", "v195", "v196", "v197",
"v198", "v199", "v200", "v201", "v202", "v203", "v204", "v205", "v206",
"v207", "v208", "v209", "v210", "v211", "v212", "v213", "v214", "v215",
"v216", "v217", "v218", "v219", "v220", "v221", "v222", "v223", "v224",
"v225", "v226", "v227", "v228", "v229", "v230", "v231", "v232", "v233",
"v234", "v235", "v236", "v237", "v238", "v239", "v240", "v241", "v242",
"v243", "v244", "v245", "v246", "v247", "v248", "v249", "v250", "v251",
"v252", "v253", "v254", "v255", "s0", "s1", "s2", "s3", "s4",
"s5", "s6", "s7", "s8", "s9", "s10", "s11", "s12", "s13",
"s14", "s15", "s16", "s17", "s18", "s19", "s20", "s21", "s22",
"s23", "s24", "s25", "s26", "s27", "s28", "s29", "s30", "s31",
"s32", "s33", "s34", "s35", "s36", "s37", "s38", "s39", "s40",
"s41", "s42", "s43", "s44", "s45", "s46", "s47", "s48", "s49",
"s50", "s51", "s52", "s53", "s54", "s55", "s56", "s57", "s58",
"s59", "s60", "s61", "s62", "s63", "s64", "s65", "s66", "s67",
"s68", "s69", "s70", "s71", "s72", "s73", "s74", "s75", "s76",
"s77", "s78", "s79", "s80", "s81", "s82", "s83", "s84", "s85",
"s86", "s87", "s88", "s89", "s90", "s91", "s92", "s93", "s94",
"s95", "s96", "s97", "s98", "s99", "s100", "s101", "s102", "s103",
"s104", "s105", "s106", "s107", "s108", "s109", "s110", "s111", "s112",
"s113", "s114", "s115", "s116", "s117", "s118", "s119", "s120", "s121",
"s122", "s123", "s124", "s125", "s126", "s127", "exec", "vcc", "scc",
"m0", "flat_scratch", "exec_lo", "exec_hi", "vcc_lo", "vcc_hi",
"flat_scratch_lo", "flat_scratch_hi",
"a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7", "a8",
"a9", "a10", "a11", "a12", "a13", "a14", "a15", "a16", "a17",
"a18", "a19", "a20", "a21", "a22", "a23", "a24", "a25", "a26",
"a27", "a28", "a29", "a30", "a31", "a32", "a33", "a34", "a35",
"a36", "a37", "a38", "a39", "a40", "a41", "a42", "a43", "a44",
"a45", "a46", "a47", "a48", "a49", "a50", "a51", "a52", "a53",
"a54", "a55", "a56", "a57", "a58", "a59", "a60", "a61", "a62",
"a63", "a64", "a65", "a66", "a67", "a68", "a69", "a70", "a71",
"a72", "a73", "a74", "a75", "a76", "a77", "a78", "a79", "a80",
"a81", "a82", "a83", "a84", "a85", "a86", "a87", "a88", "a89",
"a90", "a91", "a92", "a93", "a94", "a95", "a96", "a97", "a98",
"a99", "a100", "a101", "a102", "a103", "a104", "a105", "a106", "a107",
"a108", "a109", "a110", "a111", "a112", "a113", "a114", "a115", "a116",
"a117", "a118", "a119", "a120", "a121", "a122", "a123", "a124", "a125",
"a126", "a127", "a128", "a129", "a130", "a131", "a132", "a133", "a134",
"a135", "a136", "a137", "a138", "a139", "a140", "a141", "a142", "a143",
"a144", "a145", "a146", "a147", "a148", "a149", "a150", "a151", "a152",
"a153", "a154", "a155", "a156", "a157", "a158", "a159", "a160", "a161",
"a162", "a163", "a164", "a165", "a166", "a167", "a168", "a169", "a170",
"a171", "a172", "a173", "a174", "a175", "a176", "a177", "a178", "a179",
"a180", "a181", "a182", "a183", "a184", "a185", "a186", "a187", "a188",
"a189", "a190", "a191", "a192", "a193", "a194", "a195", "a196", "a197",
"a198", "a199", "a200", "a201", "a202", "a203", "a204", "a205", "a206",
"a207", "a208", "a209", "a210", "a211", "a212", "a213", "a214", "a215",
"a216", "a217", "a218", "a219", "a220", "a221", "a222", "a223", "a224",
"a225", "a226", "a227", "a228", "a229", "a230", "a231", "a232", "a233",
"a234", "a235", "a236", "a237", "a238", "a239", "a240", "a241", "a242",
"a243", "a244", "a245", "a246", "a247", "a248", "a249", "a250", "a251",
"a252", "a253", "a254", "a255"
};

} // anonymous namespace

ArrayRef<const char *> SPIRV64AMDGCNTargetInfo::getGCCRegNames() const {
return llvm::ArrayRef(AMDGPUGCCRegNames);
}

bool SPIRV64AMDGCNTargetInfo::initFeatureMap(
llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags, StringRef,
const std::vector<std::string> &FeatureVec) const {
// This represents the union of all AMDGCN features.
Features["atomic-ds-pk-add-16-insts"] = true;
Features["atomic-flat-pk-add-16-insts"] = true;
Features["atomic-buffer-global-pk-add-f16-insts"] = true;
Features["atomic-global-pk-add-bf16-inst"] = true;
Features["atomic-fadd-rtn-insts"] = true;
Features["ci-insts"] = true;
Features["dot1-insts"] = true;
Features["dot2-insts"] = true;
Features["dot3-insts"] = true;
Features["dot4-insts"] = true;
Features["dot5-insts"] = true;
Features["dot7-insts"] = true;
Features["dot8-insts"] = true;
Features["dot9-insts"] = true;
Features["dot10-insts"] = true;
Features["dot11-insts"] = true;
Features["dl-insts"] = true;
Features["16-bit-insts"] = true;
Features["dpp"] = true;
Features["gfx8-insts"] = true;
Features["gfx9-insts"] = true;
Features["gfx90a-insts"] = true;
Features["gfx940-insts"] = true;
Features["gfx10-insts"] = true;
Features["gfx10-3-insts"] = true;
Features["gfx11-insts"] = true;
Features["gfx12-insts"] = true;
Features["image-insts"] = true;
Features["fp8-conversion-insts"] = true;
Features["s-memrealtime"] = true;
Features["s-memtime-inst"] = true;
Features["gws"] = true;
Features["fp8-insts"] = true;
Features["fp8-conversion-insts"] = true;
Features["atomic-ds-pk-add-16-insts"] = true;
Features["mai-insts"] = true;

return TargetInfo::initFeatureMap(Features, Diags, {}, FeatureVec);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not want to maintain 2 copies of either of these things. Can we just move this to a common place? I thought we already moved the Features gunk into lib/TargetSupport

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a common place for the register names, the current design assumes they're a private static detail of a target - suggestions welcome though, I agree the duplication is pretty terrible. As for the features, if you're thinking about extending and then re-using AMDGPU::fillAMDGPUFeatureMap, I can do that, and it's probably a better solution I had not considered. Is that what you had in mind?


bool SPIRV64AMDGCNTargetInfo::validateAsmConstraint(
const char *&Name, TargetInfo::ConstraintInfo &Info) const {
// This is a 1:1 copy of AMDGPUTargetInfo::validateAsmConstraint()
static const ::llvm::StringSet<> SpecialRegs({
"exec", "vcc", "flat_scratch", "m0", "scc", "tba", "tma",
"flat_scratch_lo", "flat_scratch_hi", "vcc_lo", "vcc_hi", "exec_lo",
"exec_hi", "tma_lo", "tma_hi", "tba_lo", "tba_hi",
});

switch (*Name) {
case 'I':
Info.setRequiresImmediate(-16, 64);
return true;
case 'J':
Info.setRequiresImmediate(-32768, 32767);
return true;
case 'A':
case 'B':
case 'C':
Info.setRequiresImmediate();
return true;
default:
break;
}

StringRef S(Name);

if (S == "DA" || S == "DB") {
Name++;
Info.setRequiresImmediate();
return true;
}

bool HasLeftParen = S.consume_front("{");
if (S.empty())
return false;
if (S.front() != 'v' && S.front() != 's' && S.front() != 'a') {
if (!HasLeftParen)
return false;
auto E = S.find('}');
if (!SpecialRegs.count(S.substr(0, E)))
return false;
S = S.drop_front(E + 1);
if (!S.empty())
return false;
// Found {S} where S is a special register.
Info.setAllowsRegister();
Name = S.data() - 1;
return true;
}
S = S.drop_front();
if (!HasLeftParen) {
if (!S.empty())
return false;
// Found s, v or a.
Info.setAllowsRegister();
Name = S.data() - 1;
return true;
}
bool HasLeftBracket = S.consume_front("[");
unsigned long long N;
if (S.empty() || consumeUnsignedInteger(S, 10, N))
return false;
if (S.consume_front(":")) {
if (!HasLeftBracket)
return false;
unsigned long long M;
if (consumeUnsignedInteger(S, 10, M) || N >= M)
return false;
}
if (HasLeftBracket) {
if (!S.consume_front("]"))
return false;
}
if (!S.consume_front("}"))
return false;
if (!S.empty())
return false;
// Found {vn}, {sn}, {an}, {v[n]}, {s[n]}, {a[n]}, {v[n:m]}, {s[n:m]}
// or {a[n:m]}.
Info.setAllowsRegister();
Name = S.data() - 1;
return true;
}

std::string
SPIRV64AMDGCNTargetInfo::convertConstraint(const char *&Constraint) const {
// This is a 1:1 copy of AMDGPUTargetInfo::convertConstraint()
StringRef S(Constraint);
if (S == "DA" || S == "DB") {
return std::string("^") + std::string(Constraint++, 2);
}

const char *Begin = Constraint;
TargetInfo::ConstraintInfo Info("", "");
if (validateAsmConstraint(Constraint, Info))
return std::string(Begin).substr(0, Constraint - Begin + 1);

Constraint = Begin;
return std::string(1, *Constraint);
}

ArrayRef<Builtin::Info> SPIRV64AMDGCNTargetInfo::getTargetBuiltins() const {
return llvm::ArrayRef(BuiltinInfo,
clang::AMDGPU::LastTSBuiltin - Builtin::FirstTSBuiltin);
}

void SPIRV64AMDGCNTargetInfo::getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const {
BaseSPIRVTargetInfo::getTargetDefines(Opts, Builder);
DefineStd(Builder, "SPIRV64", Opts);

Builder.defineMacro("__AMD__");
Builder.defineMacro("__AMDGPU__");
Builder.defineMacro("__AMDGCN__");
Comment on lines +93 to +95
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these defined on both the host and device? I remember having a quite annoying time with these macros because HIP was defining stuff like __AMDGCN_WAVEFRONT_SIZE on the host.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__AMDGCN_WAVEFRONT_SIZE is a pretty terrible error, which should never have been defined on host (not in the least because it's non-uniform across targets). These do (should) end up defined on host too, but they are harmless because they are uniform i.e. all potential consumers of spirv64-amd-amdhsa are __AMD__, __AMDGPU__ and __AMDGCN__. They're mostly a necessary nuisance for getting library code to work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've always thought defining those for both targets was obtuse, since those should act like architecture macros, i.e. (X86). But considering this is how it's done already, I suppose it's a necessary evil.

}

void SPIRV64AMDGCNTargetInfo::setAuxTarget(const TargetInfo *Aux) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is AUX guaranteed non-null here? I know in the NVPTX target we only have an Aux when compiling for CUDA and use that to make sure type widths match up. However, if you have a direct compilation via --target=nvptx64-nvidia-cuda it will be null and not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, setAuxTarget is only used when compiling for CUDA / HIP, and wouldn't get invoked otherwise (I could be wrong though); having said that, it probably makes sense to assert that Aux is non-null, thanks for pointing this out.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a reasonable assumption. Since HIP/CUDA needs to make sure type consistency between host and device, normal compilation by clang driver always specify aux-target for device compilation.

// This is a 1:1 copy of AMDGPUTargetInfo::setAuxTarget()
assert(HalfFormat == Aux->HalfFormat);
assert(FloatFormat == Aux->FloatFormat);
assert(DoubleFormat == Aux->DoubleFormat);

// On x86_64 long double is 80-bit extended precision format, which is
// not supported by AMDGPU. 128-bit floating point format is also not
// supported by AMDGPU. Therefore keep its own format for these two types.
auto SaveLongDoubleFormat = LongDoubleFormat;
auto SaveFloat128Format = Float128Format;
auto SaveLongDoubleWidth = LongDoubleWidth;
auto SaveLongDoubleAlign = LongDoubleAlign;
copyAuxTarget(Aux);
LongDoubleFormat = SaveLongDoubleFormat;
Float128Format = SaveFloat128Format;
LongDoubleWidth = SaveLongDoubleWidth;
LongDoubleAlign = SaveLongDoubleAlign;
// For certain builtin types support on the host target, claim they are
// supported to pass the compilation of the host code during the device-side
// compilation.
// FIXME: As the side effect, we also accept `__float128` uses in the device
// code. To reject these builtin types supported in the host target but not in
// the device target, one approach would support `device_builtin` attribute
// so that we could tell the device builtin types from the host ones. This
// also solves the different representations of the same builtin type, such
// as `size_t` in the MSVC environment.
if (Aux->hasFloat128Type()) {
HasFloat128 = true;
Float128Format = DoubleFormat;
}
}
51 changes: 51 additions & 0 deletions clang/lib/Basic/Targets/SPIR.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,57 @@ class LLVM_LIBRARY_VISIBILITY SPIRV64TargetInfo : public BaseSPIRVTargetInfo {
MacroBuilder &Builder) const override;
};

class LLVM_LIBRARY_VISIBILITY SPIRV64AMDGCNTargetInfo
: public BaseSPIRVTargetInfo {
public:
SPIRV64AMDGCNTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
: BaseSPIRVTargetInfo(Triple, Opts) {
assert(Triple.getArch() == llvm::Triple::spirv64 &&
"Invalid architecture for 64-bit AMDGCN SPIR-V.");
assert(Triple.getVendor() == llvm::Triple::VendorType::AMD &&
"64-bit AMDGCN SPIR-V target must use AMD vendor");
assert(getTriple().getOS() == llvm::Triple::OSType::AMDHSA &&
"64-bit AMDGCN SPIR-V target must use AMDHSA OS");
assert(getTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
"64-bit SPIR-V target must use unknown environment type");
PointerWidth = PointerAlign = 64;
SizeType = TargetInfo::UnsignedLong;
PtrDiffType = IntPtrType = TargetInfo::SignedLong;

resetDataLayout("e-i64:64-v16:16-v24:32-v32:32-v48:64-"
"v96:128-v192:256-v256:256-v512:512-v1024:1024-G1-P4-A0");

BFloat16Width = BFloat16Align = 16;
BFloat16Format = &llvm::APFloat::BFloat();

HasLegalHalfType = true;
HasFloat16 = true;
HalfArgsAndReturns = true;
}

bool hasBFloat16Type() const override { return true; }

ArrayRef<const char *> getGCCRegNames() const override;

bool initFeatureMap(llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags,
StringRef,
const std::vector<std::string> &) const override;

bool validateAsmConstraint(const char *&Name,
TargetInfo::ConstraintInfo &Info) const override;

std::string convertConstraint(const char *&Constraint) const override;

ArrayRef<Builtin::Info> getTargetBuiltins() const override;

void getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const override;

void setAuxTarget(const TargetInfo *Aux) override;

bool hasInt128Type() const override { return TargetInfo::hasInt128Type(); }
};

} // namespace targets
} // namespace clang
#endif // LLVM_CLANG_LIB_BASIC_TARGETS_SPIR_H
7 changes: 7 additions & 0 deletions clang/lib/CodeGen/CGBuiltin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6083,6 +6083,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
StringRef Prefix =
llvm::Triple::getArchTypePrefix(getTarget().getTriple().getArch());
if (!Prefix.empty()) {
if (Prefix == "spv" &&
getTarget().getTriple().getOS() == llvm::Triple::OSType::AMDHSA)
Prefix = "amdgcn";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this required for? I'm wondering why we'd need to reset the prefix here instead of updating the logic somewhere else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite see any other point to tweak the logic. We have some amdgcn builtins that get handled here, as opposed to via EmitTargetBuiltinExpr, and this is the narrowest scope for adjusting Prefix; the information that this is coming from AMDGCN flavoured SPIRV is lost past this point. Did you have something in mind regarding where to handle this as an alternative?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I was just wondering if it would make more sense to put this in Triple::getArchTypePrefix(ArchType Kind) because I wasn't sure if this logic is the expected return value there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can, with the current signature of that interface / the current approach of keeping spirv64 as the prefix, since we need at least the OS type as well to disambiguate. So it'd require either overloading that interface, or using a different prefix (this'd ripple elsewhere).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to allow this choice to be target-specific, we can just make the "IntrinsicPrefix" a property of the TargetInfo, instead of trying to derive it directly from the target triple. The default constructor would derive it from the triple, but specific targets could override the field.

I'm more concerned this is going to prevent us from using builtins which shoud be usable; do we need to getIntrinsicForClangBuiltin() for both "spv" and "amdgcn"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second bit is actually a good catch, I had assumed there's no SPIRV builtins, however looking at it it seems a few exist, so it appears we'll need both. I'll rework this part, cheers!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed the "spv" exclusion. Circling back to the idea of moving the prefix into TargetInfo, that seems like a pretty nice improvement - I think we could also deal with scenarios like the one here where there's an union of target specific builtins (spv & amdgcn in this case) by having some sort of parent-child / main-aux relationship. It should probably be a separate PR since it's an independent piece of functionality (and I've not thought through it fully yet) - unless you disagree?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can look at it as a followup, sure; the code in its current form seems okay for now.

IntrinsicID = Intrinsic::getIntrinsicForClangBuiltin(Prefix.data(), Name);
// NOTE we don't need to perform a compatibility flag check here since the
// intrinsics are declared in Builtins*.def via LANGBUILTIN which filter the
Expand Down Expand Up @@ -6254,6 +6257,10 @@ static Value *EmitTargetArchBuiltinExpr(CodeGenFunction *CGF,
case llvm::Triple::riscv32:
case llvm::Triple::riscv64:
return CGF->EmitRISCVBuiltinExpr(BuiltinID, E, ReturnValue);
case llvm::Triple::spirv64:
if (CGF->getTarget().getTriple().getOS() != llvm::Triple::OSType::AMDHSA)
return nullptr;
return CGF->EmitAMDGPUBuiltinExpr(BuiltinID, E);
default:
return nullptr;
}
Expand Down
4 changes: 4 additions & 0 deletions clang/test/CodeGen/target-data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,7 @@
// RUN: %clang_cc1 -triple ve -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=VE
// VE: target datalayout = "e-m:e-i64:64-n32:64-S128-v64:64:64-v128:64:64-v256:64:64-v512:64:64-v1024:64:64-v2048:64:64-v4096:64:64-v8192:64:64-v16384:64:64"

// RUN: %clang_cc1 -triple spirv64-amd -o - -emit-llvm %s | \
// RUN: FileCheck %s -check-prefix=SPIR64
// AMDGPUSPIRV64: target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-G1-P4-A0"
Loading
Loading