Skip to content

[HLSL] Buffer handle globals should not be constants #130231

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.

Sign up for GitHub

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 4 commits into from
Mar 20, 2025
Merged

[HLSL] Buffer handle globals should not be constants #130231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf5e5b4
[HLSL] Buffer handle globals should not be constants
bogner Mar 7, 2025
74d2200
fixup: update for new upstream tests
bogner Mar 20, 2025
7b1d605
fixup: two more tests
bogner Mar 20, 2025
59dd3be
fixup: initialize globals in test
bogner Mar 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions clang/lib/CodeGen/CGHLSLRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -245,12 +244,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),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why PoisonValue? Looking at the documentation it sounds like thats intended to be used for an erroneous operation. What am I missing?

Copy link
Member

Choose a reason for hiding this comment

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

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.

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
Expand Down
26 changes: 13 additions & 13 deletions clang/test/CodeGenHLSL/cbuffer.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
// CHECK: %__cblayout_CBScalars = type <{ float, double, half, i64, i32, i16, i32, i64 }>
// CHECK: %__cblayout_CBVectors = type <{ <3 x float>, <3 x double>, <2 x half>, <3 x i64>, <4 x i32>, <3 x i16>, <3 x i64> }>
// CHECK: %__cblayout_CBArrays = type <{ [3 x float], [2 x <3 x double>], [2 x [2 x half]], [3 x i64], [2 x [3 x [4 x <4 x i32>]]], [1 x i16], [2 x i64], [4 x i32] }>
// CHECK: %__cblayout_CBStructs = type <{ target("dx.Layout", %A, 8, 0), target("dx.Layout", %B, 14, 0, 8),
// CHECK-SAME: target("dx.Layout", %C, 24, 0, 16), [5 x target("dx.Layout", %A, 8, 0)],
// CHECK: %__cblayout_CBStructs = type <{ target("dx.Layout", %A, 8, 0), target("dx.Layout", %B, 14, 0, 8),
// CHECK-SAME: target("dx.Layout", %C, 24, 0, 16), [5 x target("dx.Layout", %A, 8, 0)],
// CHECK-SAME: target("dx.Layout", %__cblayout_D, 94, 0), half, <3 x i16> }>

// CHECK: %A = type <{ <2 x float> }>
Expand Down Expand Up @@ -39,7 +39,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
Expand All @@ -61,7 +61,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
Expand All @@ -82,7 +82,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
Expand Down Expand Up @@ -113,7 +113,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
Expand All @@ -137,7 +137,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
Expand All @@ -161,9 +161,9 @@ cbuffer CBMix {
float f7;
vector<double,1> f8;
uint16_t f9;
};
};

// CHECK: @CB_A.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB_A, 188, 0, 32, 76, 80, 120, 128, 144, 160, 182))
// CHECK: @CB_A.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CB_A, 188, 0, 32, 76, 80, 120, 128, 144, 160, 182))

cbuffer CB_A {
double B0[2];
Expand All @@ -177,7 +177,7 @@ cbuffer CB_A {
half3 B8;
}

// CHECK: @CB_B.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB_B, 94, 0, 88))
// CHECK: @CB_B.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CB_B, 94, 0, 88))
cbuffer CB_B {
double3 B9[3];
half3 B10;
Expand Down Expand Up @@ -212,7 +212,7 @@ struct G {
half C3;
};

// CHECK: @CB_C.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB_C, 400, 0, 16, 112, 128, 392))
// CHECK: @CB_C.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CB_C, 400, 0, 16, 112, 128, 392))
cbuffer CB_C {
int D0;
F D1;
Expand Down Expand Up @@ -254,10 +254,10 @@ void main() {
// CHECK: ![[CBVECTORS]] = !{ptr @CBVectors.cb, ptr addrspace(2) @b1, ptr addrspace(2) @b2, ptr addrspace(2) @b3, ptr addrspace(2) @b4,
// CHECK-SAME: ptr addrspace(2) @b5, ptr addrspace(2) @b6, ptr addrspace(2) @b7}

// CHECK: ![[CBARRAYS]] = !{ptr @CBArrays.cb, ptr addrspace(2) @c1, ptr addrspace(2) @c2, ptr addrspace(2) @c3, ptr addrspace(2) @c4,
// CHECK: ![[CBARRAYS]] = !{ptr @CBArrays.cb, ptr addrspace(2) @c1, ptr addrspace(2) @c2, ptr addrspace(2) @c3, ptr addrspace(2) @c4,
// CHECK-SAME: ptr addrspace(2) @c5, ptr addrspace(2) @c6, ptr addrspace(2) @c7, ptr addrspace(2) @c8}

// CHECK: ![[CBSTRUCTS]] = !{ptr @CBStructs.cb, ptr addrspace(2) @a, ptr addrspace(2) @b, ptr addrspace(2) @c, ptr addrspace(2) @array_of_A,
// CHECK: ![[CBSTRUCTS]] = !{ptr @CBStructs.cb, ptr addrspace(2) @a, ptr addrspace(2) @b, ptr addrspace(2) @c, ptr addrspace(2) @array_of_A,
// CHECK-SAME: ptr addrspace(2) @d, ptr addrspace(2) @e, ptr addrspace(2) @f}

// CHECK: ![[CBMIX]] = !{ptr @CBMix.cb, ptr addrspace(2) @test, ptr addrspace(2) @f1, ptr addrspace(2) @f2, ptr addrspace(2) @f3,
Expand Down
6 changes: 3 additions & 3 deletions clang/test/CodeGenHLSL/cbuffer_and_namespaces.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions clang/test/CodeGenHLSL/cbuffer_with_packoffset.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// CHECK: %__cblayout_CB = type <{ float, double, <2 x i32> }>
// CHECK: %__cblayout_CB_1 = type <{ float, <2 x float> }>

// 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
Expand All @@ -16,7 +16,7 @@ cbuffer CB : register(b1, space3) {
int2 c : packoffset(c5.z);
}

// CHECK: @CB.cb.1 = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB_1, 92, 88, 80))
// CHECK: @CB.cb.1 = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CB_1, 92, 88, 80))
// CHECK: @x = external addrspace(2) global float, align 4
// CHECK: @y = external addrspace(2) global <2 x float>, align 8

Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenHLSL/cbuffer_with_static_global_and_function.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenHLSL/default_cbuffer.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenHLSL/default_cbuffer_with_layout.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

// CHECK-DAG: @b = external addrspace(2) global float, align 4
// CHECK-DAG: @d = external addrspace(2) global <4 x i32>, align 16
// CHECK-DAG: @"$Globals.cb" = external constant target("dx.CBuffer",
// CHECK-DAG: @"$Globals.cb" = global target("dx.CBuffer",
// CHECK-DAG-SAME: target("dx.Layout", %"__cblayout_$Globals", 144, 120, 16, 32, 64, 128, 112))
// CHECK-DAG: @a = external addrspace(2) global i32, align 4
// CHECK-DAG: @c = external addrspace(2) global [4 x double], align 8
Expand Down
6 changes: 3 additions & 3 deletions llvm/test/CodeGen/DirectX/Metadata/cbuffer_metadata.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32
target triple = "dxil-pc-shadermodel6.6-compute"

%__cblayout_CB1 = type <{ float, i32, double, <2 x i32> }>
@CB1.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB1, 24, 0, 4, 8, 16))
@CB1.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CB1, 24, 0, 4, 8, 16)) poison

%__cblayout_CB2 = type <{ float, double, float, half, i16, i64, i32 }>
@CB2.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB2, 36, 0, 8, 16, 20, 22, 24, 32))
@CB2.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CB2, 36, 0, 8, 16, 20, 22, 24, 32)) poison

%__cblayout_CB3 = type <{ double, <3 x float>, float, <3 x double>, half, <2 x double>, float, <3 x half>, <3 x half> }>
@CB3.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB3, 96, 0, 16, 28, 32, 56, 64, 80, 84, 90))
@CB3.cb = global target("dx.CBuffer", target("dx.Layout", %__cblayout_CB3, 96, 0, 16, 28, 32, 56, 64, 80, 84, 90)) poison

; PRINT:; Resource Bindings:
; PRINT-NEXT:;
Expand Down
Loading