https://github.com/hekota created https://github.com/llvm/llvm-project/pull/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 >From 3594a7af4e21daa461cb928fc1119bd09cc114e9 Mon Sep 17 00:00:00 2001 From: Helena Kotas <heko...@microsoft.com> Date: Tue, 5 Aug 2025 22:27:42 -0700 Subject: [PATCH] [DirectX] Fix resource binding analysis incorrectly removing duplicates --- llvm/lib/Frontend/HLSL/HLSLBinding.cpp | 2 +- .../DXILPostOptimizationValidation.cpp | 8 +-- .../CodeGen/DirectX/Metadata/srv_metadata.ll | 45 +++++++++++------ .../CodeGen/DirectX/Metadata/uav_metadata.ll | 49 ++++++++++++------- 4 files changed, 66 insertions(+), 38 deletions(-) diff --git a/llvm/lib/Frontend/HLSL/HLSLBinding.cpp b/llvm/lib/Frontend/HLSL/HLSLBinding.cpp index d581311f22028..45391460354d0 100644 --- a/llvm/lib/Frontend/HLSL/HLSLBinding.cpp +++ b/llvm/lib/Frontend/HLSL/HLSLBinding.cpp @@ -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; diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index 398dcbb8d1737..9d995bc70a997 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -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, diff --git a/llvm/test/CodeGen/DirectX/Metadata/srv_metadata.ll b/llvm/test/CodeGen/DirectX/Metadata/srv_metadata.ll index abab5c9fb1166..5c9fb93d7f570 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/srv_metadata.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/srv_metadata.ll @@ -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]]} diff --git a/llvm/test/CodeGen/DirectX/Metadata/uav_metadata.ll b/llvm/test/CodeGen/DirectX/Metadata/uav_metadata.ll index 9893f8b9ea4d7..77f36dd40fd7c 100644 --- a/llvm/test/CodeGen/DirectX/Metadata/uav_metadata.ll +++ b/llvm/test/CodeGen/DirectX/Metadata/uav_metadata.ll @@ -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]+]]} _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
