Skip to content

Conversation

sarnex
Copy link
Member

@sarnex sarnex commented Apr 22, 2025

This fixes a CUDA SPIR-V regression introduced in #134399.

Copy link

github-actions bot commented Apr 22, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Signed-off-by: Sarnie, Nick <[email protected]>
@sarnex sarnex marked this pull request as ready for review April 23, 2025 14:04
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 23, 2025

@llvm/pr-subscribers-clang

Author: Nick Sarnie (sarnex)

Changes

This fixes a CUDA SPIR-V regression introduced in #134399.


Full diff: https://github.com/llvm/llvm-project/pull/136753.diff

2 Files Affected:

  • (modified) clang/lib/Basic/Targets/SPIR.h (+3-2)
  • (added) clang/test/CodeGenCUDASPIRV/printf.cu (+11)
diff --git a/clang/lib/Basic/Targets/SPIR.h b/clang/lib/Basic/Targets/SPIR.h
index 4509748589b76..310ef9f2df2c6 100644
--- a/clang/lib/Basic/Targets/SPIR.h
+++ b/clang/lib/Basic/Targets/SPIR.h
@@ -58,8 +58,9 @@ static const unsigned SPIRDefIsPrivMap[] = {
 // Used by both the SPIR and SPIR-V targets.
 static const unsigned SPIRDefIsGenMap[] = {
     4, // Default
-    // OpenCL address space values for this map are dummy and they can't be used
-    0, // opencl_global
+    // Some OpenCL address space values for this map are dummy and they can't be
+    // used
+    1, // opencl_global
     0, // opencl_local
     0, // opencl_constant
     0, // opencl_private
diff --git a/clang/test/CodeGenCUDASPIRV/printf.cu b/clang/test/CodeGenCUDASPIRV/printf.cu
new file mode 100644
index 0000000000000..936e920f4a755
--- /dev/null
+++ b/clang/test/CodeGenCUDASPIRV/printf.cu
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fcuda-is-device -triple spirv32 -o - -emit-llvm -x cuda %s  | FileCheck --check-prefix=CHECK-SPIRV32 %s
+// RUN: %clang_cc1 -fcuda-is-device -triple spirv64 -o - -emit-llvm -x cuda %s  | FileCheck --check-prefix=CHECK-SPIRV64 %s
+
+// CHECK-SPIRV32: @.str = private unnamed_addr addrspace(4) constant [13 x i8] c"Hello World\0A\00", align 1
+// CHECK-SPIRV64: @.str = private unnamed_addr addrspace(1) constant [13 x i8] c"Hello World\0A\00", align 1
+
+extern "C" __attribute__((device)) int printf(const char* format, ...);
+
+__attribute__((global)) void printf_kernel() {
+  printf("Hello World\n");
+}

Copy link
Contributor

@AlexVlx AlexVlx left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! FWIW, perhaps it'd be worthwhile (as a follow-up, not here) to simply make the DefIsGen AS map correct/valid, and replace the dummy bits?

@sarnex
Copy link
Member Author

sarnex commented Apr 24, 2025

Thanks for the review, and will do @AlexVlx!

@sarnex sarnex merged commit 52a9649 into llvm:main Apr 24, 2025
15 checks passed
sarnex added a commit that referenced this pull request Apr 28, 2025
…ult AS (#137187)

Based on feedback from #136753,
remove the dummy values for OpenCL and make them match the zero default
AS map.

Signed-off-by: Sarnie, Nick <[email protected]>
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…36753)

This fixes a CUDA SPIR-V regression introduced in
llvm#134399.

---------

Signed-off-by: Sarnie, Nick <[email protected]>
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…ult AS (llvm#137187)

Based on feedback from llvm#136753,
remove the dummy values for OpenCL and make them match the zero default
AS map.

Signed-off-by: Sarnie, Nick <[email protected]>
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request May 6, 2025
…n-zero default AS (#137187)

Based on feedback from llvm/llvm-project#136753,
remove the dummy values for OpenCL and make them match the zero default
AS map.

Signed-off-by: Sarnie, Nick <[email protected]>
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…ult AS (llvm#137187)

Based on feedback from llvm#136753,
remove the dummy values for OpenCL and make them match the zero default
AS map.

Signed-off-by: Sarnie, Nick <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants