Skip to content

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

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

Closed
wants to merge 3 commits into 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 is covered in this proposal: llvm/wg-hlsl@4c9e11a

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.

@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 IR generation bugs: mangling, exceptions, etc. 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>

@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

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]>
@Keenuts Keenuts force-pushed the private-address-space branch from 4336e42 to c11ace7 Compare March 6, 2025 12:44
@Keenuts Keenuts changed the title [SPIR-V] Add hlsl_private address space for HLSL/SPIR-V [HLSL][SPIR-V] Add hlsl_private address space for HLSL/SPIR-V Mar 6, 2025
@Keenuts Keenuts requested review from hekota, llvm-beanz and arsenm March 6, 2025 12:46
@Keenuts
Copy link
Contributor Author

Keenuts commented Mar 6, 2025

Hi all!

This PR was dormant for a while because proposals had to be made & merged.
The HLSL proposal which covers hlsl_private is now merged: llvm/wg-hlsl@4c9e11a

Copy link
Member

@hekota hekota left a comment

Choose a reason for hiding this comment

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

LGTM with one question.

// CHECK: store i32 [[tmp]], ptr %value1, align 4
value1 = S::Value;

// CHECK: [[tmp:%.*]] = load i32, ptr @_ZN1S5ValueE, align 4
Copy link
Member

Choose a reason for hiding this comment

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

Why is this testing the same thing twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it? The codegen is the same, but the syntax is slightly different, hence could imagine the code paths would be different?

@llvm-beanz
Copy link
Collaborator

Is there a reason we can't do this in Sema? It would probably be ideal to have the AST represent the address spaces of values accurately.

@Keenuts
Copy link
Contributor Author

Keenuts commented Mar 28, 2025

Is there a reason we can't do this in Sema? It would probably be ideal to have the AST represent the address spaces of values accurately.

Hello! So I didn't recalled the details on why, so I made another PR which implements this in sema (#133464)

Keenuts added a commit that referenced this pull request Apr 10, 2025
This is an alternative to
#122103

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 is covered in this proposal: llvm/wg-hlsl@4c9e11a

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.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Apr 10, 2025
…#133464)

This is an alternative to
llvm/llvm-project#122103

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 is covered in this proposal: llvm/wg-hlsl@4c9e11a

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.
@s-perron
Copy link
Contributor

This PR is superseded by #133464

@s-perron s-perron closed this Apr 16, 2025
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
This is an alternative to
llvm#122103

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 is covered in this proposal: llvm/wg-hlsl@4c9e11a

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.
@damyanp damyanp removed this from HLSL Support Jun 25, 2025
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 IR generation bugs: mangling, exceptions, etc. clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants