Skip to content

Commit bca1bf3

Browse files
hekotahekota
committed
code review feedback - rename flags, remove unnecessary functions, do not initialize space0
1 parent 9a70f06 commit bca1bf3

File tree

3 files changed

+30
-54
lines changed

3 files changed

+30
-54
lines changed

‎llvm/include/llvm/Analysis/DXILResource.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -637,10 +637,7 @@ class DXILResourceBindingInfo {
637637
struct BindingSpaces {
638638
dxil::ResourceClass ResClass;
639639
llvm::SmallVector<RegisterSpace> Spaces;
640-
BindingSpaces(dxil::ResourceClass ResClass) : ResClass(ResClass) {
641-
// initialize space0
642-
Spaces.emplace_back(0);
643-
}
640+
BindingSpaces(dxil::ResourceClass ResClass) : ResClass(ResClass) {}
644641
};
645642

646643
private:
@@ -660,8 +657,8 @@ class DXILResourceBindingInfo {
660657
SamplerSpaces(dxil::ResourceClass::Sampler), ImplicitBinding(false),
661658
OverlappingBinding(false) {}
662659

663-
bool containsImplicitBinding() const { return ImplicitBinding; }
664-
bool containsOverlappingBinding() const { return OverlappingBinding; }
660+
bool hasImplicitBinding() const { return ImplicitBinding; }
661+
bool hasOverlappingBinding() const { return OverlappingBinding; }
665662

666663
BindingSpaces &getBindingSpaces(dxil::ResourceClass RC) {
667664
switch (RC) {

‎llvm/lib/Analysis/DXILResource.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -927,8 +927,7 @@ void DXILResourceBindingInfo::populate(Module &M, DXILResourceTypeMap &DRTM) {
927927
break;
928928
}
929929
case Intrinsic::dx_resource_handlefromimplicitbinding: {
930-
if (!F.user_empty())
931-
ImplicitBinding = true;
930+
ImplicitBinding = true;
932931
break;
933932
}
934933
}
@@ -957,7 +956,8 @@ void DXILResourceBindingInfo::populate(Module &M, DXILResourceTypeMap &DRTM) {
957956
// move to the next resource class spaces
958957
BS = &getBindingSpaces(B.ResClass);
959958

960-
RegisterSpace *S = &BS->Spaces.back();
959+
RegisterSpace *S = BS->Spaces.empty() ? &BS->Spaces.emplace_back(B.Space)
960+
: &BS->Spaces.back();
961961
assert(S->Space <= B.Space && "bindings not sorted correctly?");
962962
if (B.Space != S->Space)
963963
// add new space
@@ -983,6 +983,8 @@ void DXILResourceBindingInfo::populate(Module &M, DXILResourceTypeMap &DRTM) {
983983
if (B.UpperBound < UINT32_MAX)
984984
S->FreeRanges.emplace_back(B.UpperBound + 1, UINT32_MAX);
985985
} else {
986+
// FIXME: This only detects overlapping bindings that are not an exact
987+
// match (llvm/llvm-project#110723)
986988
OverlappingBinding = true;
987989
if (B.UpperBound < UINT32_MAX)
988990
LastFreeRange.LowerBound =

‎llvm/unittests/Target/DirectX/ResourceBindingAnalysisTests.cpp

Lines changed: 22 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,17 @@ TEST_F(ResourceBindingAnalysisTest, TestTrivialCase) {
6565
define void @main() {
6666
entry:
6767
%handle = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 5, i32 1, i32 0, i1 false)
68-
69-
call void @a.func(target("dx.TypedBuffer", float, 1, 0, 0) %handle)
7068
ret void
7169
}
72-
73-
declare void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
7470
)";
7571

7672
auto M = parseAsm(Assembly);
7773

7874
DXILResourceBindingInfo &DRBI =
7975
MAM->getResult<DXILResourceBindingAnalysis>(*M);
8076

81-
EXPECT_EQ(false, DRBI.containsImplicitBinding());
82-
EXPECT_EQ(false, DRBI.containsOverlappingBinding());
77+
EXPECT_EQ(false, DRBI.hasImplicitBinding());
78+
EXPECT_EQ(false, DRBI.hasOverlappingBinding());
8379

8480
// check that UAV has exactly one gap
8581
DXILResourceBindingInfo::BindingSpaces &UAVSpaces =
@@ -94,8 +90,7 @@ declare void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
9490
{ResourceClass::SRV, ResourceClass::CBuffer, ResourceClass::Sampler}) {
9591
DXILResourceBindingInfo::BindingSpaces &Spaces = DRBI.getBindingSpaces(RC);
9692
EXPECT_EQ(Spaces.ResClass, RC);
97-
EXPECT_EQ(Spaces.Spaces.size(), 1u);
98-
checkExpectedSpaceAndFreeRanges(Spaces.Spaces[0], 0, {0, UINT32_MAX});
93+
EXPECT_EQ(Spaces.Spaces.size(), 0u);
9994
}
10095
}
10196

@@ -116,27 +111,17 @@ define void @main() {
116111
%handleC = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 5, i32 1, i32 0, i1 false)
117112
%handleD = call target("dx.RawBuffer", i32, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 5, i32 4, i1 false)
118113
%handleE = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 2, i32 2, i32 0, i1 false)
119-
120-
call void @a.func(target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 4, 0)) %handleCB)
121-
call void @a.func(target("dx.TypedBuffer", float, 1, 0, 0) %handleA)
122-
call void @a.func(target("dx.TypedBuffer", float, 1, 0, 0) %handleC)
123-
call void @a.func(target("dx.TypedBuffer", float, 1, 0, 0) %handleE)
124-
call void @a.func(target("dx.RawBuffer", i32, 0, 0) %handleB)
125-
call void @a.func(target("dx.RawBuffer", i32, 0, 0) %handleD)
126-
127114
ret void
128115
}
129-
130-
declare void @a.func(target("dx.RawBuffer", float, 1, 0) %handle)
131116
)";
132117

