-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[DirectX] Detect resources with identical overlapping binding #140645
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.
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
base: users/hekota/pr139991-resource-names-llvm-intr
Are you sure you want to change the base?
[DirectX] Detect resources with identical overlapping binding #140645
Conversation
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#110723).
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-backend-directx Author: Helena Kotas (hekota) ChangesThis 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 Depends on #139991 Full diff: https://github.com/llvm/llvm-project/pull/140645.diff 2 Files Affected:
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);
|
@@ -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 }> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing %__cblayout_CB
because it is not used (copy & paste error from previous change). Same thing on line 201.
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