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.

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] Move driver related __CUDA_ARCH__ test to Driver folder from Preprocessor #15521

Merged
merged 2 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions clang/test/Driver/sycl-cuda-arch-macro.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Verify the __CUDA_ARCH__ macro has not been defined when offloading SYCL on NVPTX
Copy link
Contributor

Choose a reason for hiding this comment

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

Are AMDGPU targets covered by other tests? FYI: #15544

Copy link
Contributor Author

@GeorgeWeb GeorgeWeb Sep 30, 2024

Choose a reason for hiding this comment

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

Thanks for catching this. I had noticed the macro being defined in the HIP toolchain at the time of fixing the regression for the Cuda flow. It isn't tested correctly.

I made a draft PR (here #15443) for HIP-AMDGPU right after the Cuda one and I was planning to make it ready for review after this gets merged. Seems others caught it too in the meantime. I'll add a sycl clang driver for AMDGPU targets here as part of the said PR.

// 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
// CHECK-CUDA-ARCH-MACRO-NOT: #define __CUDA_ARCH__ {{[0-9]+}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the original nature of the driver change, one thing we should also verify is that -fcuda-is-device is not supplied with -fsycl -fsycl-targets=nvptx64-nvidia-cuda for the device compilation (-### check)

Copy link
Contributor Author

@GeorgeWeb GeorgeWeb Sep 27, 2024

Choose a reason for hiding this comment

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

@mdtoguchi Thanks for being rigorous here! That's definitely what the original change prevented, where the unwanted macro was a product of this. I've added the test and updated the PR description accordingly.
Let me know if you think something needs to be updated.


// Verify that '-fcuda-is-device' is not supplied when offloading SYCL on NVPTX
// 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"
// CHECK-CUDA-IS-DEVICE-NOT: "-fcuda-is-device"
// CHECK-CUDA-IS-DEVICE-SAME: "-fsycl-is-device"
4 changes: 0 additions & 4 deletions clang/test/Preprocessor/sycl-macro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
// RUN: --check-prefix=CHECK-NO-SYCL_FIT_IN_INT %s
// RUN: %clang_cc1 %s -triple nvptx64-nvidia-cuda -target-cpu sm_80 -fsycl-is-device -E -dM | FileCheck \
// RUN: --check-prefix=CHECK-CUDA %s -DARCH_CODE=800
// RUN: %clangxx %s -fsycl -nocudalib -fsycl-targets=nvptx64-nvidia-cuda -Xsycl-target-backend --offload-arch=sm_80 -E -dM | FileCheck \
// RUN: --check-prefix=CHECK-CUDA-SYCL-DRIVER %s
// RUN: %clang_cc1 %s -triple amdgcn-amd-amdhsa -target-cpu gfx906 -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-HIP %s

// RUN: %clang_cc1 %s -triple nvptx64-nvidia-cuda -target-cpu sm_90a -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-CUDA-FEATURE %s
Expand Down Expand Up @@ -38,8 +36,6 @@
// CHECK-CUDA:#define __SYCL_CUDA_ARCH__ [[ARCH_CODE]]
// CHECK-CUDA-NOT:#define __CUDA_ARCH__ {{[0-9]+}}

// CHECK-CUDA-SYCL-DRIVER-NOT: #define __CUDA_ARCH__ {{[0-9]+}}

// CHECK-HIP:#define __CUDA_ARCH__ 0

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