Skip to content

[HLSL] Add SPIR-V target type for StructuredBuffers #132027

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

s-perron
Copy link
Contributor

This PR adds the target type that should be used for
RWStructuredBuffers. It does not handle ByteAddressBuffers yet.

For now all structs will be laid out using the standard C/C++ layout
rules. Other layout rules will be implemented later.

@s-perron s-perron requested a review from Keenuts March 19, 2025 13:44
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. HLSL HLSL Language Support labels Mar 19, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 19, 2025

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Steven Perron (s-perron)

Changes

This PR adds the target type that should be used for
RWStructuredBuffers. It does not handle ByteAddressBuffers yet.

For now all structs will be laid out using the standard C/C++ layout
rules. Other layout rules will be implemented later.


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/Targets/SPIR.cpp (+20-3)
  • (modified) clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl (+38-30)
diff --git a/clang/lib/CodeGen/Targets/SPIR.cpp b/clang/lib/CodeGen/Targets/SPIR.cpp
index 43f511e572d37..5f5c10e291d89 100644
--- a/clang/lib/CodeGen/Targets/SPIR.cpp
+++ b/clang/lib/CodeGen/Targets/SPIR.cpp
@@ -386,13 +386,30 @@ llvm::Type *CommonSPIRTargetCodeGenInfo::getHLSLType(
     if (ContainedTy.isNull())
       return nullptr;
 
-    assert(!ResAttrs.RawBuffer &&
-           "Raw buffers handles are not implemented for SPIR-V yet");
     assert(!ResAttrs.IsROV &&
            "Rasterizer order views not implemented for SPIR-V yet");
 
-    // convert element type
     llvm::Type *ElemType = CGM.getTypes().ConvertType(ContainedTy);
+    if (ResAttrs.RawBuffer) {
+      // TODO: Handle types with layout information.
+      assert((ElemType->isIntegerTy() || ElemType->isFloatingPointTy() ||
+              ElemType->isVectorTy()) &&
+             "The element type for a SPIR-V resource must be a types that does "
+             "not require layout information.");
+      llvm::ArrayType *RuntimeArrayType = llvm::ArrayType::get(ElemType, 0);
+
+      uint32_t StorageClass = /* StorageBuffer storage class */ 12;
+      bool IsWritable =
+          ResAttrs.ResourceClass == llvm::dxil::ResourceClass::UAV;
+      assert(!IsWritable && "Writable buffers require a corresponding counter "
+                            "variable. Not implemented yet.");
+      bool IsRov = ResAttrs.IsROV;
+      return llvm::TargetExtType::get(Ctx, "spirv.VulkanBuffer",
+                                      {RuntimeArrayType},
+                                      {StorageClass, IsWritable, IsRov});
+    }
+
+    // convert element type
     return getSPIRVImageTypeFromHLSLResource(ResAttrs, ElemType, Ctx);
   }
   case llvm::dxil::ResourceClass::CBuffer:
diff --git a/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl b/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl
index 04534c5550252..74abf6b1340de 100644
--- a/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl
@@ -1,59 +1,67 @@
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-DXIL
-// RUN-DISABLED: %clang_cc1 -triple spirv-vulkan-library -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-SPIRV
+// RUN: %clang_cc1 -triple spirv-vulkan-library -x hlsl -emit-llvm -DSPIRV -disable-llvm-passes -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-SPIRV
 
-// NOTE: SPIRV codegen for resource types is not yet implemented
 
 StructuredBuffer<float> Buf : register(t10);
+
+#ifndef SPIRV
+// NOTE: SPIRV codegen for resource types with counter variable is not yet implemented
 RWStructuredBuffer<float> Buf2 : register(u5, space1);
 AppendStructuredBuffer<float> Buf3 : register(u3);
 ConsumeStructuredBuffer<float> Buf4 : register(u4);
 RasterizerOrderedStructuredBuffer<float> Buf5 : register(u1, space2);
+#endif
+
+// CHECK-DXIL: %"class.hlsl::StructuredBuffer" = type { target("dx.RawBuffer", float, 0, 0) }
+// CHECK-DXIL: %"class.hlsl::RWStructuredBuffer" = type { target("dx.RawBuffer", float, 1, 0) }
+// CHECK-DXIL: %"class.hlsl::AppendStructuredBuffer" = type { target("dx.RawBuffer", float, 1, 0) }
+// CHECK-DXIL: %"class.hlsl::ConsumeStructuredBuffer" = type { target("dx.RawBuffer", float, 1, 0) }
+// CHECK-DXIL: %"class.hlsl::RasterizerOrderedStructuredBuffer" = type { target("dx.RawBuffer", float, 1, 1) }
+
+// CHECK-SPIRV: %"class.hlsl::StructuredBuffer" = type { target("spirv.VulkanBuffer", [0 x float], 12, 0, 0) }
 
-// CHECK: %"class.hlsl::StructuredBuffer" = type { target("dx.RawBuffer", float, 0, 0) }
-// CHECK: %"class.hlsl::RWStructuredBuffer" = type { target("dx.RawBuffer", float, 1, 0) }
-// CHECK: %"class.hlsl::AppendStructuredBuffer" = type { target("dx.RawBuffer", float, 1, 0) }
-// CHECK: %"class.hlsl::ConsumeStructuredBuffer" = type { target("dx.RawBuffer", float, 1, 0) }
-// CHECK: %"class.hlsl::RasterizerOrderedStructuredBuffer" = type { target("dx.RawBuffer", float, 1, 1) }
 
-// CHECK: @_ZL3Buf = internal global %"class.hlsl::StructuredBuffer" poison, align 4
-// CHECK: @_ZL4Buf2 = internal global %"class.hlsl::RWStructuredBuffer" poison, align 4
-// CHECK: @_ZL4Buf3 = internal global %"class.hlsl::AppendStructuredBuffer" poison, align 4
-// CHECK: @_ZL4Buf4 = internal global %"class.hlsl::ConsumeStructuredBuffer" poison, align 4
-// CHECK: @_ZL4Buf5 = internal global %"class.hlsl::RasterizerOrderedStructuredBuffer" poison, align 4
+// CHECK: @_ZL3Buf = internal global %"class.hlsl::StructuredBuffer" poison
+// CHECK-DXIL: @_ZL4Buf2 = internal global %"class.hlsl::RWStructuredBuffer" poison, align 4
+// CHECK-DXIL: @_ZL4Buf3 = internal global %"class.hlsl::AppendStructuredBuffer" poison, align 4
+// CHECK-DXIL: @_ZL4Buf4 = internal global %"class.hlsl::ConsumeStructuredBuffer" poison, align 4
+// CHECK-DXIL: @_ZL4Buf5 = internal global %"class.hlsl::RasterizerOrderedStructuredBuffer" poison, align 4
 
 // CHECK: define internal void @_init_resource__ZL3Buf()
 // CHECK-DXIL: [[H:%.*]] = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_0_0t(i32 0, i32 10, i32 1, i32 0, i1 false)
 // CHECK-DXIL: store target("dx.RawBuffer", float, 0, 0) [[H]], ptr @_ZL3Buf, align 4
+// CHECK-SPIRV: [[H:%.*]] = call target("spirv.VulkanBuffer", [0 x float], 12, 0, 0) @llvm.spv.resource.handlefrombinding.tspirv.VulkanBuffer_a0f32_12_0_0t(i32 0, i32 10, i32 1, i32 0, i1 false)
+// CHECK-SPIRV: store target("spirv.VulkanBuffer", [0 x float], 12, 0, 0) [[H]], ptr @_ZL3Buf, align 8
 
-// CHECK: define internal void @_init_resource__ZL4Buf2()
+// CHECK-DXIL: define internal void @_init_resource__ZL4Buf2()
 // CHECK-DXIL: [[H:%.*]] = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 5, i32 1, i32 0, i1 false)
 // CHECK-DXIL: store target("dx.RawBuffer", float, 1, 0) [[H]], ptr @_ZL4Buf2, align 4
 
-// CHECK: define internal void @_init_resource__ZL4Buf3()
+// CHECK-DXIL: define internal void @_init_resource__ZL4Buf3()
 // CHECK-DXIL: [[H:%.*]] = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 3, i32 1, i32 0, i1 false)
 // CHECK-DXIL: store target("dx.RawBuffer", float, 1, 0) [[H]], ptr @_ZL4Buf3, align 4
 
-// CHECK: define internal void @_init_resource__ZL4Buf4()
+// CHECK-DXIL: define internal void @_init_resource__ZL4Buf4()
 // CHECK-DXIL: [[H:%.*]] = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 4, i32 1, i32 0, i1 false)
 // CHECK-DXIL: store target("dx.RawBuffer", float, 1, 0) [[H]], ptr @_ZL4Buf4, align 4
 
-// CHECK: define internal void @_init_resource__ZL4Buf5()
+// CHECK-DXIL: define internal void @_init_resource__ZL4Buf5()
 // CHECK-DXIL: [[H:%.*]] = call target("dx.RawBuffer", float, 1, 1) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_f32_1_1t(i32 2, i32 1, i32 1, i32 0, i1 false)
 // CHECK-DXIL: store target("dx.RawBuffer", float, 1, 1) [[H]], ptr @_ZL4Buf5, align 4
 
-// CHECK: define linkonce_odr void @_ZN4hlsl16StructuredBufferIfEC2Ev(ptr noundef nonnull align 4 dereferenceable(4) %this)
-// CHECK-NEXT: entry:
-// CHECK: define linkonce_odr void @_ZN4hlsl18RWStructuredBufferIfEC2Ev(ptr noundef nonnull align 4 dereferenceable(4) %this)
-// CHECK-NEXT: entry:
-// CHECK: define linkonce_odr void @_ZN4hlsl22AppendStructuredBufferIfEC2Ev(ptr noundef nonnull align 4 dereferenceable(4) %this)
-// CHECK-NEXT: entry:
-// CHECK: define linkonce_odr void @_ZN4hlsl23ConsumeStructuredBufferIfEC2Ev(ptr noundef nonnull align 4 dereferenceable(4) %this)
-// CHECK: define linkonce_odr void @_ZN4hlsl33RasterizerOrderedStructuredBufferIfEC2Ev(ptr noundef nonnull align 4 dereferenceable(4) %this)
+// CHECK: define linkonce_odr void @_ZN4hlsl16StructuredBufferIfEC2Ev(ptr noundef nonnull align {{[48]}} dereferenceable({{[48]}}) %this)
 // CHECK-NEXT: entry:
+// CHECK-DXIL: define linkonce_odr void @_ZN4hlsl18RWStructuredBufferIfEC2Ev(ptr noundef nonnull align 4 dereferenceable(4) %this)
+// CHECK-DXIL-NEXT: entry:
+// CHECK-DXIL: define linkonce_odr void @_ZN4hlsl22AppendStructuredBufferIfEC2Ev(ptr noundef nonnull align 4 dereferenceable(4) %this)
+// CHECK-DXIL-NEXT: entry:
+// CHECK-DXIL: define linkonce_odr void @_ZN4hlsl23ConsumeStructuredBufferIfEC2Ev(ptr noundef nonnull align 4 dereferenceable(4) %this)
+// CHECK-DXIL: define linkonce_odr void @_ZN4hlsl33RasterizerOrderedStructuredBufferIfEC2Ev(ptr noundef nonnull align 4 dereferenceable(4) %this)
+// CHECK-DXIL-NEXT: entry:
 
-// CHECK: define internal void @_GLOBAL__sub_I_StructuredBuffers_constructors.hlsl()
-// CHECK: call void @_init_resource__ZL3Buf()
-// CHECK: call void @_init_resource__ZL4Buf2()
-// CHECK: call void @_init_resource__ZL4Buf3()
-// CHECK: call void @_init_resource__ZL4Buf4()
-// CHECK: call void @_init_resource__ZL4Buf5()
+// CHECK: define {{.*}} void @_GLOBAL__sub_I_StructuredBuffers_constructors.hlsl()
+// CHECK: call {{.*}} @_init_resource__ZL3Buf()
+// CHECK-DXIL: call void @_init_resource__ZL4Buf2()
+// CHECK-DXIL: call void @_init_resource__ZL4Buf3()
+// CHECK-DXIL: call void @_init_resource__ZL4Buf4()
+// CHECK-DXIL: call void @_init_resource__ZL4Buf5()

@s-perron s-perron force-pushed the hlsl_structured_buffers branch from bca59de to d56c416 Compare March 19, 2025 15:21
@s-perron s-perron changed the title [HLSL] Add SPIR-V target type for RWStructuredBuffers [HLSL] Add SPIR-V target type for StructuredBuffers Mar 19, 2025
@s-perron s-perron marked this pull request as draft March 19, 2025 17:25
We add the hlsl_device address space to represent the device memory
space as defined in section 1.7.1.3 of the [HLSL
spec](https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf).

Fixes llvm#127075
This PR adds the target type that should be used for
RWStructuredBuffers. It does not handle ByteAddressBuffers yet.

For now all structs will be laid out using the standard C/C++ layout
rules. Other layout rules will be implemented later.
@s-perron s-perron force-pushed the hlsl_structured_buffers branch from d56c416 to e1f4778 Compare March 20, 2025 13:47
"not require layout information.");
llvm::ArrayType *RuntimeArrayType = llvm::ArrayType::get(ElemType, 0);

uint32_t StorageClass = /* StorageBuffer storage class */ 12;
Copy link
Contributor

Choose a reason for hiding this comment

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

There is SPIRV::StorageClass::StorageBuffer defined, and the TD files has the value 12 associated, so might be a way to extract the int from this enum

QualType AddrSpaceElemTy =
AST.getAddrSpaceQualType(ElemTy, LangAS::hlsl_device);
QualType ElemPtrTy = AST.getPointerType(AddrSpaceElemTy);
// QualType ReturnTy = (IsRef ? AST.getLValueReferenceType(ElemTy) : ElemTy);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove?

@s-perron s-perron closed this Mar 28, 2025
@s-perron s-perron deleted the hlsl_structured_buffers branch March 28, 2025 18:03
@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
clang:codegen IR generation bugs: mangling, exceptions, etc. 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.

3 participants