Skip to content

[DirectX] Calculate resource binding offsets using the lower bound #117303

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

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

bogner
Copy link
Contributor

@bogner bogner commented Nov 22, 2024

In the DXIL CreateHandle and CreateHandleFromBinding ops, resource bindings are
indexed from the beginning of the binding space, not from the binding itself.
Translate from an index into the binding to one from the beginning of the space
when lowering to these operations.

Resource bindings are indexed from the beginning of the binding space, not from
the binding itself. This means that we need to populate the index for non-array
resources with the same value as the slot number (and arrays, when we implement
them, will be the sum of the slot number and the access index).
@bogner bogner requested a review from hekota November 22, 2024 08:40
@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 Nov 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 22, 2024

@llvm/pr-subscribers-backend-directx
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Justin Bogner (bogner)

Changes

Resource bindings are indexed from the beginning of the binding space, not from the binding itself. This means that we need to populate the index for non-array resources with the same value as the slot number (and arrays, when we implement them, will be the sum of the slot number and the access index).


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

4 Files Affected:

  • (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+1-1)
  • (modified) clang/test/CodeGenHLSL/builtins/RWBuffer-constructor.hlsl (+1-1)
  • (modified) clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl (+10-10)
  • (added) clang/test/CodeGenHLSL/resource-bindings.hlsl (+19)
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 7ba0d615018181..b4e17466087b16 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -588,7 +588,7 @@ llvm::Function *CGHLSLRuntime::createResourceBindingInitFn() {
       auto *Slot = llvm::ConstantInt::get(CGM.IntTy, RBA->getSlotNumber());
       // FIXME: resource arrays are not yet implemented
       auto *Range = llvm::ConstantInt::get(CGM.IntTy, 1);
-      auto *Index = llvm::ConstantInt::get(CGM.IntTy, 0);
+      auto *Index = llvm::ConstantInt::get(CGM.IntTy, RBA->getSlotNumber());
       // FIXME: NonUniformResourceIndex bit is not yet implemented
       auto *NonUniform = llvm::ConstantInt::get(Int1Ty, false);
       llvm::Value *Args[] = {Space, Slot, Range, Index, NonUniform};
diff --git a/clang/test/CodeGenHLSL/builtins/RWBuffer-constructor.hlsl b/clang/test/CodeGenHLSL/builtins/RWBuffer-constructor.hlsl
index 3949f7b943cfe0..a820d782896aaf 100644
--- a/clang/test/CodeGenHLSL/builtins/RWBuffer-constructor.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/RWBuffer-constructor.hlsl
@@ -19,7 +19,7 @@ RWBuffer<float> Buf : register(u5, space3);
 
 // CHECK: define internal void @_init_resource_bindings() {
 // CHECK-NEXT: entry:
-// CHECK-DXIL-NEXT: %Buf_h = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.handle.fromBinding.tdx.TypedBuffer_f32_1_0_0t(i32 3, i32 5, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf_h = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.handle.fromBinding.tdx.TypedBuffer_f32_1_0_0t(i32 3, i32 5, i32 1, i32 5, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.TypedBuffer", float, 1, 0, 0) %Buf_h, ptr @Buf, align 4
 // CHECK-SPIRV-NEXT: %Buf_h = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.spv.handle.fromBinding.tdx.TypedBuffer_f32_1_0_0t(i32 3, i32 5, i32 1, i32 0, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.TypedBuffer", float, 1, 0, 0) %Buf_h, ptr @Buf, align 4
diff --git a/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl b/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl
index 2e141b9279fa61..d7478c2981a85d 100644
--- a/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl
@@ -37,24 +37,24 @@ RasterizerOrderedStructuredBuffer<float> Buf5 : register(u1, space2);
 
 // CHECK: define internal void @_init_resource_bindings() {
 // CHECK-NEXT: entry:
-// CHECK-DXIL-NEXT: %Buf_h = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_0_0t(i32 0, i32 10, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf_h = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_0_0t(i32 0, i32 10, i32 1, i32 10, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.RawBuffer", float, 0, 0) %Buf_h, ptr @Buf, align 4
-// CHECK-DXIL-NEXT: %Buf2_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 5, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf2_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 5, i32 1, i32 5, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.RawBuffer", float, 1, 0) %Buf2_h, ptr @Buf2, align 4
-// CHECK-DXIL-NEXT: %Buf3_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 3, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf3_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 3, i32 1, i32 3, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.RawBuffer", float, 1, 0) %Buf3_h, ptr @Buf3, align 4
-// CHECK-DXIL-NEXT: %Buf4_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 4, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf4_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 4, i32 1, i32 4, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.RawBuffer", float, 1, 0) %Buf4_h, ptr @Buf4, align 4
-// CHECK-DXIL-NEXT: %Buf5_h = call target("dx.RawBuffer", float, 1, 1) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_1t(i32 2, i32 1, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf5_h = call target("dx.RawBuffer", float, 1, 1) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_1t(i32 2, i32 1, i32 1, i32 1, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.RawBuffer", float, 1, 1) %Buf5_h, ptr @Buf5, align 4
 
-// CHECK-SPIRV-NEXT: %Buf_h = call target("dx.RawBuffer", float, 0, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_0_0t(i32 0, i32 10, i32 1, i32 0, i1 false)
+// CHECK-SPIRV-NEXT: %Buf_h = call target("dx.RawBuffer", float, 0, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_0_0t(i32 0, i32 10, i32 1, i32 10, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.RawBuffer", float, 0, 0) %Buf_h, ptr @Buf", align 4
-// CHECK-SPIRV-NEXT: %Buf2_h = call target("dx.RawBuffer", float, 1, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 5, i32 1, i32 0, i1 false)
+// CHECK-SPIRV-NEXT: %Buf2_h = call target("dx.RawBuffer", float, 1, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 5, i32 1, i32 5, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.RawBuffer", float, 1, 0) %Buf2_h, ptr @Buf2", align 4
-// CHECK-SPIRV-NEXT: %Buf3_h = call target("dx.RawBuffer", float, 0, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 3, i32 1, i32 0, i1 false)
+// CHECK-SPIRV-NEXT: %Buf3_h = call target("dx.RawBuffer", float, 0, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 3, i32 1, i32 3, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.RawBuffer", float, 0, 0) %Buf3_h, ptr @Buf3, align 4
-// CHECK-SPIRV-NEXT: %Buf4_h = call target("dx.RawBuffer", float, 1, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 4, i32 1, i32 0, i1 false)
+// CHECK-SPIRV-NEXT: %Buf4_h = call target("dx.RawBuffer", float, 1, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 4, i32 1, i32 4, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.RawBuffer", float, 1, 0) %Buf4_h, ptr @Buf4, align 4
-// CHECK-SPIRV-NEXT: %Buf5_h = call target("dx.RawBuffer", float, 1, 1) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_1t(i32 2, i32 1, i32 1, i32 0, i1 false)
+// CHECK-SPIRV-NEXT: %Buf5_h = call target("dx.RawBuffer", float, 1, 1) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_1t(i32 2, i32 1, i32 1, i32 1, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.RawBuffer", float, 1, 1) %Buf5_h, ptr @Buf5, align 4
diff --git a/clang/test/CodeGenHLSL/resource-bindings.hlsl b/clang/test/CodeGenHLSL/resource-bindings.hlsl
new file mode 100644
index 00000000000000..dfa65f3e6cc314
--- /dev/null
+++ b/clang/test/CodeGenHLSL/resource-bindings.hlsl
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple dxil--shadermodel6.6-compute -x hlsl -finclude-default-header -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: define internal void @_init_resource_bindings() {
+
+// CHECK: %U0S0_h = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0) @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_1_0_0t(i32 0, i32 0, i32 1, i32 0, i1 false)
+RWBuffer<float4> U0S0 : register(u0);
+
+// CHECK: %U5S3_h = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.handle.fromBinding.tdx.TypedBuffer_f32_1_0_0t(i32 3, i32 5, i32 1, i32 5, i1 false)
+RWBuffer<float> U5S3 : register(u5, space3);
+
+// CHECK: %T2S2_h = call target("dx.RawBuffer", i32, 0, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_i32_0_0t(i32 2, i32 2, i32 1, i32 2, i1 false)
+StructuredBuffer<int> T2S2 : register(t2, space2);
+struct S {
+  float4 f;
+  int i;
+};
+
+// CHECK: %T3S0_h = call target("dx.RawBuffer", %struct.S = type { <4 x float>, i32, [12 x i8] }, 0, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_s_struct.Ss_0_0t(i32 0, i32 3, i32 1, i32 3, i1 false)
+StructuredBuffer<S> T3S0 : register(t3);

@llvmbot
Copy link
Member

llvmbot commented Nov 22, 2024

@llvm/pr-subscribers-hlsl

Author: Justin Bogner (bogner)

Changes

Resource bindings are indexed from the beginning of the binding space, not from the binding itself. This means that we need to populate the index for non-array resources with the same value as the slot number (and arrays, when we implement them, will be the sum of the slot number and the access index).


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

4 Files Affected:

  • (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+1-1)
  • (modified) clang/test/CodeGenHLSL/builtins/RWBuffer-constructor.hlsl (+1-1)
  • (modified) clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl (+10-10)
  • (added) clang/test/CodeGenHLSL/resource-bindings.hlsl (+19)
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 7ba0d615018181..b4e17466087b16 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -588,7 +588,7 @@ llvm::Function *CGHLSLRuntime::createResourceBindingInitFn() {
       auto *Slot = llvm::ConstantInt::get(CGM.IntTy, RBA->getSlotNumber());
       // FIXME: resource arrays are not yet implemented
       auto *Range = llvm::ConstantInt::get(CGM.IntTy, 1);
-      auto *Index = llvm::ConstantInt::get(CGM.IntTy, 0);
+      auto *Index = llvm::ConstantInt::get(CGM.IntTy, RBA->getSlotNumber());
       // FIXME: NonUniformResourceIndex bit is not yet implemented
       auto *NonUniform = llvm::ConstantInt::get(Int1Ty, false);
       llvm::Value *Args[] = {Space, Slot, Range, Index, NonUniform};
diff --git a/clang/test/CodeGenHLSL/builtins/RWBuffer-constructor.hlsl b/clang/test/CodeGenHLSL/builtins/RWBuffer-constructor.hlsl
index 3949f7b943cfe0..a820d782896aaf 100644
--- a/clang/test/CodeGenHLSL/builtins/RWBuffer-constructor.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/RWBuffer-constructor.hlsl
@@ -19,7 +19,7 @@ RWBuffer<float> Buf : register(u5, space3);
 
 // CHECK: define internal void @_init_resource_bindings() {
 // CHECK-NEXT: entry:
-// CHECK-DXIL-NEXT: %Buf_h = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.handle.fromBinding.tdx.TypedBuffer_f32_1_0_0t(i32 3, i32 5, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf_h = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.handle.fromBinding.tdx.TypedBuffer_f32_1_0_0t(i32 3, i32 5, i32 1, i32 5, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.TypedBuffer", float, 1, 0, 0) %Buf_h, ptr @Buf, align 4
 // CHECK-SPIRV-NEXT: %Buf_h = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.spv.handle.fromBinding.tdx.TypedBuffer_f32_1_0_0t(i32 3, i32 5, i32 1, i32 0, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.TypedBuffer", float, 1, 0, 0) %Buf_h, ptr @Buf, align 4
diff --git a/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl b/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl
index 2e141b9279fa61..d7478c2981a85d 100644
--- a/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/StructuredBuffers-constructors.hlsl
@@ -37,24 +37,24 @@ RasterizerOrderedStructuredBuffer<float> Buf5 : register(u1, space2);
 
 // CHECK: define internal void @_init_resource_bindings() {
 // CHECK-NEXT: entry:
-// CHECK-DXIL-NEXT: %Buf_h = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_0_0t(i32 0, i32 10, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf_h = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_0_0t(i32 0, i32 10, i32 1, i32 10, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.RawBuffer", float, 0, 0) %Buf_h, ptr @Buf, align 4
-// CHECK-DXIL-NEXT: %Buf2_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 5, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf2_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 5, i32 1, i32 5, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.RawBuffer", float, 1, 0) %Buf2_h, ptr @Buf2, align 4
-// CHECK-DXIL-NEXT: %Buf3_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 3, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf3_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 3, i32 1, i32 3, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.RawBuffer", float, 1, 0) %Buf3_h, ptr @Buf3, align 4
-// CHECK-DXIL-NEXT: %Buf4_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 4, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf4_h = call target("dx.RawBuffer", float, 1, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 4, i32 1, i32 4, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.RawBuffer", float, 1, 0) %Buf4_h, ptr @Buf4, align 4
-// CHECK-DXIL-NEXT: %Buf5_h = call target("dx.RawBuffer", float, 1, 1) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_1t(i32 2, i32 1, i32 1, i32 0, i1 false)
+// CHECK-DXIL-NEXT: %Buf5_h = call target("dx.RawBuffer", float, 1, 1) @llvm.dx.handle.fromBinding.tdx.RawBuffer_f32_1_1t(i32 2, i32 1, i32 1, i32 1, i1 false)
 // CHECK-DXIL-NEXT: store target("dx.RawBuffer", float, 1, 1) %Buf5_h, ptr @Buf5, align 4
 
-// CHECK-SPIRV-NEXT: %Buf_h = call target("dx.RawBuffer", float, 0, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_0_0t(i32 0, i32 10, i32 1, i32 0, i1 false)
+// CHECK-SPIRV-NEXT: %Buf_h = call target("dx.RawBuffer", float, 0, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_0_0t(i32 0, i32 10, i32 1, i32 10, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.RawBuffer", float, 0, 0) %Buf_h, ptr @Buf", align 4
-// CHECK-SPIRV-NEXT: %Buf2_h = call target("dx.RawBuffer", float, 1, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 5, i32 1, i32 0, i1 false)
+// CHECK-SPIRV-NEXT: %Buf2_h = call target("dx.RawBuffer", float, 1, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 1, i32 5, i32 1, i32 5, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.RawBuffer", float, 1, 0) %Buf2_h, ptr @Buf2", align 4
-// CHECK-SPIRV-NEXT: %Buf3_h = call target("dx.RawBuffer", float, 0, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 3, i32 1, i32 0, i1 false)
+// CHECK-SPIRV-NEXT: %Buf3_h = call target("dx.RawBuffer", float, 0, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 3, i32 1, i32 3, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.RawBuffer", float, 0, 0) %Buf3_h, ptr @Buf3, align 4
-// CHECK-SPIRV-NEXT: %Buf4_h = call target("dx.RawBuffer", float, 1, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 4, i32 1, i32 0, i1 false)
+// CHECK-SPIRV-NEXT: %Buf4_h = call target("dx.RawBuffer", float, 1, 0) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_0t(i32 0, i32 4, i32 1, i32 4, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.RawBuffer", float, 1, 0) %Buf4_h, ptr @Buf4, align 4
-// CHECK-SPIRV-NEXT: %Buf5_h = call target("dx.RawBuffer", float, 1, 1) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_1t(i32 2, i32 1, i32 1, i32 0, i1 false)
+// CHECK-SPIRV-NEXT: %Buf5_h = call target("dx.RawBuffer", float, 1, 1) @llvm.spv.handle.fromBinding.tdx.RawBuffer_f32_1_1t(i32 2, i32 1, i32 1, i32 1, i1 false)
 // CHECK-SPIRV-NEXT: store target("dx.RawBuffer", float, 1, 1) %Buf5_h, ptr @Buf5, align 4
diff --git a/clang/test/CodeGenHLSL/resource-bindings.hlsl b/clang/test/CodeGenHLSL/resource-bindings.hlsl
new file mode 100644
index 00000000000000..dfa65f3e6cc314
--- /dev/null
+++ b/clang/test/CodeGenHLSL/resource-bindings.hlsl
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple dxil--shadermodel6.6-compute -x hlsl -finclude-default-header -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: define internal void @_init_resource_bindings() {
+
+// CHECK: %U0S0_h = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0) @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_1_0_0t(i32 0, i32 0, i32 1, i32 0, i1 false)
+RWBuffer<float4> U0S0 : register(u0);
+
+// CHECK: %U5S3_h = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.handle.fromBinding.tdx.TypedBuffer_f32_1_0_0t(i32 3, i32 5, i32 1, i32 5, i1 false)
+RWBuffer<float> U5S3 : register(u5, space3);
+
+// CHECK: %T2S2_h = call target("dx.RawBuffer", i32, 0, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_i32_0_0t(i32 2, i32 2, i32 1, i32 2, i1 false)
+StructuredBuffer<int> T2S2 : register(t2, space2);
+struct S {
+  float4 f;
+  int i;
+};
+
+// CHECK: %T3S0_h = call target("dx.RawBuffer", %struct.S = type { <4 x float>, i32, [12 x i8] }, 0, 0) @llvm.dx.handle.fromBinding.tdx.RawBuffer_s_struct.Ss_0_0t(i32 0, i32 3, i32 1, i32 3, i1 false)
+StructuredBuffer<S> T3S0 : register(t3);

@@ -588,7 +588,7 @@ llvm::Function *CGHLSLRuntime::createResourceBindingInitFn() {
auto *Slot = llvm::ConstantInt::get(CGM.IntTy, RBA->getSlotNumber());
// FIXME: resource arrays are not yet implemented
auto *Range = llvm::ConstantInt::get(CGM.IntTy, 1);
auto *Index = llvm::ConstantInt::get(CGM.IntTy, 0);
auto *Index = llvm::ConstantInt::get(CGM.IntTy, RBA->getSlotNumber());
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not the way I implemented this for SPIR-V. This assumes the Slot and Index as interwined, which they are in DXIL, but not in SPIR-V.

In SPIR-V, the Slot is the binding number, and all elements of the resource array are accessed through the same binding number. The element does not get their own binding number. The binding and index are independent.

Would it be possible to keep this as is, and have the DXIL backend add the slot number to the index?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be better to just diverge this by the target. That way the intermediate IR reflects the index correctly.

Copy link
Contributor

@s-perron s-perron Nov 22, 2024

Choose a reason for hiding this comment

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

Diverging is fine. We will have to diverge when identifying the slots anyway. Because we keep samplers and textures in the same "space", we will need a more complicated way of determining the right space and slot. Plus we will eventually add the vk::binding attribute which will be another consideration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we opt to diverge this by target we should probably not use the matching intrinsic names between dxil and spirv in this case, as that could end up confusing. It might be best to defer that until we're dealing with textures or the binding attribute so we can make sure we're making the right decisions on how to split it up.

I think it would be fine to just do this indexing in the backend, since any index that's less than the slot should be invalid anyway. I'll try that out and report back.

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've changed this to do the translation in the backend.

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.

This change looks right for DXIL (and I'll approve in that context), but @s-perron's comment about SPIR-V is something we should address before merging.

@bogner bogner changed the title [HLSL] Get the index for resource bindings from the slot [DirectX] Calculate resource binding offsets using the lower bound Nov 22, 2024
@bogner bogner requested a review from llvm-beanz November 22, 2024 22:10
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.

This approach makes sense to me.

@bogner bogner merged commit bb88fd1 into llvm:main Nov 25, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:DirectX clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants