-
Notifications
You must be signed in to change notification settings - Fork 15k
[HLSL] Allow completely unused cbuffers #164557
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
Conversation
We were checking for cbuffers where the global was removed, but if the buffer is completely unused the whole thing can be null.
|
@llvm/pr-subscribers-backend-directx Author: Justin Bogner (bogner) ChangesWe were checking for cbuffers where the global was removed, but if the buffer is completely unused the whole thing can be null. Full diff: https://github.com/llvm/llvm-project/pull/164557.diff 2 Files Affected:
diff --git a/llvm/lib/Frontend/HLSL/CBuffer.cpp b/llvm/lib/Frontend/HLSL/CBuffer.cpp
index 407b6ad6d5a7e..1e4a1a5020b0b 100644
--- a/llvm/lib/Frontend/HLSL/CBuffer.cpp
+++ b/llvm/lib/Frontend/HLSL/CBuffer.cpp
@@ -43,8 +43,13 @@ std::optional<CBufferMetadata> CBufferMetadata::get(Module &M) {
for (const MDNode *MD : CBufMD->operands()) {
assert(MD->getNumOperands() && "Invalid cbuffer metadata");
- auto *Handle = cast<GlobalVariable>(
- cast<ValueAsMetadata>(MD->getOperand(0))->getValue());
+ // For an unused cbuffer, the handle may have been optimizzd out
+ Metadata *OpMD = MD->getOperand(0);
+ if (!OpMD)
+ continue;
+
+ auto *Handle =
+ cast<GlobalVariable>(cast<ValueAsMetadata>(OpMD)->getValue());
CBufferMapping &Mapping = Result->Mappings.emplace_back(Handle);
for (int I = 1, E = MD->getNumOperands(); I < E; ++I) {
diff --git a/llvm/test/CodeGen/DirectX/CBufferAccess/unused.ll b/llvm/test/CodeGen/DirectX/CBufferAccess/unused.ll
new file mode 100644
index 0000000000000..8c0d82e43b4b1
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/CBufferAccess/unused.ll
@@ -0,0 +1,13 @@
+; RUN: opt -S -dxil-cbuffer-access -mtriple=dxil--shadermodel6.3-library %s | FileCheck %s
+; Check that we correctly ignore cbuffers that were nulled out by optimizations.
+
+%__cblayout_CB = type <{ float }>
+@CB.cb = local_unnamed_addr global target("dx.CBuffer", %__cblayout_CB) poison
+@x = external local_unnamed_addr addrspace(2) global float, align 4
+
+; CHECK-NOT: !hlsl.cbs =
+!hlsl.cbs = !{!0, !1, !2}
+
+!0 = !{ptr @CB.cb, ptr addrspace(2) @x}
+!1 = !{ptr @CB.cb, null}
+!2 = !{null, null}
|
|
@llvm/pr-subscribers-hlsl Author: Justin Bogner (bogner) ChangesWe were checking for cbuffers where the global was removed, but if the buffer is completely unused the whole thing can be null. Full diff: https://github.com/llvm/llvm-project/pull/164557.diff 2 Files Affected:
diff --git a/llvm/lib/Frontend/HLSL/CBuffer.cpp b/llvm/lib/Frontend/HLSL/CBuffer.cpp
index 407b6ad6d5a7e..1e4a1a5020b0b 100644
--- a/llvm/lib/Frontend/HLSL/CBuffer.cpp
+++ b/llvm/lib/Frontend/HLSL/CBuffer.cpp
@@ -43,8 +43,13 @@ std::optional<CBufferMetadata> CBufferMetadata::get(Module &M) {
for (const MDNode *MD : CBufMD->operands()) {
assert(MD->getNumOperands() && "Invalid cbuffer metadata");
- auto *Handle = cast<GlobalVariable>(
- cast<ValueAsMetadata>(MD->getOperand(0))->getValue());
+ // For an unused cbuffer, the handle may have been optimizzd out
+ Metadata *OpMD = MD->getOperand(0);
+ if (!OpMD)
+ continue;
+
+ auto *Handle =
+ cast<GlobalVariable>(cast<ValueAsMetadata>(OpMD)->getValue());
CBufferMapping &Mapping = Result->Mappings.emplace_back(Handle);
for (int I = 1, E = MD->getNumOperands(); I < E; ++I) {
diff --git a/llvm/test/CodeGen/DirectX/CBufferAccess/unused.ll b/llvm/test/CodeGen/DirectX/CBufferAccess/unused.ll
new file mode 100644
index 0000000000000..8c0d82e43b4b1
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/CBufferAccess/unused.ll
@@ -0,0 +1,13 @@
+; RUN: opt -S -dxil-cbuffer-access -mtriple=dxil--shadermodel6.3-library %s | FileCheck %s
+; Check that we correctly ignore cbuffers that were nulled out by optimizations.
+
+%__cblayout_CB = type <{ float }>
+@CB.cb = local_unnamed_addr global target("dx.CBuffer", %__cblayout_CB) poison
+@x = external local_unnamed_addr addrspace(2) global float, align 4
+
+; CHECK-NOT: !hlsl.cbs =
+!hlsl.cbs = !{!0, !1, !2}
+
+!0 = !{ptr @CB.cb, ptr addrspace(2) @x}
+!1 = !{ptr @CB.cb, null}
+!2 = !{null, null}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have an HLSL sample that produces this? I want to make sure I have a test for SPIR-V, but I'm not sure how to reproduce it.
I hit this with a shader who's entry point wasn't "main", but I hadn't specified a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Co-authored-by: Helena Kotas <hekotas@microsoft.com>
We were checking for cbuffers where the global was removed, but if the buffer is completely unused the whole thing can be null. --------- Co-authored-by: Helena Kotas <hekotas@microsoft.com>
We were checking for cbuffers where the global was removed, but if the buffer is completely unused the whole thing can be null. --------- Co-authored-by: Helena Kotas <hekotas@microsoft.com>
We were checking for cbuffers where the global was removed, but if the buffer is completely unused the whole thing can be null. --------- Co-authored-by: Helena Kotas <hekotas@microsoft.com>
We were checking for cbuffers where the global was removed, but if the buffer is completely unused the whole thing can be null. --------- Co-authored-by: Helena Kotas <hekotas@microsoft.com>
We were checking for cbuffers where the global was removed, but if the buffer is completely unused the whole thing can be null.