Skip to content
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

[SYCL][Driver][HIP] Do not define __CUDA_ARCH__ for HIP-AMD targets #15443

Merged
merged 13 commits into from
Oct 17, 2024
Merged

[SYCL][Driver][HIP] Do not define __CUDA_ARCH__ for HIP-AMD targets #15443

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
daa81eb
[SYCL][Driver][HIP] Do not define __CUDA_ARCH__ for HIP-AMD targets
GeorgeWeb Sep 19, 2024
7062717
Add clang Driver test
GeorgeWeb Oct 2, 2024
d421f18
Merge remote-tracking branch 'upstream/sycl' into georgi/hip-undef-cu…
GeorgeWeb Oct 3, 2024
c279604
Split NVPTX and AMDGPU filecheck tests
GeorgeWeb Oct 3, 2024
9f8f5e5
Do not add fcuda-is-device to CC1 when compiling for HIPAMD
GeorgeWeb Oct 3, 2024
34a8d9a
Combine the fcuda-is-device test for both triples
GeorgeWeb Oct 3, 2024
bf72790
Restrict fcuda-is-device to NVPTX targets and add a comment
GeorgeWeb Oct 4, 2024
6220620
cleanup
GeorgeWeb Oct 4, 2024
5808cf3
Move the check for SYCL restricting adding CUDA_ARCH macro to the pre…
GeorgeWeb Oct 4, 2024
ce47bfc
Remove the AMDGPU test for fcuda-is-device
GeorgeWeb Oct 4, 2024
917f3ae
Simplify the CHECK-HIP-NOT sycl-macro test check line
GeorgeWeb Oct 8, 2024
e27ee12
Merge remote-tracking branch 'upstream/sycl' into georgi/hip-undef-cu…
GeorgeWeb Oct 8, 2024
7d7b808
Merge remote-tracking branch 'upstream/sycl' into georgi/hip-undef-cu…
GeorgeWeb Oct 10, 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
1 change: 0 additions & 1 deletion clang/lib/Basic/Targets/AMDGPU.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,6 @@ void AMDGPUTargetInfo::getTargetDefines(const LangOptions &Opts,
Twine("\"") + Twine(CanonName) + Twine("\""));
Builder.defineMacro("__amdgcn_target_id__",
Twine("\"") + Twine(*getTargetID()) + Twine("\""));
Builder.defineMacro("__CUDA_ARCH__", "0");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE:
This was previously added to avoid setting it to a non-zero value but is not needed anymore after the changes in this PR completely disable the definition of the macro for SYCL offload. Additionally, the upstream llvm-project doesn't have this line https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Targets/AMDGPU.cpp#L311.

for (auto F : getAllPossibleTargetIDFeatures(getTriple(), CanonName)) {
auto Loc = OffloadArchFeatures.find(F);
if (Loc != OffloadArchFeatures.end()) {
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Frontend/InitPreprocessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1491,9 +1491,10 @@ static void InitializePredefinedMacros(const TargetInfo &TI,
}

// CUDA device path compilaton
if (LangOpts.CUDAIsDevice && !LangOpts.HIP) {
if (LangOpts.CUDAIsDevice && !LangOpts.HIP && !LangOpts.isSYCL()) {
// The CUDA_ARCH value is set for the GPU target specified in the NVPTX
// backend's target defines.
// Note: SYCL targeting nvptx-cuda relies on __SYCL_CUDA_ARCH__ instead.
Builder.defineMacro("__CUDA_ARCH__");
}

Expand Down
4 changes: 4 additions & 0 deletions clang/test/Driver/sycl-cuda-arch-macro.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
// Verify the __CUDA_ARCH__ macro has not been defined when offloading SYCL on NVPTX
// RUN: %clangxx -E -dM -fsycl -fsycl-targets=nvptx64-nvidia-cuda -Xsycl-target-backend --offload-arch=sm_80 -nocudalib -fno-sycl-libspirv %s 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CUDA-ARCH-MACRO %s
// Verify the __CUDA_ARCH__ macro has not been defined when offloading SYCL on AMDGPU
// RUN: %clangxx -E -dM -fsycl -fsycl-targets=amdgcn-amd-amdhsa -Xsycl-target-backend --offload-arch=gfx90a -nogpulib -fno-sycl-libspirv %s 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CUDA-ARCH-MACRO %s
// CHECK-CUDA-ARCH-MACRO-NOT: #define __CUDA_ARCH__ {{[0-9]+}}

// Verify that '-fcuda-is-device' is not supplied when offloading SYCL on NVPTX
// NOTE: AMDGPU targets, i.e. "amdgcn-amd-amdhsa" may rely on "fcuda-is-device"
// RUN: %clangxx -### -fsycl -fsycl-targets=nvptx64-nvidia-cuda -Xsycl-target-backend --offload-arch=sm_80 -nocudalib -fno-sycl-libspirv %s 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CUDA-IS-DEVICE %s
// CHECK-CUDA-IS-DEVICE: clang{{.*}} "-cc1" "-triple" "nvptx64-nvidia-cuda"
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Preprocessor/sycl-macro.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@
// CHECK-CUDA:#define __SYCL_CUDA_ARCH__ [[ARCH_CODE]]
// CHECK-CUDA-NOT:#define __CUDA_ARCH__ {{[0-9]+}}

// CHECK-HIP:#define __CUDA_ARCH__ 0
// CHECK-HIP-NOT:#define __CUDA_ARCH__ [[ARCH:[0-9]+]]

// CHECK-CUDA-FEATURE:#define __CUDA_ARCH_FEAT_SM90_ALL 1
Loading