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

[SPIR-V] Add hlsl_private address space for HLSL/SPIR-V #122103

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Keenuts
Copy link
Contributor

@Keenuts Keenuts commented Jan 8, 2025

In SPIR-V, private global variables have the Private storage class. This PR adds a new address space which allows frontend to emit variable with this storage class when targeting this backend.

This PR will cause addrspacecast to show up in several cases, like class member functions or assignment. Those will have to be handled in the backend later on, particularly to fixup pointer storage classes in some functions.

Before this change, global variable were emitted with the 'Function' storage class, which was wrong.

In SPIR-V, private global variables have the `Private` storage class.
This PR adds a new address space which allows frontend to emit variable
with this storage class.

Before this change, global variable were emitted with the 'Function'
storage class, which was wrong.

Signed-off-by: Nathan Gauër <[email protected]>
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:AArch64 backend:AMDGPU backend:SystemZ backend:WebAssembly clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen backend:DirectX HLSL HLSL Language Support labels Jan 8, 2025
@Keenuts Keenuts requested a review from s-perron January 8, 2025 12:48
@llvmbot
Copy link
Member

llvmbot commented Jan 8, 2025

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-backend-systemz

@llvm/pr-subscribers-backend-amdgpu

Author: Nathan Gauër (Keenuts)

Changes

In SPIR-V, private global variables have the Private storage class. This PR adds a new address space which allows frontend to emit variable with this storage class.

Before this change, global variable were emitted with the 'Function' storage class, which was wrong.


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

17 Files Affected:

  • (modified) clang/include/clang/Basic/AddressSpaces.h (+1)
  • (modified) clang/lib/AST/TypePrinter.cpp (+2)
  • (modified) clang/lib/Basic/TargetInfo.cpp (+1)
  • (modified) clang/lib/Basic/Targets/AArch64.h (+1)
  • (modified) clang/lib/Basic/Targets/AMDGPU.cpp (+2)
  • (modified) clang/lib/Basic/Targets/DirectX.h (+10-9)
  • (modified) clang/lib/Basic/Targets/NVPTX.h (+1)
  • (modified) clang/lib/Basic/Targets/SPIR.h (+22-20)
  • (modified) clang/lib/Basic/Targets/SystemZ.h (+1)
  • (modified) clang/lib/Basic/Targets/TCE.h (+1)
  • (modified) clang/lib/Basic/Targets/WebAssembly.h (+1)
  • (modified) clang/lib/Basic/Targets/X86.h (+1)
  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+17)
  • (modified) clang/test/CodeGenHLSL/GlobalDestructors.hlsl (+1-1)
  • (added) clang/test/CodeGenHLSL/out-of-line-static.hlsl (+27)
  • (modified) clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl (+5-4)
  • (modified) clang/test/SemaTemplate/address_space-dependent.cpp (+1-1)
