[DirectX] Detect resources with identical overlapping binding

[hekota]

This change uses resource name during DXIL resource binding analysis to detect when two (or more) resources have identical overlapping binding.

The DXIL resource analysis just detects that there is a problem with the binding and sets the hasOverlappingBinding flag. Full error reporting will happen later in DXILPostOptimizationValidation pass (#110723).

Depends on #139991

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-backend-directx

Author: Helena Kotas (hekota)

Changes

This change uses resource name during DXIL resource binding analysis to detect when two (or more) resources have identical overlapping binding.

The DXIL resource analysis just detects that there is a problem with the binding and sets the hasOverlappingBinding flag. Full error reporting will happen later in DXILPostOptimizationValidation pass (llvm/llvm-project#110723).

Depends on #139991

---

Full diff: https://github.com/llvm/llvm-project/pull/140645.diff

2 Files Affected:

• (modified) llvm/lib/Analysis/DXILResource.cpp (+9-8)
• (modified) llvm/unittests/Target/DirectX/ResourceBindingAnalysisTests.cpp (+5-7)

diff --git a/llvm/lib/Analysis/DXILResource.cpp b/llvm/lib/Analysis/DXILResource.cpp
index 36b3901246285..d0c1e1e9ba2d3 100644
--- a/llvm/lib/Analysis/DXILResource.cpp
+++ b/llvm/lib/Analysis/DXILResource.cpp
@@ -892,10 +892,11 @@ void DXILResourceBindingInfo::populate(Module &M, DXILResourceTypeMap &DRTM) {
     uint32_t Space;
     uint32_t LowerBound;
     uint32_t UpperBound;
+    Value *Name;
     Binding(ResourceClass RC, uint32_t Space, uint32_t LowerBound,
-            uint32_t UpperBound)
-        : RC(RC), Space(Space), LowerBound(LowerBound), UpperBound(UpperBound) {
-    }
+            uint32_t UpperBound, Value *Name)
+        : RC(RC), Space(Space), LowerBound(LowerBound), UpperBound(UpperBound),
+          Name(Name) {}
   };
   SmallVector<Binding> Bindings;
 
@@ -920,6 +921,7 @@ void DXILResourceBindingInfo::populate(Module &M, DXILResourceTypeMap &DRTM) {
               cast<ConstantInt>(CI->getArgOperand(1))->getZExtValue();
           int32_t Size =
               cast<ConstantInt>(CI->getArgOperand(2))->getZExtValue();
+          Value *Name = CI->getArgOperand(5);
 
           // negative size means unbounded resource array;
           // upper bound register overflow should be detected in Sema
@@ -927,7 +929,7 @@ void DXILResourceBindingInfo::populate(Module &M, DXILResourceTypeMap &DRTM) {
                  "upper bound register overflow");
           uint32_t UpperBound = Size < 0 ? UINT32_MAX : LowerBound + Size - 1;
           Bindings.emplace_back(RTI.getResourceClass(), Space, LowerBound,
-                                UpperBound);
+                                UpperBound, Name);
         }
       break;
     }
@@ -946,8 +948,9 @@ void DXILResourceBindingInfo::populate(Module &M, DXILResourceTypeMap &DRTM) {
 
   // remove duplicates
   Binding *NewEnd = llvm::unique(Bindings, [](auto &LHS, auto &RHS) {
-    return std::tie(LHS.RC, LHS.Space, LHS.LowerBound, LHS.UpperBound) ==
-           std::tie(RHS.RC, RHS.Space, RHS.LowerBound, RHS.UpperBound);
+    return std::tie(LHS.RC, LHS.Space, LHS.LowerBound, LHS.UpperBound,
+                    LHS.Name) == std::tie(RHS.RC, RHS.Space, RHS.LowerBound,
+                                          RHS.UpperBound, RHS.Name);
   });
   if (NewEnd != Bindings.end())
     Bindings.erase(NewEnd);
@@ -987,8 +990,6 @@ void DXILResourceBindingInfo::populate(Module &M, DXILResourceTypeMap &DRTM) {
       if (B.UpperBound < UINT32_MAX)
         S->FreeRanges.emplace_back(B.UpperBound + 1, UINT32_MAX);
     } else {
-      // FIXME: This only detects overlapping bindings that are not an exact
-      // match (llvm/llvm-project#110723)
       OverlappingBinding = true;
       if (B.UpperBound < UINT32_MAX)
         LastFreeRange.LowerBound =
diff --git a/llvm/unittests/Target/DirectX/ResourceBindingAnalysisTests.cpp b/llvm/unittests/Target/DirectX/ResourceBindingAnalysisTests.cpp
index 6cd0d9f4fc5e7..6cd1a48d8d5fd 100644
--- a/llvm/unittests/Target/DirectX/ResourceBindingAnalysisTests.cpp
+++ b/llvm/unittests/Target/DirectX/ResourceBindingAnalysisTests.cpp
@@ -167,7 +167,6 @@ TEST_F(ResourceBindingAnalysisTest, TestUnboundedAndOverlap) {
   // StructuredBuffer<float> C[]  : register(t0, space2);
   // StructuredBuffer<float> D    : register(t4, space2); /* overlapping */
   StringRef Assembly = R"(
-%__cblayout_CB = type <{ i32 }>
 define void @main() {
 entry:
   %handleA = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 5, i32 -1, i32 10, i1 false, ptr null)
@@ -198,11 +197,12 @@ TEST_F(ResourceBindingAnalysisTest, TestExactOverlap) {
   // StructuredBuffer<float> A  : register(t5);
   // StructuredBuffer<float> B  : register(t5);
   StringRef Assembly = R"(
-%__cblayout_CB = type <{ i32 }>
+@A.str = private unnamed_addr constant [2 x i8] c"A\00", align 1
+@B.str = private unnamed_addr constant [2 x i8] c"B\00", align 1
 define void @main() {
 entry:
-  %handleA = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 5, i32 1, i32 0, i1 false, ptr null)
-  %handleB = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 5, i32 1, i32 0, i1 false, ptr null)
+  %handleA = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 5, i32 1, i32 0, i1 false, ptr @A.str)
+  %handleB = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 5, i32 1, i32 0, i1 false, ptr @B.str)
   ret void
 }
   )";
@@ -213,9 +213,7 @@ define void @main() {
       MAM->getResult<DXILResourceBindingAnalysis>(*M);
 
   EXPECT_EQ(false, DRBI.hasImplicitBinding());
-  // FIXME (XFAIL): detecting overlap of two resource with identical binding
-  // is not yet supported (llvm/llvm-project#110723).
-  EXPECT_EQ(false, DRBI.hasOverlappingBinding());
+  EXPECT_EQ(true, DRBI.hasOverlappingBinding());
 
   DXILResourceBindingInfo::BindingSpaces &SRVSpaces =
       DRBI.getBindingSpaces(ResourceClass::SRV);

[pow2clk]

Looks elegantly simple in isolation. It's looking at some other changes that I wonder if there is some opportunity to consolidate binding representations, but that could be a clean up for later as they both already exist.