[Clang][AMDGPU] Permit language address spaces for AMDGPU globals

[jhuber6]

Summary:
Currently, there is an assertion that prevents us from emitting an
AMDGPU global with a non-target specific address space (i.e. numerical
attribute). I'm unsure what the original intentions of this assertion
were, but we should be able to use OpenCL address spaces when compiling
directly to AMDGPU from C++. This is permitted on NVPTX so I'm unsure
what this assertion is guarding. The patch simply removes the assertion
and adds a test to ensure that these emit the expected address spaces.

Fixes #65069

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-amdgpu

Changes

Summary: Currently, there is an assertion that prevents us from emitting an AMDGPU global with a non-target specific address space (i.e. numerical attribute). I'm unsure what the original intentions of this assertion were, but we should be able to use OpenCL address spaces when compiling directly to AMDGPU from C++. This is permitted on NVPTX so I'm unsure what this assertion is guarding. The patch simply removes the assertion and adds a test to ensure that these emit the expected address spaces.

Fixes #65069

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

2 Files Affected:

• (modified) clang/lib/CodeGen/Targets/AMDGPU.cpp (-1)
• (added) clang/test/CodeGen/amdgpu-address-spaces.cpp (+31)

diff --git a/clang/lib/CodeGen/Targets/AMDGPU.cpp b/clang/lib/CodeGen/Targets/AMDGPU.cpp
index c168bd4b7c7cc15..5aca4e540752524 100644
--- a/clang/lib/CodeGen/Targets/AMDGPU.cpp
+++ b/clang/lib/CodeGen/Targets/AMDGPU.cpp
@@ -438,7 +438,6 @@ AMDGPUTargetCodeGenInfo::getGlobalVarAddressSpace(CodeGenModule &CGM,
     return DefaultGlobalAS;
 
   LangAS AddrSpace = D->getType().getAddressSpace();
-  assert(AddrSpace == LangAS::Default || isTargetAddressSpace(AddrSpace));
   if (AddrSpace != LangAS::Default)
     return AddrSpace;
 
diff --git a/clang/test/CodeGen/amdgpu-address-spaces.cpp b/clang/test/CodeGen/amdgpu-address-spaces.cpp
new file mode 100644
index 000000000000000..e247cbb911added
--- /dev/null
+++ b/clang/test/CodeGen/amdgpu-address-spaces.cpp
@@ -0,0 +1,31 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --check-globals --version 3
+// RUN: %clang_cc1 -cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -o - | FileCheck %s
+
+int [[clang::opencl_global]] a = 0;
+int [[clang::opencl_generic]] b = 0;
+int [[clang::opencl_constant]] c = 0;
+[[clang::loader_uninitialized]] int [[clang::opencl_local]] d;
+[[clang::loader_uninitialized]] int [[clang::opencl_private]] e;
+
+//.
+// CHECK: @a = addrspace(1) global i32 0, align 4
+// CHECK: @b = global i32 0, align 4
+// CHECK: @c = addrspace(4) constant i32 0, align 4
+// CHECK: @d = addrspace(3) global i32 undef, align 4
+// CHECK: @e = addrspace(5) global i32 undef, align 4
+//.
+// CHECK-LABEL: define dso_local amdgpu_kernel void @_Z3foov(
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    store i32 1, ptr addrspace(1) @a, align 4
+// CHECK-NEXT:    store i32 1, ptr @b, align 4
+// CHECK-NEXT:    store i32 1, ptr addrspace(3) @d, align 4
+// CHECK-NEXT:    store i32 1, ptr addrspace(5) @e, align 4
+// CHECK-NEXT:    ret void
+//
+[[clang::amdgpu_kernel]] void foo() {
+  a = 1;
+  b = 1;
+  d = 1;
+  e = 1;
+}

[yxsamliu]

The assertion is for preventing language-specific address space e.g. OpenCL addr space from being used in addr space agnostic languages. The numerical addr space attr are mapped to target-specific AST addr space therefore won't cause assertion.

I am OK to remove that assertion if you would like to allow using AST addr space from different languages.

[arsenm]

The assertion is for preventing language-specific address space e.g. OpenCL addr space from being used in addr space agnostic languages.

If the intent was to forbid this it should be a proper error. Part of the point is to use these from other languages

[jhuber6]

The assertion is for preventing language-specific address space e.g. OpenCL addr space from being used in addr space agnostic languages. The numerical addr space attr are mapped to target-specific AST addr space therefore won't cause assertion.

I am OK to remove that assertion if you would like to allow using AST addr space from different languages.

I don't think there should be a restriction on these, since they should have the same exact semantics. It would be nice if we could split away a lot of these attributes from the OpenCL namespace. Specifically having access to these semantically checked address spaces as well as scoped atomics would be very nice. I keep putting off making an RFC for overloading the standard __atomic_ builtins to optionally accept a device scope.