[DirectX] Fix resource binding analysis incorrectly removing duplicates (#152253)
The resource binding analysis was incorrectly reducing the size of the `Bindings` vector by one element after sorting and de-duplication. This led to an inaccurate setting of the `HasOverlappingBinding` flag in the `DXILResourceBindingInfo` analysis, as the truncated vector no longer reflected the true binding state. This update corrects the shrink logic and introduces an `assert` in the `DXILPostOptimizationValidation` pass. The assertion will trigger if `HasOverlappingBinding` is set but no corresponding error is detected, helping catch future inconsistencies. The bug surfaced when the `srv_metadata.hlsl` and `uav_metadata.hlsl` tests were updated to include unbounded resource arrays as part of https://github.com/llvm/llvm-project/issues/145422. These updated test files are included in this PR, as they would cause the new assertion to fire if the original issue remained unresolved. Depends on #152250
This commit is contained in:
Helena Kotas
GitHub
parent
049953fe8d
commit
fb1035cfb4
@ -76,7 +76,7 @@ BindingInfo BindingInfoBuilder::calculateBindingInfo(
|
||||
// remove duplicates
|
||||
Binding *NewEnd = llvm::unique(Bindings);
|
||||
if (NewEnd != Bindings.end())
|
||||
Bindings.erase(NewEnd);
|
||||
Bindings.erase(NewEnd, Bindings.end());
|
||||
|
||||
BindingInfo Info;
|
||||
|
||||
|
||||
@ -63,9 +63,7 @@ static void reportOverlappingError(Module &M, ResourceInfo R1,
|
||||
}
|
||||
|
||||
static void reportOverlappingBinding(Module &M, DXILResourceMap &DRM) {
|
||||
if (DRM.empty())
|
||||
return;
|
||||
|
||||
bool ErrorFound = false;
|
||||
for (const auto &ResList :
|
||||
{DRM.srvs(), DRM.uavs(), DRM.cbuffers(), DRM.samplers()}) {
|
||||
if (ResList.empty())
|
||||
@ -77,11 +75,15 @@ static void reportOverlappingBinding(Module &M, DXILResourceMap &DRM) {
|
||||
while (RI != ResList.end() &&
|
||||
PrevRI->getBinding().overlapsWith(RI->getBinding())) {
|
||||
reportOverlappingError(M, *PrevRI, *RI);
|
||||
ErrorFound = true;
|
||||
RI++;
|
||||
}
|
||||
PrevRI = CurrentRI;
|
||||
}
|
||||
}
|
||||
assert(ErrorFound && "this function should be called only when if "
|
||||
"DXILResourceBindingInfo::hasOverlapingBinding() is "
|
||||
"true, yet no overlapping binding was found");
|
||||
}
|
||||
|
||||
static void reportErrors(Module &M, DXILResourceMap &DRM,
|
||||
|
||||
@ -1,6 +1,6 @@
|
||||
; RUN: opt -S -dxil-translate-metadata < %s | FileCheck %s
|
||||
; RUN: opt -S --passes="dxil-pretty-printer" < %s 2>&1 | FileCheck %s --check-prefix=PRINT
|
||||
; RUN: llc %s --filetype=asm -o - < %s 2>&1 | FileCheck %s --check-prefixes=CHECK,PRINT
|
||||
; RUN: llc %s --filetype=asm -o - 2>&1 | FileCheck %s --check-prefixes=CHECK,PRINT
|
||||
|
||||
target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32-f64:64-n8:16:32:64"
|
||||
target triple = "dxil-pc-shadermodel6.6-compute"
|
||||
@ -14,20 +14,22 @@ target triple = "dxil-pc-shadermodel6.6-compute"
|
||||
@Six.str = private unnamed_addr constant [4 x i8] c"Six\00", align 1
|
||||
@Seven.str = private unnamed_addr constant [6 x i8] c"Seven\00", align 1
|
||||
@Array.str = private unnamed_addr constant [6 x i8] c"Array\00", align 1
|
||||
@Array2.str = private unnamed_addr constant [7 x i8] c"Array2\00", align 1
|
||||
|
||||
; PRINT:; Resource Bindings:
|
||||
; PRINT-NEXT:;
|
||||
; PRINT-NEXT:; Name Type Format Dim ID HLSL Bind Count
|
||||
; PRINT-NEXT:; ------------------------------ ---------- ------- ----------- ------- -------------- ------
|
||||
; PRINT-NEXT:; Zero texture f16 buf T0 t0 1
|
||||
; PRINT-NEXT:; One texture f32 buf T1 t1 1
|
||||
; PRINT-NEXT:; Two texture f64 buf T2 t2 1
|
||||
; PRINT-NEXT:; Three texture i32 buf T3 t3 1
|
||||
; PRINT-NEXT:; Four texture byte r/o T4 t5 1
|
||||
; PRINT-NEXT:; Five texture struct r/o T5 t6 1
|
||||
; PRINT-NEXT:; Six texture u64 buf T6 t10,space2 1
|
||||
; PRINT-NEXT:; Array texture f32 buf T7 t4,space3 100
|
||||
; PRINT-NEXT:; Seven texture u64 buf T8 t20,space5 1
|
||||
; PRINT-NEXT:; Name Type Format Dim ID HLSL Bind Count
|
||||
; PRINT-NEXT:; ------------------------------ ---------- ------- ----------- ------- -------------- ---------
|
||||
; PRINT-NEXT:; Zero texture f16 buf T0 t0 1
|
||||
; PRINT-NEXT:; One texture f32 buf T1 t1 1
|
||||
; PRINT-NEXT:; Two texture f64 buf T2 t2 1
|
||||
; PRINT-NEXT:; Three texture i32 buf T3 t3 1
|
||||
; PRINT-NEXT:; Four texture byte r/o T4 t5 1
|
||||
; PRINT-NEXT:; Five texture struct r/o T5 t6 1
|
||||
; PRINT-NEXT:; Six texture u64 buf T6 t10,space2 1
|
||||
; PRINT-NEXT:; Array texture f32 buf T7 t4,space3 100
|
||||
; PRINT-NEXT:; Array2 texture f64 buf T8 t2,space4 unbounded
|
||||
; PRINT-NEXT:; Seven texture u64 buf T9 t20,space5 1
|
||||
;
|
||||
|
||||
define void @test() #0 {
|
||||
@ -60,19 +62,28 @@ define void @test() #0 {
|
||||
@llvm.dx.resource.handlefrombinding(i32 2, i32 10, i32 1, i32 0, i1 false, ptr @Six.str)
|
||||
|
||||
; Same buffer type as Six - should have the same type in metadata
|
||||
; Buffer<double> Seven : register(t10, space2);
|
||||
; Buffer<double> Seven : register(t20, space5);
|
||||
%Seven_h = call target("dx.TypedBuffer", i64, 0, 0, 0)
|
||||
@llvm.dx.resource.handlefrombinding(i32 5, i32 20, i32 1, i32 0, i1 false, ptr @Seven.str)
|
||||
|
||||
; Buffer<float4> Array[100] : register(t4, space3);
|
||||
; Buffer<float4> B1 = Array[30];
|
||||
; Buffer<float4> B1 = Array[42];
|
||||
; Buffer<float4> B2 = Array[42];
|
||||
; resource array accesses should produce one metadata entry
|
||||
%Array_30_h = call target("dx.TypedBuffer", <4 x float>, 0, 0, 0)
|
||||
@llvm.dx.resource.handlefrombinding(i32 3, i32 4, i32 100, i32 30, i1 false, ptr @Array.str)
|
||||
%Array_42_h = call target("dx.TypedBuffer", <4 x float>, 0, 0, 0)
|
||||
@llvm.dx.resource.handlefrombinding(i32 3, i32 4, i32 100, i32 42, i1 false, ptr @Array.str)
|
||||
|
||||
; test unbounded resource array
|
||||
; Buffer<double> Array2[] : register(t2, space4);
|
||||
; Buffer<double> C1 = Array[10];
|
||||
; Buffer<double> C2 = Array[20];
|
||||
%Array2_10_h = call target("dx.TypedBuffer", double, 0, 0, 0)
|
||||
@llvm.dx.resource.handlefrombinding(i32 4, i32 2, i32 -1, i32 10, i1 false, ptr @Array2.str)
|
||||
%Array2_20_h = call target("dx.TypedBuffer", double, 0, 0, 0)
|
||||
@llvm.dx.resource.handlefrombinding(i32 4, i32 2, i32 -1, i32 20, i1 false, ptr @Array2.str)
|
||||
|
||||
ret void
|
||||
}
|
||||
|
||||
@ -95,6 +106,7 @@ attributes #0 = { noinline nounwind "hlsl.shader"="compute" }
|
||||
; CHECK: @Five = external constant %"StructuredBuffer<int16_t>"
|
||||
; CHECK: @Six = external constant %"Buffer<uint32_t>"
|
||||
; CHECK: @Array = external constant %"Buffer<float4>"
|
||||
; CHECK: @Array2 = external constant %"Buffer<double>"
|
||||
; CHECK: @Seven = external constant %"Buffer<uint32_t>"
|
||||
|
||||
; CHECK: !dx.resources = !{[[ResList:[!][0-9]+]]}
|
||||
@ -102,7 +114,7 @@ attributes #0 = { noinline nounwind "hlsl.shader"="compute" }
|
||||
; CHECK: [[ResList]] = !{[[SRVList:[!][0-9]+]], null, null, null}
|
||||
; CHECK: [[SRVList]] = !{![[Zero:[0-9]+]], ![[One:[0-9]+]], ![[Two:[0-9]+]],
|
||||
; CHECK-SAME: ![[Three:[0-9]+]], ![[Four:[0-9]+]], ![[Five:[0-9]+]],
|
||||
; CHECK-SAME: ![[Six:[0-9]+]], ![[Array:[0-9]+]], ![[Seven:[0-9]+]]}
|
||||
; CHECK-SAME: ![[Six:[0-9]+]], ![[Array:[0-9]+]], ![[Array2:[0-9]+]], ![[Seven:[0-9]+]]}
|
||||
|
||||
; CHECK: ![[Zero]] = !{i32 0, ptr @Zero, !"Zero", i32 0, i32 0, i32 1, i32 10, i32 0, ![[Half:[0-9]+]]}
|
||||
; CHECK: ![[Half]] = !{i32 0, i32 8}
|
||||
@ -118,4 +130,5 @@ attributes #0 = { noinline nounwind "hlsl.shader"="compute" }
|
||||
; CHECK: ![[Six]] = !{i32 6, ptr @Six, !"Six", i32 2, i32 10, i32 1, i32 10, i32 0, ![[U64:[0-9]+]]}
|
||||
; CHECK: ![[U64]] = !{i32 0, i32 7}
|
||||
; CHECK: ![[Array]] = !{i32 7, ptr @Array, !"Array", i32 3, i32 4, i32 100, i32 10, i32 0, ![[Float]]}
|
||||
; CHECK: ![[Seven]] = !{i32 8, ptr @Seven, !"Seven", i32 5, i32 20, i32 1, i32 10, i32 0, ![[U64]]}
|
||||
; CHECK: ![[Array2]] = !{i32 8, ptr @Array2, !"Array2", i32 4, i32 2, i32 -1, i32 10, i32 0, ![[Double]]}
|
||||
; CHECK: ![[Seven]] = !{i32 9, ptr @Seven, !"Seven", i32 5, i32 20, i32 1, i32 10, i32 0, ![[U64]]}
|
||||
|
||||
@ -1,6 +1,6 @@
|
||||
; RUN: opt -S -dxil-translate-metadata < %s | FileCheck %s
|
||||
; RUN: opt -S --passes="dxil-pretty-printer" < %s 2>&1 | FileCheck %s --check-prefix=PRINT
|
||||
; RUN: llc %s --filetype=asm -o - < %s 2>&1 | FileCheck %s --check-prefixes=CHECK,PRINT
|
||||
; RUN: llc %s --filetype=asm -o - 2>&1 | FileCheck %s --check-prefixes=CHECK,PRINT
|
||||
|
||||
target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32-f64:64-n8:16:32:64"
|
||||
target triple = "dxil-pc-shadermodel6.6-compute"
|
||||
@ -17,23 +17,25 @@ target triple = "dxil-pc-shadermodel6.6-compute"
|
||||
@Nine.str = private unnamed_addr constant [5 x i8] c"Nine\00", align 1
|
||||
@Ten.str = private unnamed_addr constant [4 x i8] c"Ten\00", align 1
|
||||
@Array.str = private unnamed_addr constant [6 x i8] c"Array\00", align 1
|
||||
@Array2.str = private unnamed_addr constant [7 x i8] c"Array2\00", align 1
|
||||
|
||||
; PRINT:; Resource Bindings:
|
||||
; PRINT-NEXT:;
|
||||
; PRINT-NEXT:; Name Type Format Dim ID HLSL Bind Count
|
||||
; PRINT-NEXT:; ------------------------------ ---------- ------- ----------- ------- -------------- ------
|
||||
; PRINT-NEXT:; Zero UAV f16 buf U0 u0 1
|
||||
; PRINT-NEXT:; One UAV f32 buf U1 u1 1
|
||||
; PRINT-NEXT:; Two UAV f64 buf U2 u2 1
|
||||
; PRINT-NEXT:; Three UAV i32 buf U3 u3 1
|
||||
; PRINT-NEXT:; Four UAV byte r/w U4 u5 1
|
||||
; PRINT-NEXT:; Five UAV struct r/w U5 u6 1
|
||||
; PRINT-NEXT:; Six UAV i32 buf U6 u7 1
|
||||
; PRINT-NEXT:; Seven UAV struct r/w U7 u8 1
|
||||
; PRINT-NEXT:; Eight UAV byte r/w U8 u9 1
|
||||
; PRINT-NEXT:; Nine UAV u64 buf U9 u10,space2 1
|
||||
; PRINT-NEXT:; Array UAV f32 buf U10 u4,space3 100
|
||||
; PRINT-NEXT:; Ten UAV u64 buf U11 u22,space5 1
|
||||
; PRINT-NEXT:; Name Type Format Dim ID HLSL Bind Count
|
||||
; PRINT-NEXT:; ------------------------------ ---------- ------- ----------- ------- -------------- ---------
|
||||
; PRINT-NEXT:; Zero UAV f16 buf U0 u0 1
|
||||
; PRINT-NEXT:; One UAV f32 buf U1 u1 1
|
||||
; PRINT-NEXT:; Two UAV f64 buf U2 u2 1
|
||||
; PRINT-NEXT:; Three UAV i32 buf U3 u3 1
|
||||
; PRINT-NEXT:; Four UAV byte r/w U4 u5 1
|
||||
; PRINT-NEXT:; Five UAV struct r/w U5 u6 1
|
||||
; PRINT-NEXT:; Six UAV i32 buf U6 u7 1
|
||||
; PRINT-NEXT:; Seven UAV struct r/w U7 u8 1
|
||||
; PRINT-NEXT:; Eight UAV byte r/w U8 u9 1
|
||||
; PRINT-NEXT:; Nine UAV u64 buf U9 u10,space2 1
|
||||
; PRINT-NEXT:; Array UAV f32 buf U10 u4,space3 100
|
||||
; PRINT-NEXT:; Array2 UAV f64 buf U11 u2,space4 unbounded
|
||||
; PRINT-NEXT:; Ten UAV u64 buf U12 u22,space5 1
|
||||
|
||||
define void @test() #0 {
|
||||
; RWBuffer<half4> Zero : register(u0)
|
||||
@ -78,13 +80,22 @@ define void @test() #0 {
|
||||
|
||||
; RWBuffer<float4> Array[100] : register(u4, space3);
|
||||
; RWBuffer<float4> B1 = Array[30];
|
||||
; RWBuffer<float4> B1 = Array[42];
|
||||
; RWBuffer<float4> B2 = Array[42];
|
||||
; resource array accesses should produce one metadata entry
|
||||
%Array_30_h = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
|
||||
@llvm.dx.resource.handlefrombinding(i32 3, i32 4, i32 100, i32 30, i1 false, ptr @Array.str)
|
||||
%Array_42_h = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
|
||||
@llvm.dx.resource.handlefrombinding(i32 3, i32 4, i32 100, i32 42, i1 false, ptr @Array.str)
|
||||
|
||||
; test unbounded resource array
|
||||
; RWBuffer<double> Array2[] : register(u2, space4);
|
||||
; RWBuffer<double> C1 = Array[10];
|
||||
; RWBuffer<double> C2 = Array[20];
|
||||
%Array2_10_h = call target("dx.TypedBuffer", double, 1, 0, 0)
|
||||
@llvm.dx.resource.handlefrombinding(i32 4, i32 2, i32 -1, i32 10, i1 false, ptr @Array2.str)
|
||||
%Array2_20_h = call target("dx.TypedBuffer", double, 1, 0, 0)
|
||||
@llvm.dx.resource.handlefrombinding(i32 4, i32 2, i32 -1, i32 20, i1 false, ptr @Array2.str)
|
||||
|
||||
; Same buffer type as Nine - should have the same type in metadata
|
||||
; RWBuffer<double> Ten : register(u2);
|
||||
%Ten_h = call target("dx.TypedBuffer", i64, 1, 0, 0)
|
||||
@ -118,6 +129,7 @@ attributes #0 = { noinline nounwind "hlsl.shader"="compute" }
|
||||
; CHECK: @Eight = external constant %RasterizerOrderedByteAddressBuffer
|
||||
; CHECK: @Nine = external constant %"RWBuffer<uint32_t>"
|
||||
; CHECK: @Array = external constant %"RWBuffer<float4>"
|
||||
; CHECK: @Array2 = external constant %"RWBuffer<double>"
|
||||
; CHECK: @Ten = external constant %"RWBuffer<uint32_t>"
|
||||
|
||||
; CHECK: !dx.resources = !{[[ResList:[!][0-9]+]]}
|
||||
@ -126,7 +138,7 @@ attributes #0 = { noinline nounwind "hlsl.shader"="compute" }
|
||||
; CHECK: [[UAVList]] = !{![[Zero:[0-9]+]], ![[One:[0-9]+]], ![[Two:[0-9]+]],
|
||||
; CHECK-SAME: ![[Three:[0-9]+]], ![[Four:[0-9]+]], ![[Five:[0-9]+]],
|
||||
; CHECK-SAME: ![[Six:[0-9]+]], ![[Seven:[0-9]+]], ![[Eight:[0-9]+]],
|
||||
; CHECK-SAME: ![[Nine:[0-9]+]], ![[Array:[0-9]+]], ![[Ten:[0-9]+]]}
|
||||
; CHECK-SAME: ![[Nine:[0-9]+]], ![[Array:[0-9]+]], ![[Array2:[0-9]+]], ![[Ten:[0-9]+]]}
|
||||
|
||||
; CHECK: ![[Zero]] = !{i32 0, ptr @Zero, !"Zero", i32 0, i32 0, i32 1, i32 10, i1 false, i1 false, i1 false, ![[Half:[0-9]+]]}
|
||||
; CHECK: ![[Half]] = !{i32 0, i32 8}
|
||||
@ -146,4 +158,5 @@ attributes #0 = { noinline nounwind "hlsl.shader"="compute" }
|
||||
; CHECK: ![[Nine]] = !{i32 9, ptr @Nine, !"Nine", i32 2, i32 10, i32 1, i32 10, i1 false, i1 false, i1 false, ![[U64:[0-9]+]]}
|
||||
; CHECK: ![[U64]] = !{i32 0, i32 7}
|
||||
; CHECK: ![[Array]] = !{i32 10, ptr @Array, !"Array", i32 3, i32 4, i32 100, i32 10, i1 false, i1 false, i1 false, ![[Float]]}
|
||||
; CHECK: ![[Ten]] = !{i32 11, ptr @Ten, !"Ten", i32 5, i32 22, i32 1, i32 10, i1 false, i1 false, i1 false, ![[U64:[0-9]+]]}
|
||||
; CHECK: ![[Array2]] = !{i32 11, ptr @Array2, !"Array2", i32 4, i32 2, i32 -1, i32 10, i1 false, i1 false, i1 false, ![[Double]]}
|
||||
; CHECK: ![[Ten]] = !{i32 12, ptr @Ten, !"Ten", i32 5, i32 22, i32 1, i32 10, i1 false, i1 false, i1 false, ![[U64:[0-9]+]]}
|
||||
|
||||
@ -273,3 +273,21 @@ TEST(HLSLBindingTest, TestFindAvailable) {
|
||||
// In an empty space we find the slot at the beginning.
|
||||
EXPECT_THAT(V, HasSpecificValue(0u));
|
||||
}
|
||||
|
||||
// Test that add duplicate bindings are correctly de-duplicated
|
||||
TEST(HLSLBindingTest, TestNoOverlapWithDuplicates) {
|
||||
hlsl::BindingInfoBuilder Builder;
|
||||
|
||||
// We add the same binding three times, and just use `nullptr` for the cookie
|
||||
// so that they should all be uniqued away.
|
||||
Builder.trackBinding(ResourceClass::SRV, /*Space=*/0, /*LowerBound=*/5,
|
||||
/*UpperBound=*/5, /*Cookie=*/nullptr);
|
||||
Builder.trackBinding(ResourceClass::SRV, /*Space=*/0, /*LowerBound=*/5,
|
||||
/*UpperBound=*/5, /*Cookie=*/nullptr);
|
||||
Builder.trackBinding(ResourceClass::SRV, /*Space=*/0, /*LowerBound=*/5,
|
||||
/*UpperBound=*/5, /*Cookie=*/nullptr);
|
||||
bool HasOverlap;
|
||||
hlsl::BindingInfo Info = Builder.calculateBindingInfo(HasOverlap);
|
||||
|
||||
EXPECT_FALSE(HasOverlap);
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user