Skip to content

[DirectX] Fix printing of DXIL cbuffer info #128698

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 1 commit into from
Feb 25, 2025

Conversation

bogner
Copy link
Contributor

@bogner bogner commented Feb 25, 2025

Make sure we're able to print cbuffer comments in a way that's compatible with DXC.

Fixes #128562

@llvmbot
Copy link
Member

llvmbot commented Feb 25, 2025

@llvm/pr-subscribers-backend-directx

Author: Justin Bogner (bogner)

Changes

Make sure we're able to print cbuffer comments in a way that's compatible with DXC.

Fixes #128562


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

2 Files Affected:

  • (modified) llvm/lib/Target/DirectX/DXILPrettyPrinter.cpp (+3)
  • (modified) llvm/test/CodeGen/DirectX/CreateHandleFromBinding.ll (+9-1)
diff --git a/llvm/lib/Target/DirectX/DXILPrettyPrinter.cpp b/llvm/lib/Target/DirectX/DXILPrettyPrinter.cpp
index ff690f2abe490..7255a9be06d51 100644
--- a/llvm/lib/Target/DirectX/DXILPrettyPrinter.cpp
+++ b/llvm/lib/Target/DirectX/DXILPrettyPrinter.cpp
@@ -164,6 +164,9 @@ struct FormatResourceDimension
     case dxil::ResourceKind::TypedBuffer:
       OS << "buf";
       break;
+    case dxil::ResourceKind::CBuffer:
+      OS << "NA";
+      break;
     case dxil::ResourceKind::RTAccelerationStructure:
       // TODO: dxc would print "ras" here. Can/should this happen?
       llvm_unreachable("RTAccelerationStructure printing is not implemented");
diff --git a/llvm/test/CodeGen/DirectX/CreateHandleFromBinding.ll b/llvm/test/CodeGen/DirectX/CreateHandleFromBinding.ll
index e0fcd4b2d9ac5..9919a3f2dbd25 100644
--- a/llvm/test/CodeGen/DirectX/CreateHandleFromBinding.ll
+++ b/llvm/test/CodeGen/DirectX/CreateHandleFromBinding.ll
@@ -9,6 +9,7 @@
 ; CHECK-PRETTY:        SRV     u32         buf      T3      t3,space5        24
 ; CHECK-PRETTY:        UAV     i32         buf      U0      u7,space2         1
 ; CHECK-PRETTY:        UAV     f32         buf      U1      u5,space3         1
+; CHECK-PRETTY:    cbuffer      NA          NA     CB0            cb0         1
 
 target triple = "dxil-pc-shadermodel6.6-compute"
 
@@ -63,6 +64,12 @@ define void @test_bindings() {
   ; CHECK: [[BUF5:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 217, %dx.types.ResBind { i32 7, i32 -1, i32 0, i8 0 }, i32 %[[IX]], i1 false) #[[#ATTR]]
   ; CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle [[BUF5]], %dx.types.ResourceProperties { i32 10, i32 1033 }) #[[#ATTR]]
 
+  ; cbuffer cb0 : register(b0) { int4 i; float4 f; }
+  %cb0 = call target("dx.CBuffer", target("dx.Layout", {<4 x i32>, <4 x float>}, 32, 0, 16))
+      @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 1, i32 0, i1 false)
+  ; CHECK: [[BUF6:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 217, %dx.types.ResBind { i32 0, i32 0, i32 0, i8 2 }, i32 0, i1 false) #[[#ATTR]]
+  ; CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle [[BUF6]], %dx.types.ResourceProperties { i32 13, i32 32 }) #[[#ATTR]]
+
   ret void
 }
 
@@ -72,8 +79,9 @@ define void @test_bindings() {
 ; contents of the metadata are tested elsewhere.
 ;
 ; CHECK: !dx.resources = !{[[RESMD:![0-9]+]]}
-; CHECK: [[RESMD]] = !{[[SRVMD:![0-9]+]], [[UAVMD:![0-9]+]], null, null}
+; CHECK: [[RESMD]] = !{[[SRVMD:![0-9]+]], [[UAVMD:![0-9]+]], [[CBUFMD:![0-9]+]], null}
 ; CHECK-DAG: [[SRVMD]] = !{!{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}}
 ; CHECK-DAG: [[UAVMD]] = !{!{{[0-9]+}}, !{{[0-9]+}}}
+; CHECK-DAG: [[CBUFMD]] = !{!{{[0-9]+}}}
 
 attributes #0 = { nocallback nofree nosync nounwind willreturn memory(none) }

@bogner bogner linked an issue Feb 25, 2025 that may be closed by this pull request
When some resource types were present, but not all of them, we were
ending up in a situation where we would fail to initialize the `FirstX`
variables and get incorrect iterators.

Fixes llvm#128560.
@bogner
Copy link
Contributor Author

bogner commented Feb 25, 2025

sorry for the mass ping - screwed up order of operations...

@bogner bogner merged commit fc655b1 into llvm:main Feb 25, 2025
14 of 21 checks passed
bogner added a commit that referenced this pull request Feb 26, 2025
Make sure we're able to print cbuffer comments in a way that's
compatible with DXC.

Fixes #128562

Note: This is a re-commit because I somehow managed to get a completely
empty commit the first time.
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Mar 3, 2025
Make sure we're able to print cbuffer comments in a way that's
compatible with DXC.

Fixes llvm#128562
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Mar 5, 2025
Make sure we're able to print cbuffer comments in a way that's
compatible with DXC.

Fixes llvm#128562
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Mar 6, 2025
Make sure we're able to print cbuffer comments in a way that's
compatible with DXC.

Fixes llvm#128562
@damyanp damyanp moved this to Closed in HLSL Support Apr 25, 2025
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DirectX] DXILPrettyPrinter fails to print cbuffers
5 participants