diff --git a/clang/include/clang/Basic/AddressSpaces.h b/clang/include/clang/Basic/AddressSpaces.h
index 7b723d508fff17..8563d373d87367 100644
--- a/clang/include/clang/Basic/AddressSpaces.h
+++ b/clang/include/clang/Basic/AddressSpaces.h
@@ -58,6 +58,7 @@ enum class LangAS : unsigned {
 
   // HLSL specific address spaces.
   hlsl_groupshared,
+  hlsl_private,
 
   // Wasm specific address spaces.
   wasm_funcref,
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 7caebfb061a50b..1a273c76f71c41 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -2553,6 +2553,8 @@ std::string Qualifiers::getAddrSpaceAsString(LangAS AS) {
     return "__funcref";
   case LangAS::hlsl_groupshared:
     return "groupshared";
+  case LangAS::hlsl_private:
+    return "hlsl_private";
   default:
     return std::to_string(toTargetAddressSpace(AS));
   }
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 86befb1cbc74fc..80aa212afc5c91 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -47,6 +47,7 @@ static const LangASMap FakeAddrSpaceMap = {
     11, // ptr32_uptr
     12, // ptr64
     13, // hlsl_groupshared
+    14, // hlsl_private
     20, // wasm_funcref
 };
 
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 68a8b1ebad8cde..6ef38fac6da280 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -44,6 +44,7 @@ static const unsigned ARM64AddrSpaceMap[] = {
     static_cast<unsigned>(AArch64AddrSpace::ptr32_uptr),
     static_cast<unsigned>(AArch64AddrSpace::ptr64),
     0, // hlsl_groupshared
+    0, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/Basic/Targets/AMDGPU.cpp b/clang/lib/Basic/Targets/AMDGPU.cpp
index 99f8f2944e2796..83aac92e2ea3ca 100644
--- a/clang/lib/Basic/Targets/AMDGPU.cpp
+++ b/clang/lib/Basic/Targets/AMDGPU.cpp
@@ -59,6 +59,7 @@ const LangASMap AMDGPUTargetInfo::AMDGPUDefIsGenMap = {
     llvm::AMDGPUAS::FLAT_ADDRESS,     // ptr32_uptr
     llvm::AMDGPUAS::FLAT_ADDRESS,     // ptr64
     llvm::AMDGPUAS::FLAT_ADDRESS,     // hlsl_groupshared
+    llvm::AMDGPUAS::FLAT_ADDRESS,     // hlsl_private
 };
 
 const LangASMap AMDGPUTargetInfo::AMDGPUDefIsPrivMap = {
@@ -83,6 +84,7 @@ const LangASMap AMDGPUTargetInfo::AMDGPUDefIsPrivMap = {
     llvm::AMDGPUAS::FLAT_ADDRESS, // ptr32_uptr
     llvm::AMDGPUAS::FLAT_ADDRESS, // ptr64
     llvm::AMDGPUAS::FLAT_ADDRESS, // hlsl_groupshared
+    llvm::AMDGPUAS::FLAT_ADDRESS, // hlsl_private
 
 };
 } // namespace targets
diff --git a/clang/lib/Basic/Targets/DirectX.h b/clang/lib/Basic/Targets/DirectX.h
index ab22d1281a4df7..2cbb724386870e 100644
--- a/clang/lib/Basic/Targets/DirectX.h
+++ b/clang/lib/Basic/Targets/DirectX.h
@@ -33,15 +33,16 @@ static const unsigned DirectXAddrSpaceMap[] = {
     0, // cuda_constant
     0, // cuda_shared
     // SYCL address space values for this map are dummy
-    0, // sycl_global
-    0, // sycl_global_device
-    0, // sycl_global_host
-    0, // sycl_local
-    0, // sycl_private
-    0, // ptr32_sptr
-    0, // ptr32_uptr
-    0, // ptr64
-    3, // hlsl_groupshared
+    0,  // sycl_global
+    0,  // sycl_global_device
+    0,  // sycl_global_host
+    0,  // sycl_local
+    0,  // sycl_private
+    0,  // ptr32_sptr
+    0,  // ptr32_uptr
+    0,  // ptr64
+    3,  // hlsl_groupshared
+    10, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/Basic/Targets/NVPTX.h b/clang/lib/Basic/Targets/NVPTX.h
index d81b89a7f24ac0..c6f4e1938a04df 100644
--- a/clang/lib/Basic/Targets/NVPTX.h
+++ b/clang/lib/Basic/Targets/NVPTX.h
@@ -46,6 +46,7 @@ static const unsigned NVPTXAddrSpaceMap[] = {
     0, // ptr32_uptr
     0, // ptr64
     0, // hlsl_groupshared
+    0, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/Basic/Targets/SPIR.h b/clang/lib/Basic/Targets/SPIR.h
index 5a328b9ceeb1d1..b3fdc055822b1b 100644
--- a/clang/lib/Basic/Targets/SPIR.h
+++ b/clang/lib/Basic/Targets/SPIR.h
@@ -38,15 +38,16 @@ static const unsigned SPIRDefIsPrivMap[] = {
     0, // cuda_constant
     0, // cuda_shared
     // SYCL address space values for this map are dummy
-    0, // sycl_global
-    0, // sycl_global_device
-    0, // sycl_global_host
-    0, // sycl_local
-    0, // sycl_private
-    0, // ptr32_sptr
-    0, // ptr32_uptr
-    0, // ptr64
-    0, // hlsl_groupshared
+    0,  // sycl_global
+    0,  // sycl_global_device
+    0,  // sycl_global_host
+    0,  // sycl_local
+    0,  // sycl_private
+    0,  // ptr32_sptr
+    0,  // ptr32_uptr
+    0,  // ptr64
+    0,  // hlsl_groupshared
+    10, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
@@ -69,17 +70,18 @@ static const unsigned SPIRDefIsGenMap[] = {
     // cuda_constant pointer can be casted to default/"flat" pointer, but in
     // SPIR-V casts between constant and generic pointers are not allowed. For
     // this reason cuda_constant is mapped to SPIR-V CrossWorkgroup.
-    1, // cuda_constant
-    3, // cuda_shared
-    1, // sycl_global
-    5, // sycl_global_device
-    6, // sycl_global_host
-    3, // sycl_local
-    0, // sycl_private
-    0, // ptr32_sptr
-    0, // ptr32_uptr
-    0, // ptr64
-    0, // hlsl_groupshared
+    1,  // cuda_constant
+    3,  // cuda_shared
+    1,  // sycl_global
+    5,  // sycl_global_device
+    6,  // sycl_global_host
+    3,  // sycl_local
+    0,  // sycl_private
+    0,  // ptr32_sptr
+    0,  // ptr32_uptr
+    0,  // ptr64
+    0,  // hlsl_groupshared
+    10, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/Basic/Targets/SystemZ.h b/clang/lib/Basic/Targets/SystemZ.h
index e6405f174f660f..6d9b168fea8580 100644
--- a/clang/lib/Basic/Targets/SystemZ.h
+++ b/clang/lib/Basic/Targets/SystemZ.h
@@ -42,6 +42,7 @@ static const unsigned ZOSAddressMap[] = {
     1, // ptr32_uptr
     0, // ptr64
     0, // hlsl_groupshared
+    0, // hlsl_private
     0  // wasm_funcref
 };
 
diff --git a/clang/lib/Basic/Targets/TCE.h b/clang/lib/Basic/Targets/TCE.h
index d6280b02f07b25..c2e9b9681f0a89 100644
--- a/clang/lib/Basic/Targets/TCE.h
+++ b/clang/lib/Basic/Targets/TCE.h
@@ -51,6 +51,7 @@ static const unsigned TCEOpenCLAddrSpaceMap[] = {
     0, // ptr32_uptr
     0, // ptr64
     0, // hlsl_groupshared
+    0, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/Basic/Targets/WebAssembly.h b/clang/lib/Basic/Targets/WebAssembly.h
index 0a14da6a277b8e..a5c6d5c3778423 100644
--- a/clang/lib/Basic/Targets/WebAssembly.h
+++ b/clang/lib/Basic/Targets/WebAssembly.h
@@ -42,6 +42,7 @@ static const unsigned WebAssemblyAddrSpaceMap[] = {
     0,  // ptr32_uptr
     0,  // ptr64
     0,  // hlsl_groupshared
+    0,  // hlsl_private
     20, // wasm_funcref
 };
 
diff --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index 553c452d4ba3c2..f00722be6a0c74 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -46,6 +46,7 @@ static const unsigned X86AddrSpaceMap[] = {
     271, // ptr32_uptr
     272, // ptr64
     0,   // hlsl_groupshared
+    0,   // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 5f15f0f48c54e4..d4318b9c7abc9a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5362,6 +5362,23 @@ LangAS CodeGenModule::GetGlobalVarAddressSpace(const VarDecl *D) {
     if (OpenMPRuntime->hasAllocateAttributeForGlobalVar(D, AS))
       return AS;
   }
+
+  if (LangOpts.HLSL) {
+    if (D == nullptr)
+      return LangAS::hlsl_private;
+
+    // Except resources (Uniform, UniformConstant) & instanglble (handles)
+    if (D->getType()->isHLSLResourceType() ||
+        D->getType()->isHLSLIntangibleType())
+      return D->getType().getAddressSpace();
+
+    if (D->getStorageClass() != SC_Static)
+      return D->getType().getAddressSpace();
+
+    LangAS AS = D->getType().getAddressSpace();
+    return AS == LangAS::Default ? LangAS::hlsl_private : AS;
+  }
+
   return getTargetCodeGenInfo().getGlobalVarAddressSpace(*this, D);
 }
 
diff --git a/clang/test/CodeGenHLSL/GlobalDestructors.hlsl b/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
index f98318601134bb..62489457ff0868 100644
--- a/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
+++ b/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
@@ -71,7 +71,7 @@ void main(unsigned GI : SV_GroupIndex) {
 
 // NOINLINE: define internal void @_GLOBAL__D_a() [[IntAttr:\#[0-9]+]]
 // NOINLINE-NEXT: entry:
-// NOINLINE-NEXT:   call void @_ZN4TailD1Ev(ptr @_ZZ3WagvE1T)
+// NOINLINE-NEXT:   call void @_ZN4TailD1Ev(ptr addrspacecast (ptr addrspace(10) @_ZZ3WagvE1T to ptr))
 // NOINLINE-NEXT:   call void @_ZN6PupperD1Ev(ptr @GlobalPup)
 // NOINLINE-NEXT:   ret void
 
diff --git a/clang/test/CodeGenHLSL/out-of-line-static.hlsl b/clang/test/CodeGenHLSL/out-of-line-static.hlsl
new file mode 100644
index 00000000000000..98666560dabd43
--- /dev/null
+++ b/clang/test/CodeGenHLSL/out-of-line-static.hlsl
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -std=hlsl202x -emit-llvm -o - -disable-llvm-passes %s | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_cc1 -triple spirv-pc-vulkan1.3-compute -std=hlsl202x -emit-llvm -o - -disable-llvm-passes %s | FileCheck %s --check-prefixes=CHECK
+
+struct S {
+  static int Value;
+};
+
+int S::Value = 1;
+// CHECK: @_ZN1S5ValueE = global i32 1, align 4
+
+[shader("compute")]
+[numthreads(1,1,1)]
+void main() {
+  S s;
+  int value1, value2;
+// CHECK:      %s = alloca %struct.S, align 1
+// CHECK: %value1 = alloca i32, align 4
+// CHECK: %value2 = alloca i32, align 4
+
+// CHECK: [[tmp:%.*]] = load i32, ptr @_ZN1S5ValueE, align 4
+// CHECK: store i32 [[tmp]], ptr %value1, align 4
+  value1 = S::Value;
+
+// CHECK: [[tmp:%.*]] = load i32, ptr @_ZN1S5ValueE, align 4
+// CHECK: store i32 [[tmp]], ptr %value2, align 4
+  value2 = s.Value;
+}
diff --git a/clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl b/clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
index f85bab2113170b..c78a5aa6881e2a 100644
--- a/clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
+++ b/clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
@@ -3,12 +3,13 @@
 
 // CHECK-DAG: @[[CB:.+]] = external constant { float }
 
+// CHECK-DAG:@_ZL1b = internal addrspace(10) global float 3.000000e+00, align 4
+static float b = 3;
+
 cbuffer A {
-    float a;
-  // CHECK-DAG:@_ZL1b = internal global float 3.000000e+00, align 4
-  static float b = 3;
+  float a;
   // CHECK:load float, ptr @[[CB]], align 4
-  // CHECK:load float, ptr @_ZL1b, align 4
+  // CHECK:load float, ptr addrspacecast (ptr addrspace(10) @_ZL1b to ptr), align 4
   float foo() { return a + b; }
 }
 
diff --git a/clang/test/SemaTemplate/address_space-dependent.cpp b/clang/test/SemaTemplate/address_space-dependent.cpp
index 2ca9b8007ab418..eb8dbc69a945e2 100644
--- a/clang/test/SemaTemplate/address_space-dependent.cpp
+++ b/clang/test/SemaTemplate/address_space-dependent.cpp
@@ -43,7 +43,7 @@ void neg() {
 
 template <long int I>
 void tooBig() {
-  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388586)}}
+  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388585)}}
 }
 
 template <long int I>

@llvmbot
Copy link
Member

llvmbot commented Jan 8, 2025

@llvm/pr-subscribers-backend-directx

Author: Nathan Gauër (Keenuts)

Changes

In SPIR-V, private global variables have the Private storage class. This PR adds a new address space which allows frontend to emit variable with this storage class.

Before this change, global variable were emitted with the 'Function' storage class, which was wrong.


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

17 Files Affected:

  • (modified) clang/include/clang/Basic/AddressSpaces.h (+1)
  • (modified) clang/lib/AST/TypePrinter.cpp (+2)
  • (modified) clang/lib/Basic/TargetInfo.cpp (+1)
  • (modified) clang/lib/Basic/Targets/AArch64.h (+1)
  • (modified) clang/lib/Basic/Targets/AMDGPU.cpp (+2)
  • (modified) clang/lib/Basic/Targets/DirectX.h (+10-9)
  • (modified) clang/lib/Basic/Targets/NVPTX.h (+1)
  • (modified) clang/lib/Basic/Targets/SPIR.h (+22-20)
  • (modified) clang/lib/Basic/Targets/SystemZ.h (+1)
  • (modified) clang/lib/Basic/Targets/TCE.h (+1)
  • (modified) clang/lib/Basic/Targets/WebAssembly.h (+1)
  • (modified) clang/lib/Basic/Targets/X86.h (+1)
  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+17)
  • (modified) clang/test/CodeGenHLSL/GlobalDestructors.hlsl (+1-1)
  • (added) clang/test/CodeGenHLSL/out-of-line-static.hlsl (+27)
  • (modified) clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl (+5-4)
  • (modified) clang/test/SemaTemplate/address_space-dependent.cpp (+1-1)
diff --git a/clang/include/clang/Basic/AddressSpaces.h b/clang/include/clang/Basic/AddressSpaces.h
index 7b723d508fff17..8563d373d87367 100644
--- a/clang/include/clang/Basic/AddressSpaces.h
+++ b/clang/include/clang/Basic/AddressSpaces.h
@@ -58,6 +58,7 @@ enum class LangAS : unsigned {
 
   // HLSL specific address spaces.
   hlsl_groupshared,
+  hlsl_private,
 
   // Wasm specific address spaces.
   wasm_funcref,
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 7caebfb061a50b..1a273c76f71c41 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -2553,6 +2553,8 @@ std::string Qualifiers::getAddrSpaceAsString(LangAS AS) {
     return "__funcref";
   case LangAS::hlsl_groupshared:
     return "groupshared";
+  case LangAS::hlsl_private:
+    return "hlsl_private";
   default:
     return std::to_string(toTargetAddressSpace(AS));
   }
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 86befb1cbc74fc..80aa212afc5c91 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -47,6 +47,7 @@ static const LangASMap FakeAddrSpaceMap = {
     11, // ptr32_uptr
     12, // ptr64
     13, // hlsl_groupshared
+    14, // hlsl_private
     20, // wasm_funcref
 };
 
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 68a8b1ebad8cde..6ef38fac6da280 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -44,6 +44,7 @@ static const unsigned ARM64AddrSpaceMap[] = {
     static_cast<unsigned>(AArch64AddrSpace::ptr32_uptr),
     static_cast<unsigned>(AArch64AddrSpace::ptr64),
     0, // hlsl_groupshared
+    0, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/Basic/Targets/AMDGPU.cpp b/clang/lib/Basic/Targets/AMDGPU.cpp
index 99f8f2944e2796..83aac92e2ea3ca 100644
--- a/clang/lib/Basic/Targets/AMDGPU.cpp
+++ b/clang/lib/Basic/Targets/AMDGPU.cpp
@@ -59,6 +59,7 @@ const LangASMap AMDGPUTargetInfo::AMDGPUDefIsGenMap = {
     llvm::AMDGPUAS::FLAT_ADDRESS,     // ptr32_uptr
     llvm::AMDGPUAS::FLAT_ADDRESS,     // ptr64
     llvm::AMDGPUAS::FLAT_ADDRESS,     // hlsl_groupshared
+    llvm::AMDGPUAS::FLAT_ADDRESS,     // hlsl_private
 };
 
 const LangASMap AMDGPUTargetInfo::AMDGPUDefIsPrivMap = {
@@ -83,6 +84,7 @@ const LangASMap AMDGPUTargetInfo::AMDGPUDefIsPrivMap = {
     llvm::AMDGPUAS::FLAT_ADDRESS, // ptr32_uptr
     llvm::AMDGPUAS::FLAT_ADDRESS, // ptr64
     llvm::AMDGPUAS::FLAT_ADDRESS, // hlsl_groupshared
+    llvm::AMDGPUAS::FLAT_ADDRESS, // hlsl_private
 
 };
 } // namespace targets
diff --git a/clang/lib/Basic/Targets/DirectX.h b/clang/lib/Basic/Targets/DirectX.h
index ab22d1281a4df7..2cbb724386870e 100644
--- a/clang/lib/Basic/Targets/DirectX.h
+++ b/clang/lib/Basic/Targets/DirectX.h
@@ -33,15 +33,16 @@ static const unsigned DirectXAddrSpaceMap[] = {
     0, // cuda_constant
     0, // cuda_shared
     // SYCL address space values for this map are dummy
-    0, // sycl_global
-    0, // sycl_global_device
-    0, // sycl_global_host
-    0, // sycl_local
-    0, // sycl_private
-    0, // ptr32_sptr
-    0, // ptr32_uptr
-    0, // ptr64
-    3, // hlsl_groupshared
+    0,  // sycl_global
+    0,  // sycl_global_device
+    0,  // sycl_global_host
+    0,  // sycl_local
+    0,  // sycl_private
+    0,  // ptr32_sptr
+    0,  // ptr32_uptr
+    0,  // ptr64
+    3,  // hlsl_groupshared
+    10, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/Basic/Targets/NVPTX.h b/clang/lib/Basic/Targets/NVPTX.h
index d81b89a7f24ac0..c6f4e1938a04df 100644
--- a/clang/lib/Basic/Targets/NVPTX.h
+++ b/clang/lib/Basic/Targets/NVPTX.h
@@ -46,6 +46,7 @@ static const unsigned NVPTXAddrSpaceMap[] = {
     0, // ptr32_uptr
     0, // ptr64
     0, // hlsl_groupshared
+    0, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/Basic/Targets/SPIR.h b/clang/lib/Basic/Targets/SPIR.h
index 5a328b9ceeb1d1..b3fdc055822b1b 100644
--- a/clang/lib/Basic/Targets/SPIR.h
+++ b/clang/lib/Basic/Targets/SPIR.h
@@ -38,15 +38,16 @@ static const unsigned SPIRDefIsPrivMap[] = {
     0, // cuda_constant
     0, // cuda_shared
     // SYCL address space values for this map are dummy
-    0, // sycl_global
-    0, // sycl_global_device
-    0, // sycl_global_host
-    0, // sycl_local
-    0, // sycl_private
-    0, // ptr32_sptr
-    0, // ptr32_uptr
-    0, // ptr64
-    0, // hlsl_groupshared
+    0,  // sycl_global
+    0,  // sycl_global_device
+    0,  // sycl_global_host
+    0,  // sycl_local
+    0,  // sycl_private
+    0,  // ptr32_sptr
+    0,  // ptr32_uptr
+    0,  // ptr64
+    0,  // hlsl_groupshared
+    10, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
@@ -69,17 +70,18 @@ static const unsigned SPIRDefIsGenMap[] = {
     // cuda_constant pointer can be casted to default/"flat" pointer, but in
     // SPIR-V casts between constant and generic pointers are not allowed. For
     // this reason cuda_constant is mapped to SPIR-V CrossWorkgroup.
-    1, // cuda_constant
-    3, // cuda_shared
-    1, // sycl_global
-    5, // sycl_global_device
-    6, // sycl_global_host
-    3, // sycl_local
-    0, // sycl_private
-    0, // ptr32_sptr
-    0, // ptr32_uptr
-    0, // ptr64
-    0, // hlsl_groupshared
+    1,  // cuda_constant
+    3,  // cuda_shared
+    1,  // sycl_global
+    5,  // sycl_global_device
+    6,  // sycl_global_host
+    3,  // sycl_local
+    0,  // sycl_private
+    0,  // ptr32_sptr
+    0,  // ptr32_uptr
+    0,  // ptr64
+    0,  // hlsl_groupshared
+    10, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/Basic/Targets/SystemZ.h b/clang/lib/Basic/Targets/SystemZ.h
index e6405f174f660f..6d9b168fea8580 100644
--- a/clang/lib/Basic/Targets/SystemZ.h
+++ b/clang/lib/Basic/Targets/SystemZ.h
@@ -42,6 +42,7 @@ static const unsigned ZOSAddressMap[] = {
     1, // ptr32_uptr
     0, // ptr64
     0, // hlsl_groupshared
+    0, // hlsl_private
     0  // wasm_funcref
 };
 
diff --git a/clang/lib/Basic/Targets/TCE.h b/clang/lib/Basic/Targets/TCE.h
index d6280b02f07b25..c2e9b9681f0a89 100644
--- a/clang/lib/Basic/Targets/TCE.h
+++ b/clang/lib/Basic/Targets/TCE.h
@@ -51,6 +51,7 @@ static const unsigned TCEOpenCLAddrSpaceMap[] = {
     0, // ptr32_uptr
     0, // ptr64
     0, // hlsl_groupshared
+    0, // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/Basic/Targets/WebAssembly.h b/clang/lib/Basic/Targets/WebAssembly.h
index 0a14da6a277b8e..a5c6d5c3778423 100644
--- a/clang/lib/Basic/Targets/WebAssembly.h
+++ b/clang/lib/Basic/Targets/WebAssembly.h
@@ -42,6 +42,7 @@ static const unsigned WebAssemblyAddrSpaceMap[] = {
     0,  // ptr32_uptr
     0,  // ptr64
     0,  // hlsl_groupshared
+    0,  // hlsl_private
     20, // wasm_funcref
 };
 
diff --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index 553c452d4ba3c2..f00722be6a0c74 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -46,6 +46,7 @@ static const unsigned X86AddrSpaceMap[] = {
     271, // ptr32_uptr
     272, // ptr64
     0,   // hlsl_groupshared
+    0,   // hlsl_private
     // Wasm address space values for this target are dummy values,
     // as it is only enabled for Wasm targets.
     20, // wasm_funcref
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 5f15f0f48c54e4..d4318b9c7abc9a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5362,6 +5362,23 @@ LangAS CodeGenModule::GetGlobalVarAddressSpace(const VarDecl *D) {
     if (OpenMPRuntime->hasAllocateAttributeForGlobalVar(D, AS))
       return AS;
   }
+
+  if (LangOpts.HLSL) {
+    if (D == nullptr)
+      return LangAS::hlsl_private;
+
+    // Except resources (Uniform, UniformConstant) & instanglble (handles)
+    if (D->getType()->isHLSLResourceType() ||
+        D->getType()->isHLSLIntangibleType())
+      return D->getType().getAddressSpace();
+
+    if (D->getStorageClass() != SC_Static)
+      return D->getType().getAddressSpace();
+
+    LangAS AS = D->getType().getAddressSpace();
+    return AS == LangAS::Default ? LangAS::hlsl_private : AS;
+  }
+
   return getTargetCodeGenInfo().getGlobalVarAddressSpace(*this, D);
 }
 
diff --git a/clang/test/CodeGenHLSL/GlobalDestructors.hlsl b/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
index f98318601134bb..62489457ff0868 100644
--- a/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
+++ b/clang/test/CodeGenHLSL/GlobalDestructors.hlsl
@@ -71,7 +71,7 @@ void main(unsigned GI : SV_GroupIndex) {
 
 // NOINLINE: define internal void @_GLOBAL__D_a() [[IntAttr:\#[0-9]+]]
 // NOINLINE-NEXT: entry:
-// NOINLINE-NEXT:   call void @_ZN4TailD1Ev(ptr @_ZZ3WagvE1T)
+// NOINLINE-NEXT:   call void @_ZN4TailD1Ev(ptr addrspacecast (ptr addrspace(10) @_ZZ3WagvE1T to ptr))
 // NOINLINE-NEXT:   call void @_ZN6PupperD1Ev(ptr @GlobalPup)
 // NOINLINE-NEXT:   ret void
 
diff --git a/clang/test/CodeGenHLSL/out-of-line-static.hlsl b/clang/test/CodeGenHLSL/out-of-line-static.hlsl
new file mode 100644
index 00000000000000..98666560dabd43
--- /dev/null
+++ b/clang/test/CodeGenHLSL/out-of-line-static.hlsl
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -std=hlsl202x -emit-llvm -o - -disable-llvm-passes %s | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_cc1 -triple spirv-pc-vulkan1.3-compute -std=hlsl202x -emit-llvm -o - -disable-llvm-passes %s | FileCheck %s --check-prefixes=CHECK
+
+struct S {
+  static int Value;
+};
+
+int S::Value = 1;
+// CHECK: @_ZN1S5ValueE = global i32 1, align 4
+
+[shader("compute")]
+[numthreads(1,1,1)]
+void main() {
+  S s;
+  int value1, value2;
+// CHECK:      %s = alloca %struct.S, align 1
+// CHECK: %value1 = alloca i32, align 4
+// CHECK: %value2 = alloca i32, align 4
+
+// CHECK: [[tmp:%.*]] = load i32, ptr @_ZN1S5ValueE, align 4
+// CHECK: store i32 [[tmp]], ptr %value1, align 4
+  value1 = S::Value;
+
+// CHECK: [[tmp:%.*]] = load i32, ptr @_ZN1S5ValueE, align 4
+// CHECK: store i32 [[tmp]], ptr %value2, align 4
+  value2 = s.Value;
+}
diff --git a/clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl b/clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
index f85bab2113170b..c78a5aa6881e2a 100644
--- a/clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
+++ b/clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
@@ -3,12 +3,13 @@
 
 // CHECK-DAG: @[[CB:.+]] = external constant { float }
 
+// CHECK-DAG:@_ZL1b = internal addrspace(10) global float 3.000000e+00, align 4
+static float b = 3;
+
 cbuffer A {
-    float a;
-  // CHECK-DAG:@_ZL1b = internal global float 3.000000e+00, align 4
-  static float b = 3;
+  float a;
   // CHECK:load float, ptr @[[CB]], align 4
-  // CHECK:load float, ptr @_ZL1b, align 4
+  // CHECK:load float, ptr addrspacecast (ptr addrspace(10) @_ZL1b to ptr), align 4
   float foo() { return a + b; }
 }
 
diff --git a/clang/test/SemaTemplate/address_space-dependent.cpp b/clang/test/SemaTemplate/address_space-dependent.cpp
index 2ca9b8007ab418..eb8dbc69a945e2 100644
--- a/clang/test/SemaTemplate/address_space-dependent.cpp
+++ b/clang/test/SemaTemplate/address_space-dependent.cpp
@@ -43,7 +43,7 @@ void neg() {
 
 template <long int I>
 void tooBig() {
-  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388586)}}
+  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388585)}}
 }
 
 template <long int I>

@@ -58,6 +58,7 @@ enum class LangAS : unsigned {

// HLSL specific address spaces.
hlsl_groupshared,
hlsl_private,
Copy link
Contributor

Choose a reason for hiding this comment

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

Title and description only talks about SPIRV, but this is a purely language concept you're adding here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct, slightly reworded the description.
The thing is, only SPIR-V will be using this for now (same as the few others we'll add line hlsl_input, hlsl_output).

Copy link
Contributor

Choose a reason for hiding this comment

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

only SPIR-V will be using this for now

That's not really how languages work. It's a property of the language and exists independently of what any codegen is doing with it. As it is it sounds like you're jamming some knob you want for codegen through here that isn't part of the language.

This needs reference to some HLSL spec describing the address space without any mention of SPIRV.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This being a small stepping stone for the larger change bringing hlsl_input/hlsl_output, I haven't wrote a specific HLSL spec part, only relied on current MSDN & HLSL implementations (I agree, not great).
For the hlsl_input/hlsl_output address spaces, here is the wg-hlsl proposal:
llvm/wg-hlsl#97

But you are right, maybe that's something we should now write properly in the hlsl-spec repo

@@ -83,6 +84,7 @@ const LangASMap AMDGPUTargetInfo::AMDGPUDefIsPrivMap = {
llvm::AMDGPUAS::FLAT_ADDRESS, // ptr32_uptr
llvm::AMDGPUAS::FLAT_ADDRESS, // ptr64
llvm::AMDGPUAS::FLAT_ADDRESS, // hlsl_groupshared
llvm::AMDGPUAS::FLAT_ADDRESS, // hlsl_private
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what hlsl_private means but using flat here is almost certainly wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This address space will be used for:

  • static [const] global variables
    Which matches the Private storage class for SPIR-V.

Would LOCAL_ADDRESS be a better match?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or maybe PRIVATE_ADDRESS would be best, since it's local per thread/wave

Copy link
Contributor

Choose a reason for hiding this comment

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

If it acts like thread local storage, we don't have an implementation of that right now. So it would be none of these. PRIVATE_ADDRESS globals will just fail and nothing else has thread-local like behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

HLSL documentation for those is currently this: https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-variable-syntax

FXC/DXC implementation of a static int my_global would be equivalent to a c++ static thread_local int my_global. Initialized once, thread local.

If this is not available in AMDGPU yet, what value shall be picked?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should use PRIVATE_ADDRESS. It will at least fail in a way that makes it somewhat obvious what the issue is

@Keenuts Keenuts changed the title [SPIR-V] Add hlsl_private address space for SPIR-V [SPIR-V] Add hlsl_private address space for HLSL Jan 8, 2025
@Keenuts Keenuts changed the title [SPIR-V] Add hlsl_private address space for HLSL [SPIR-V] Add hlsl_private address space for HLSL/SPIR-V Jan 8, 2025
Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

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

We talked about this yesterday in a call with @Keenuts and @s-perron.

I think one of the problems we have with HLSL is that we haven't had explicit address space annotations except for groupshared in the past. We do need to do that in the future, but I'm not sure this was the right slicing.

In HLSL static variables are stored in thread-local addresses, but have thread lifetime, whereas function local variables are stored in thread-local addresses but have function lifetime.

In both cases the storage is thread-local, and they can be used interchangeably as addresses so we should have them in the same address space, but with different storage classes and lifetimes.

There are cases (specifically around the implicit this object) where we'll need to follow the OpenCL approach and insert addrspacecasts that we can clean up later in the compiler. I don't think we have a good way around that until we can evolve the language to express address spaces more comprehensively.

Those issues will need to be addressed in the language, and we have a few different related proposals:

I've also filed an issue to track specifying this further: microsoft/hlsl-specs#364

LangAS AS = D->getType().getAddressSpace();
return AS == LangAS::Default ? LangAS::hlsl_private : AS;
}

Copy link
Member

@hekota hekota Jan 14, 2025

Choose a reason for hiding this comment

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

Have you considered applying the hlsl_private address space in the Sema (SemaHLSL::ActOnVariableDeclarator handler) onto the VarDecl QualType?

FYI, I am currently looking into adding hlsl_constant address space to be used for constant buffer declarations and that is probably going to be the place where I'll add it.

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe this is the best place to do it since it is unspellable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I recall having tried this method first, but for some reasons moved it to Codegen, but I don't recall the full context.

I just tried moving it in sema again, and seems there is a slight issue with this-reference-template.hlsl since the VarDecl type is from a template, something goes wrong (but works if the AS fixup is done in codegen).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 backend:AMDGPU backend:DirectX backend:SystemZ backend:WebAssembly clang:codegen clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants