Skip to content

[SYCL][NVPTX][NFC] Refactor NVPTX target configuration #3535

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 4 commits into from
Apr 15, 2021
Merged

[SYCL][NVPTX][NFC] Refactor NVPTX target configuration #3535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
90a95a4
[SYCL][NVPTX] Refactor NVPTX target configuration
bader Apr 12, 2021
5a89dd2
Skip modules w/o nvvm.annotations metadata in LocalAccessorToSharedMe…
bader Apr 12, 2021
d71c4b8
Disable GlobalOffset offset pass for non-CUDA OS.
bader Apr 12, 2021
bc0a82d
Added TODOs for issues to investigate.
bader Apr 13, 2021
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
3 changes: 1 addition & 2 deletions clang/lib/Basic/Targets/NVPTX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ NVPTXTargetInfo::NVPTXTargetInfo(const llvm::Triple &Triple,
.Default(32);
}

// FIXME: Needed for compiling SYCL to PTX.
TLSSupported = Triple.getEnvironment() == llvm::Triple::SYCLDevice;
TLSSupported = false;
VLASupported = false;
AddrSpaceMap = &NVPTXAddrSpaceMap;
GridValues = llvm::omp::NVPTXGpuGridValues;
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/Basic/Targets/NVPTX.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ class LLVM_LIBRARY_VISIBILITY NVPTXTargetInfo : public TargetInfo {
return CCCR_Warning;
}

void adjust(LangOptions &Opts) override {
TargetInfo::adjust(Opts);
// FIXME: Needed for compiling SYCL to PTX.
TLSSupported = TLSSupported || Opts.SYCLIsDevice;
}

bool hasExtIntType() const override { return true; }
};
} // namespace targets
Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,9 @@ void NVPTXPassConfig::addIRPasses() {
const NVPTXSubtarget &ST = *getTM<NVPTXTargetMachine>().getSubtargetImpl();
addPass(createNVVMReflectPass(ST.getSmVersion()));

if (getTM<NVPTXTargetMachine>().getTargetTriple().getOS() == Triple::CUDA &&
getTM<NVPTXTargetMachine>().getTargetTriple().getEnvironment() == Triple::SYCLDevice) {
// FIXME: should the target triple check be done by the pass itself?
// See createNVPTXLowerArgsPass as an example
if (getTM<NVPTXTargetMachine>().getTargetTriple().getOS() == Triple::CUDA) {
addPass(createGlobalOffsetPass());
addPass(createLocalAccessorToSharedMemoryPass());
}
Expand Down
7 changes: 5 additions & 2 deletions llvm/lib/Target/NVPTX/SYCL/LocalAccessorToSharedMemory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ class LocalAccessorToSharedMemory : public ModulePass {

bool runOnModule(Module &M) override {
// Invariant: This pass is only intended to operate on SYCL kernels being
// compiled to the `nvptx{,64}-nvidia-cuda-sycldevice` triple.
// compiled to the `nvptx{,64}-nvidia-cuda` triple.
// TODO: make sure that non-SYCL kernels are not impacted.
if (skipModule(M))
return false;

Expand All @@ -48,7 +49,9 @@ class LocalAccessorToSharedMemory : public ModulePass {
// Access `nvvm.annotations` to determine which functions are kernel entry
// points.
auto NvvmMetadata = M.getNamedMetadata("nvvm.annotations");
assert(NvvmMetadata && "IR compiled to PTX must have nvvm.annotations");
if (!NvvmMetadata)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a related change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. We skipped this pass for modules w/o -sycldevice triple component in llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp and now this pass is executed regardless of the environment component value. SYCL compiler always added nvvm.annotations metadata, but it might be missing. In case when this metadata is not emitted, we just skip the transformation instead of crashing (with enabled assertions).

return false;

for (auto MetadataNode : NvvmMetadata->operands()) {
if (MetadataNode->getNumOperands() != 3)
continue;
Expand Down