133118
auto M = parseAsm(Assembly);
134119

135120
DXILResourceBindingInfo &DRBI =
136121
MAM->getResult<DXILResourceBindingAnalysis>(*M);
137122

138-
EXPECT_EQ(false, DRBI.containsImplicitBinding());
139-
EXPECT_EQ(false, DRBI.containsOverlappingBinding());
123+
EXPECT_EQ(false, DRBI.hasImplicitBinding());
124+
EXPECT_EQ(false, DRBI.hasOverlappingBinding());
140125

141126
DXILResourceBindingInfo::BindingSpaces &SRVSpaces =
142127
DRBI.getBindingSpaces(ResourceClass::SRV);
@@ -174,25 +159,17 @@ define void @main() {
174159
%handleB = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 3, i32 0, i1 false)
175160
%handleC = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 2, i32 0, i32 -1, i32 100, i1 false)
176161
%handleD = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 2, i32 4, i32 1, i32 0, i1 false)
177-
178-
call void @a.func(target("dx.RawBuffer", float, 0, 0) %handleA)
179-
call void @a.func(target("dx.RawBuffer", float, 0, 0) %handleB)
180-
call void @a.func(target("dx.RawBuffer", float, 0, 0) %handleC)
181-
call void @a.func(target("dx.RawBuffer", float, 0, 0) %handleD)
182-
183162
ret void
184163
}
185-
186-
declare void @a.func(target("dx.RawBuffer", float, 0, 0) %handle)
187164
)";
188165

189166
auto M = parseAsm(Assembly);
190167

191168
DXILResourceBindingInfo &DRBI =
192169
MAM->getResult<DXILResourceBindingAnalysis>(*M);
193170

