[HLSL] Buffer handle globals should not be constants #130231
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-directx @llvm/pr-subscribers-hlsl Author: Justin Bogner (bogner) ChangesIf these are constants their initializers will be removed by InstCombine. Change them to not be constants and initialize them with poison. Full diff: https://github.com/llvm/llvm-project/pull/130231.diff 6 Files Affected:
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index dc34653e8f497..4e14c6a3e72a2 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -31,7 +31,6 @@
#include "llvm/IR/Type.h"
#include "llvm/IR/Value.h"
#include "llvm/Support/Alignment.h"
-
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/FormatVariadic.h"
@@ -214,12 +213,12 @@ void CGHLSLRuntime::addBuffer(const HLSLBufferDecl *BufDecl) {
llvm::TargetExtType *TargetTy =
cast<llvm::TargetExtType>(convertHLSLSpecificType(
ResHandleTy, BufDecl->hasValidPackoffset() ? &Layout : nullptr));
- llvm::GlobalVariable *BufGV =
- new GlobalVariable(TargetTy, /*isConstant*/ true,
- GlobalValue::LinkageTypes::ExternalLinkage, nullptr,
- llvm::formatv("{0}{1}", BufDecl->getName(),
- BufDecl->isCBuffer() ? ".cb" : ".tb"),
- GlobalValue::NotThreadLocal);
+ llvm::GlobalVariable *BufGV = new GlobalVariable(
+ TargetTy, /*isConstant*/ false,
+ GlobalValue::LinkageTypes::ExternalLinkage, PoisonValue::get(TargetTy),
+ llvm::formatv("{0}{1}", BufDecl->getName(),
+ BufDecl->isCBuffer() ? ".cb" : ".tb"),
+ GlobalValue::NotThreadLocal);
CGM.getModule().insertGlobalVariable(BufGV);
// Add globals for constant buffer elements and create metadata nodes
diff --git a/clang/test/CodeGenHLSL/cbuffer.hlsl b/clang/test/CodeGenHLSL/cbuffer.hlsl
index 38093c6dfacd7..d70ce0aae7b64 100644
--- a/clang/test/CodeGenHLSL/cbuffer.hlsl
+++ b/clang/test/CodeGenHLSL/cbuffer.hlsl
@@ -31,7 +31,7 @@ cbuffer CBScalars : register(b1, space5) {
int64_t a8;
}
-// CHECK: @CBScalars.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CBScalars,
+// CHECK: @CBScalars.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CBScalars,
// CHECK-SAME: 56, 0, 8, 16, 24, 32, 36, 40, 48))
// CHECK: @a1 = external addrspace(2) global float, align 4
// CHECK: @a2 = external addrspace(2) global double, align 8
@@ -53,7 +53,7 @@ cbuffer CBVectors {
// FIXME: add a bool vectors after llvm-project/llvm#91639 is added
}
-// CHECK: @CBVectors.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CBVectors,
+// CHECK: @CBVectors.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CBVectors,
// CHECK-SAME: 136, 0, 16, 40, 48, 80, 96, 112))
// CHECK: @b1 = external addrspace(2) global <3 x float>, align 16
// CHECK: @b2 = external addrspace(2) global <3 x double>, align 32
@@ -74,7 +74,7 @@ cbuffer CBArrays : register(b2) {
bool c8[4];
}
-// CHECK: @CBArrays.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CBArrays,
+// CHECK: @CBArrays.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CBArrays,
// CHECK-SAME: 708, 0, 48, 112, 176, 224, 608, 624, 656))
// CHECK: @c1 = external addrspace(2) global [3 x float], align 4
// CHECK: @c2 = external addrspace(2) global [2 x <3 x double>], align 32
@@ -105,7 +105,7 @@ struct D {
Empty es;
};
-// CHECK: @CBStructs.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CBStructs,
+// CHECK: @CBStructs.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CBStructs,
// CHECK-SAME: 246, 0, 16, 32, 64, 144, 238, 240))
// CHECK: @a = external addrspace(2) global target("dx.Layout", %A, 8, 0), align 8
// CHECK: @b = external addrspace(2) global target("dx.Layout", %B, 14, 0, 8), align 8
@@ -129,7 +129,7 @@ struct Test {
float a, b;
};
-// CHECK: @CBMix.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CBMix,
+// CHECK: @CBMix.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CBMix,
// CHECK-SAME: 170, 0, 24, 32, 120, 128, 136, 144, 152, 160, 168))
// CHECK: @test = external addrspace(2) global [2 x target("dx.Layout", %Test, 8, 0, 4)], align 4
// CHECK: @f1 = external addrspace(2) global float, align 4
diff --git a/clang/test/CodeGenHLSL/cbuffer_and_namespaces.hlsl b/clang/test/CodeGenHLSL/cbuffer_and_namespaces.hlsl
index 393ca3825c638..7cbde19b67d1f 100644
--- a/clang/test/CodeGenHLSL/cbuffer_and_namespaces.hlsl
+++ b/clang/test/CodeGenHLSL/cbuffer_and_namespaces.hlsl
@@ -9,13 +9,13 @@
// CHECK: %"n0::n2::__cblayout_C" = type <{ float, target("dx.Layout", %"n0::Foo", 4, 0) }>
// CHECK: %"n0::Foo" = type <{ float }>
-// CHECK: @A.cb = external constant target("dx.CBuffer", target("dx.Layout", %"n0::n1::__cblayout_A", 4, 0))
+// CHECK: @A.cb = global target("dx.CBuffer", target("dx.Layout", %"n0::n1::__cblayout_A", 4, 0))
// CHECK: @_ZN2n02n11aE = external addrspace(2) global float, align 4
-// CHECK: @B.cb = external constant target("dx.CBuffer", target("dx.Layout", %"n0::__cblayout_B", 4, 0))
+// CHECK: @B.cb = global target("dx.CBuffer", target("dx.Layout", %"n0::__cblayout_B", 4, 0))
// CHECK: @_ZN2n01aE = external addrspace(2) global float, align 4
-// CHECK: @C.cb = external constant target("dx.CBuffer", target("dx.Layout", %"n0::n2::__cblayout_C", 20, 0, 16))
+// CHECK: @C.cb = global target("dx.CBuffer", target("dx.Layout", %"n0::n2::__cblayout_C", 20, 0, 16))
// CHECK: @_ZN2n02n21aE = external addrspace(2) global float, align 4
// CHECK: external addrspace(2) global target("dx.Layout", %"n0::Foo", 4, 0), align 4
diff --git a/clang/test/CodeGenHLSL/cbuffer_with_packoffset.hlsl b/clang/test/CodeGenHLSL/cbuffer_with_packoffset.hlsl
index 870593986a976..97200a30bbf0c 100644
--- a/clang/test/CodeGenHLSL/cbuffer_with_packoffset.hlsl
+++ b/clang/test/CodeGenHLSL/cbuffer_with_packoffset.hlsl
@@ -4,7 +4,7 @@
// CHECK: %__cblayout_CB = type <{ float, double, <2 x i32> }>
-// CHECK: @CB.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 176, 16, 168, 88))
+// CHECK: @CB.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 176, 16, 168, 88))
// CHECK: @a = external addrspace(2) global float, align 4
// CHECK: @b = external addrspace(2) global double, align 8
// CHECK: @c = external addrspace(2) global <2 x i32>, align 8
diff --git a/clang/test/CodeGenHLSL/cbuffer_with_static_global_and_function.hlsl b/clang/test/CodeGenHLSL/cbuffer_with_static_global_and_function.hlsl
index 99f40d8fc93d7..c9c3ee9c43f6e 100644
--- a/clang/test/CodeGenHLSL/cbuffer_with_static_global_and_function.hlsl
+++ b/clang/test/CodeGenHLSL/cbuffer_with_static_global_and_function.hlsl
@@ -3,7 +3,7 @@
// CHECK: %__cblayout_A = type <{ float }>
-// CHECK: @A.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_A, 4, 0))
+// CHECK: @A.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_A, 4, 0))
// CHECK: @a = external addrspace(2) global float, align 4
// CHECK-DAG: @_ZL1b = internal global float 3.000000e+00, align 4
// CHECK-NOT: @B.cb
diff --git a/clang/test/CodeGenHLSL/default_cbuffer.hlsl b/clang/test/CodeGenHLSL/default_cbuffer.hlsl
index c5176aa8466e4..82dc01eb09be2 100644
--- a/clang/test/CodeGenHLSL/default_cbuffer.hlsl
+++ b/clang/test/CodeGenHLSL/default_cbuffer.hlsl
@@ -4,7 +4,7 @@
// CHECK: %"__cblayout_$Globals" = type <{ float, float, target("dx.Layout", %__cblayout_S, 4, 0) }>
// CHECK: %__cblayout_S = type <{ float }>
-// CHECK-DAG: @"$Globals.cb" = external constant target("dx.CBuffer", target("dx.Layout", %"__cblayout_$Globals", 20, 0, 4, 16))
+// CHECK-DAG: @"$Globals.cb" = global target("dx.CBuffer", target("dx.Layout", %"__cblayout_$Globals", 20, 0, 4, 16))
// CHECK-DAG: @a = external addrspace(2) global float
// CHECK-DAG: @g = external addrspace(2) global float
// CHECK-DAG: @h = external addrspace(2) global target("dx.Layout", %__cblayout_S, 4, 0), align 4
|
|
@llvm/pr-subscribers-clang Author: Justin Bogner (bogner) ChangesIf these are constants their initializers will be removed by InstCombine. Change them to not be constants and initialize them with poison. Full diff: https://github.com/llvm/llvm-project/pull/130231.diff 6 Files Affected:
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index dc34653e8f497..4e14c6a3e72a2 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -31,7 +31,6 @@
#include "llvm/IR/Type.h"
#include "llvm/IR/Value.h"
#include "llvm/Support/Alignment.h"
-
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/FormatVariadic.h"
@@ -214,12 +213,12 @@ void CGHLSLRuntime::addBuffer(const HLSLBufferDecl *BufDecl) {
llvm::TargetExtType *TargetTy =
cast<llvm::TargetExtType>(convertHLSLSpecificType(
ResHandleTy, BufDecl->hasValidPackoffset() ? &Layout : nullptr));
- llvm::GlobalVariable *BufGV =
- new GlobalVariable(TargetTy, /*isConstant*/ true,
- GlobalValue::LinkageTypes::ExternalLinkage, nullptr,
- llvm::formatv("{0}{1}", BufDecl->getName(),
- BufDecl->isCBuffer() ? ".cb" : ".tb"),
- GlobalValue::NotThreadLocal);
+ llvm::GlobalVariable *BufGV = new GlobalVariable(
+ TargetTy, /*isConstant*/ false,
+ GlobalValue::LinkageTypes::ExternalLinkage, PoisonValue::get(TargetTy),
+ llvm::formatv("{0}{1}", BufDecl->getName(),
+ BufDecl->isCBuffer() ? ".cb" : ".tb"),
+ GlobalValue::NotThreadLocal);
CGM.getModule().insertGlobalVariable(BufGV);
// Add globals for constant buffer elements and create metadata nodes
diff --git a/clang/test/CodeGenHLSL/cbuffer.hlsl b/clang/test/CodeGenHLSL/cbuffer.hlsl
index 38093c6dfacd7..d70ce0aae7b64 100644
--- a/clang/test/CodeGenHLSL/cbuffer.hlsl
+++ b/clang/test/CodeGenHLSL/cbuffer.hlsl
@@ -31,7 +31,7 @@ cbuffer CBScalars : register(b1, space5) {
int64_t a8;
}
-// CHECK: @CBScalars.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CBScalars,
+// CHECK: @CBScalars.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CBScalars,
// CHECK-SAME: 56, 0, 8, 16, 24, 32, 36, 40, 48))
// CHECK: @a1 = external addrspace(2) global float, align 4
// CHECK: @a2 = external addrspace(2) global double, align 8
@@ -53,7 +53,7 @@ cbuffer CBVectors {
// FIXME: add a bool vectors after llvm-project/llvm#91639 is added
}
-// CHECK: @CBVectors.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CBVectors,
+// CHECK: @CBVectors.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CBVectors,
// CHECK-SAME: 136, 0, 16, 40, 48, 80, 96, 112))
// CHECK: @b1 = external addrspace(2) global <3 x float>, align 16
// CHECK: @b2 = external addrspace(2) global <3 x double>, align 32
@@ -74,7 +74,7 @@ cbuffer CBArrays : register(b2) {
bool c8[4];
}
-// CHECK: @CBArrays.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CBArrays,
+// CHECK: @CBArrays.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CBArrays,
// CHECK-SAME: 708, 0, 48, 112, 176, 224, 608, 624, 656))
// CHECK: @c1 = external addrspace(2) global [3 x float], align 4
// CHECK: @c2 = external addrspace(2) global [2 x <3 x double>], align 32
@@ -105,7 +105,7 @@ struct D {
Empty es;
};
-// CHECK: @CBStructs.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CBStructs,
+// CHECK: @CBStructs.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CBStructs,
// CHECK-SAME: 246, 0, 16, 32, 64, 144, 238, 240))
// CHECK: @a = external addrspace(2) global target("dx.Layout", %A, 8, 0), align 8
// CHECK: @b = external addrspace(2) global target("dx.Layout", %B, 14, 0, 8), align 8
@@ -129,7 +129,7 @@ struct Test {
float a, b;
};
-// CHECK: @CBMix.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CBMix,
+// CHECK: @CBMix.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CBMix,
// CHECK-SAME: 170, 0, 24, 32, 120, 128, 136, 144, 152, 160, 168))
// CHECK: @test = external addrspace(2) global [2 x target("dx.Layout", %Test, 8, 0, 4)], align 4
// CHECK: @f1 = external addrspace(2) global float, align 4
diff --git a/clang/test/CodeGenHLSL/cbuffer_and_namespaces.hlsl b/clang/test/CodeGenHLSL/cbuffer_and_namespaces.hlsl
index 393ca3825c638..7cbde19b67d1f 100644
--- a/clang/test/CodeGenHLSL/cbuffer_and_namespaces.hlsl
+++ b/clang/test/CodeGenHLSL/cbuffer_and_namespaces.hlsl
@@ -9,13 +9,13 @@
// CHECK: %"n0::n2::__cblayout_C" = type <{ float, target("dx.Layout", %"n0::Foo", 4, 0) }>
// CHECK: %"n0::Foo" = type <{ float }>
-// CHECK: @A.cb = external constant target("dx.CBuffer", target("dx.Layout", %"n0::n1::__cblayout_A", 4, 0))
+// CHECK: @A.cb = global target("dx.CBuffer", target("dx.Layout", %"n0::n1::__cblayout_A", 4, 0))
// CHECK: @_ZN2n02n11aE = external addrspace(2) global float, align 4
-// CHECK: @B.cb = external constant target("dx.CBuffer", target("dx.Layout", %"n0::__cblayout_B", 4, 0))
+// CHECK: @B.cb = global target("dx.CBuffer", target("dx.Layout", %"n0::__cblayout_B", 4, 0))
// CHECK: @_ZN2n01aE = external addrspace(2) global float, align 4
-// CHECK: @C.cb = external constant target("dx.CBuffer", target("dx.Layout", %"n0::n2::__cblayout_C", 20, 0, 16))
+// CHECK: @C.cb = global target("dx.CBuffer", target("dx.Layout", %"n0::n2::__cblayout_C", 20, 0, 16))
// CHECK: @_ZN2n02n21aE = external addrspace(2) global float, align 4
// CHECK: external addrspace(2) global target("dx.Layout", %"n0::Foo", 4, 0), align 4
diff --git a/clang/test/CodeGenHLSL/cbuffer_with_packoffset.hlsl b/clang/test/CodeGenHLSL/cbuffer_with_packoffset.hlsl
index 870593986a976..97200a30bbf0c 100644
--- a/clang/test/CodeGenHLSL/cbuffer_with_packoffset.hlsl
+++ b/clang/test/CodeGenHLSL/cbuffer_with_packoffset.hlsl
@@ -4,7 +4,7 @@
// CHECK: %__cblayout_CB = type <{ float, double, <2 x i32> }>
-// CHECK: @CB.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 176, 16, 168, 88))
+// CHECK: @CB.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 176, 16, 168, 88))
// CHECK: @a = external addrspace(2) global float, align 4
// CHECK: @b = external addrspace(2) global double, align 8
// CHECK: @c = external addrspace(2) global <2 x i32>, align 8
diff --git a/clang/test/CodeGenHLSL/cbuffer_with_static_global_and_function.hlsl b/clang/test/CodeGenHLSL/cbuffer_with_static_global_and_function.hlsl
index 99f40d8fc93d7..c9c3ee9c43f6e 100644
--- a/clang/test/CodeGenHLSL/cbuffer_with_static_global_and_function.hlsl
+++ b/clang/test/CodeGenHLSL/cbuffer_with_static_global_and_function.hlsl
@@ -3,7 +3,7 @@
// CHECK: %__cblayout_A = type <{ float }>
-// CHECK: @A.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_A, 4, 0))
+// CHECK: @A.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_A, 4, 0))
// CHECK: @a = external addrspace(2) global float, align 4
// CHECK-DAG: @_ZL1b = internal global float 3.000000e+00, align 4
// CHECK-NOT: @B.cb
diff --git a/clang/test/CodeGenHLSL/default_cbuffer.hlsl b/clang/test/CodeGenHLSL/default_cbuffer.hlsl
index c5176aa8466e4..82dc01eb09be2 100644
--- a/clang/test/CodeGenHLSL/default_cbuffer.hlsl
+++ b/clang/test/CodeGenHLSL/default_cbuffer.hlsl
@@ -4,7 +4,7 @@
// CHECK: %"__cblayout_$Globals" = type <{ float, float, target("dx.Layout", %__cblayout_S, 4, 0) }>
// CHECK: %__cblayout_S = type <{ float }>
-// CHECK-DAG: @"$Globals.cb" = external constant target("dx.CBuffer", target("dx.Layout", %"__cblayout_$Globals", 20, 0, 4, 16))
+// CHECK-DAG: @"$Globals.cb" = global target("dx.CBuffer", target("dx.Layout", %"__cblayout_$Globals", 20, 0, 4, 16))
// CHECK-DAG: @a = external addrspace(2) global float
// CHECK-DAG: @g = external addrspace(2) global float
// CHECK-DAG: @h = external addrspace(2) global target("dx.Layout", %__cblayout_S, 4, 0), align 4
|
hekota
reviewed
Mar 7, 2025
hekota
approved these changes
Mar 12, 2025
alsepkow
reviewed
Mar 12, 2025
| GlobalValue::NotThreadLocal); | ||
| llvm::GlobalVariable *BufGV = new GlobalVariable( | ||
| TargetTy, /*isConstant*/ false, | ||
| GlobalValue::LinkageTypes::ExternalLinkage, PoisonValue::get(TargetTy), |
There was a problem hiding this comment.
Why PoisonValue? Looking at the documentation it sounds like thats intended to be used for an erroneous operation. What am I missing?
There was a problem hiding this comment.
The global BufGV will be initialized with a resource handle based on resource bindings at the start of the entry function (or in module init function). A resource class with uninitialized handle is an error state, so poison is the right choice here.
s-perron
approved these changes
Mar 17, 2025
If these are constants their initializers will be removed by InstCombine. Change them to not be constants and initialize them with poison.
bogner
force-pushed
the
2025-03-06-cbuffer-handles
branch
from
669992b to
bf5e5b4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If these are constants their initializers will be removed by InstCombine. Change them to not be constants and initialize them with poison.