194-
EXPECT_EQ(false, DRBI.containsImplicitBinding());
195-
EXPECT_EQ(true, DRBI.containsOverlappingBinding());
171+
EXPECT_EQ(false, DRBI.hasImplicitBinding());
172+
EXPECT_EQ(true, DRBI.hasOverlappingBinding());
196173

197174
DXILResourceBindingInfo::BindingSpaces &SRVSpaces =
198175
DRBI.getBindingSpaces(ResourceClass::SRV);
@@ -215,24 +192,17 @@ define void @main() {
215192
%handleA = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 -1, i32 1, i32 0, i1 false)
216193
%handleB = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding(i32 1, i32 -10, i32 10, i32 50, i1 false)
217194
%handleC = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding(i32 2, i32 2147483647, i32 10, i32 100, i1 false)
218-
219-
call void @a.func(target("dx.TypedBuffer", float, 1, 0, 0) %handleA)
220-
call void @a.func(target("dx.TypedBuffer", float, 1, 0, 0) %handleB)
221-
call void @a.func(target("dx.TypedBuffer", float, 1, 0, 0) %handleC)
222-
223195
ret void
224196
}
225-
226-
declare void @a.func(target("dx.TypedBuffer", float, 1, 0, 0) %handle)
227197
)";
228198

229199
auto M = parseAsm(Assembly);
230200

231201
DXILResourceBindingInfo &DRBI =
232202
MAM->getResult<DXILResourceBindingAnalysis>(*M);
233203

234-
EXPECT_EQ(false, DRBI.containsImplicitBinding());
235-
EXPECT_EQ(false, DRBI.containsOverlappingBinding());
204+
EXPECT_EQ(false, DRBI.hasImplicitBinding());
205+
EXPECT_EQ(false, DRBI.hasOverlappingBinding());
236206

237207
DXILResourceBindingInfo::BindingSpaces &UAVSpaces =
238208
DRBI.getBindingSpaces(ResourceClass::UAV);
@@ -246,23 +216,30 @@ declare void @a.func(target("dx.TypedBuffer", float, 1, 0, 0) %handle)
246216
}
247217

248218
TEST_F(ResourceBindingAnalysisTest, TestImplicitFlag) {
249-
// RWBuffer<float> A;
219+
// RWBuffer<float> A : register(u5, space100);
220+
// RWBuffer<float> B;
250221
StringRef Assembly = R"(
251-
%__cblayout_CB = type <{ i32 }>
252222
define void @main() {
253223
entry:
254-
%handleA = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefromimplicitbinding(i32 0, i32 0, i32 1, i32 0)
255-
call void @a.func(target("dx.TypedBuffer", float, 1, 0, 0) %handleA)
224+
%handleA = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding(i32 100, i32 5, i32 1, i32 0, i1 false)
225+
%handleB = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefromimplicitbinding(i32 0, i32 0, i32 1, i32 0, i1 false)
256226
ret void
257227
}
258-
declare void @a.func(target("dx.TypedBuffer", float, 1, 0, 0) %handle)
259228
)";
260229

261230
auto M = parseAsm(Assembly);
262231

263232
DXILResourceBindingInfo &DRBI =
264233
MAM->getResult<DXILResourceBindingAnalysis>(*M);
265-
EXPECT_EQ(true, DRBI.containsImplicitBinding());
234+
EXPECT_EQ(true, DRBI.hasImplicitBinding());
235+
EXPECT_EQ(false, DRBI.hasOverlappingBinding());
236+
237+
DXILResourceBindingInfo::BindingSpaces &UAVSpaces =
238+
DRBI.getBindingSpaces(ResourceClass::UAV);
239+
EXPECT_EQ(UAVSpaces.ResClass, ResourceClass::UAV);
240+
EXPECT_EQ(UAVSpaces.Spaces.size(), 1u);
241+
checkExpectedSpaceAndFreeRanges(UAVSpaces.Spaces[0], 100,
242+
{0, 4, 6, UINT32_MAX});
266243
}
267244

268245
} // namespace

0 commit comments

Comments
 (0)