Skip to content

Commit 8a27708

Browse files
committed
code review feedback - use std::optional, renames
1 parent da838a9 commit 8a27708

File tree

3 files changed

+35
-36
lines changed

3 files changed

+35
-36
lines changed

llvm/include/llvm/Analysis/DXILResource.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,7 @@ class DXILResourceBindingInfo {
633633
FreeRanges.emplace_back(0, UINT32_MAX);
634634
}
635635
// Size == -1 means unbounded array
636-
bool findAvailableBinding(int32_t Size, uint32_t *RegSlot);
636+
std::optional<uint32_t> findAvailableBinding(int32_t Size);
637637
};
638638

639639
struct BindingSpaces {
@@ -678,8 +678,8 @@ class DXILResourceBindingInfo {
678678
}
679679

680680
// Size == -1 means unbounded array
681-
bool findAvailableBinding(dxil::ResourceClass RC, uint32_t Space,
682-
int32_t Size, uint32_t *RegSlot);
681+
std::optional<uint32_t> findAvailableBinding(dxil::ResourceClass RC,
682+
uint32_t Space, int32_t Size);
683683

684684
friend class DXILResourceBindingAnalysis;
685685
friend class DXILResourceBindingWrapperPass;

llvm/lib/Analysis/DXILResource.cpp

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -995,12 +995,12 @@ void DXILResourceBindingInfo::populate(Module &M, DXILResourceTypeMap &DRTM) {
995995
}
996996

997997
// returns false if binding could not be found in given space
998-
bool DXILResourceBindingInfo::findAvailableBinding(dxil::ResourceClass RC,
999-
uint32_t Space, int32_t Size,
1000-
uint32_t *RegSlot) {
998+
std::optional<uint32_t>
999+
DXILResourceBindingInfo::findAvailableBinding(dxil::ResourceClass RC,
1000+
uint32_t Space, int32_t Size) {
10011001
BindingSpaces &BS = getBindingSpaces(RC);
10021002
RegisterSpace &RS = BS.getOrInsertSpace(Space);
1003-
return RS.findAvailableBinding(Size, RegSlot);
1003+
return RS.findAvailableBinding(Size);
10041004
}
10051005

10061006
DXILResourceBindingInfo::RegisterSpace &
@@ -1015,38 +1015,38 @@ DXILResourceBindingInfo::BindingSpaces::getOrInsertSpace(uint32_t Space) {
10151015
return Spaces.emplace_back(Space);
10161016
}
10171017

1018-
bool DXILResourceBindingInfo::RegisterSpace::findAvailableBinding(
1019-
int32_t Size, uint32_t *RegSlot) {
1018+
std::optional<uint32_t>
1019+
DXILResourceBindingInfo::RegisterSpace::findAvailableBinding(int32_t Size) {
10201020
assert((Size == -1 || Size > 0) && "invalid size");
10211021

1022+
std::optional<uint32_t> RegSlot;
10221023
if (FreeRanges.empty())
1023-
return false;
1024+
return RegSlot;
10241025

10251026
// unbounded array
10261027
if (Size == -1) {
10271028
BindingRange &Last = FreeRanges.back();
10281029
if (Last.UpperBound != UINT32_MAX)
10291030
// this space is already occupied by an unbounded array
10301031
return false;
1031-
*RegSlot = Last.LowerBound;
1032+
RegSlot = Last.LowerBound;
10321033
FreeRanges.pop_back();
1033-
return true;
1034-
}
1035-
1036-
// single resource or fixed-size array
1037-
for (BindingRange &R : FreeRanges) {
1038-
// compare the size as uint64_t to prevent overflow for range (0,
1039-
// UINT32_MAX)
1040-
if ((uint64_t)R.UpperBound - R.LowerBound + 1 < (uint64_t)Size)
1041-
continue;
1042-
*RegSlot = R.LowerBound;
1043-
// This might create a range where (LowerBound == UpperBound + 1), but
1044-
// that's ok.
1045-
R.LowerBound += Size;
1046-
return true;
1034+
} else {
1035+
// single resource or fixed-size array
1036+
for (BindingRange &R : FreeRanges) {
1037+
// compare the size as uint64_t to prevent overflow for range (0,
1038+
// UINT32_MAX)
1039+
if ((uint64_t)R.UpperBound - R.LowerBound + 1 < (uint64_t)Size)
1040+
continue;
1041+
RegSlot = R.LowerBound;
1042+
// This might create a range where (LowerBound == UpperBound + 1). When
1043+
// that happens, the next time this function is called the range will
1044+
// skipped over by the check above (at this point Size is always > 0).
1045+
R.LowerBound += Size;
1046+
break;
1047+
}
10471048
}
1048-
1049-
return false;
1049+
return RegSlot;
10501050
}
10511051

10521052
//===----------------------------------------------------------------------===//

llvm/lib/Target/DirectX/DXILResourceImplicitBinding.cpp

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,13 @@ static void diagnoseImplicitBindingNotFound(CallInst *ImplBindingCall) {
4040

4141
static bool assignBindings(Module &M, DXILResourceBindingInfo &DRBI,
4242
DXILResourceTypeMap &DRTM) {
43-
struct ImplBindingCall {
43+
struct ImplicitBindingCall {
4444
int OrderID;
4545
CallInst *Call;
46-
ImplBindingCall(int OrderID, CallInst *Call)
46+
ImplicitBindingCall(int OrderID, CallInst *Call)
4747
: OrderID(OrderID), Call(Call) {}
4848
};
49-
SmallVector<ImplBindingCall> Calls;
49+
SmallVector<ImplicitBindingCall> Calls;
5050
SmallVector<Function *> FunctionsToMaybeRemove;
5151

5252
// collect all of the llvm.dx.resource.handlefromImplicitbinding calls
@@ -78,7 +78,7 @@ static bool assignBindings(Module &M, DXILResourceBindingInfo &DRBI,
7878
bool AllBindingsAssigned = true;
7979
bool Changed = false;
8080

81-
for (auto &IB : Calls) {
81+
for (ImplicitBindingCall &IB : Calls) {
8282
IRBuilder<> Builder(IB.Call);
8383

8484
if (IB.OrderID != LastOrderID) {
@@ -91,15 +91,14 @@ static bool assignBindings(Module &M, DXILResourceBindingInfo &DRBI,
9191
int32_t Size =
9292
cast<ConstantInt>(IB.Call->getArgOperand(2))->getZExtValue();
9393

94-
uint32_t RegSlot;
95-
RegSlotOp = nullptr;
96-
if (!DRBI.findAvailableBinding(RTI.getResourceClass(), Space, Size,
97-
&RegSlot)) {
94+
std::optional<uint32_t> RegSlot =
95+
DRBI.findAvailableBinding(RTI.getResourceClass(), Space, Size);
96+
if (!RegSlot) {
9897
diagnoseImplicitBindingNotFound(IB.Call);
9998
AllBindingsAssigned = false;
10099
continue;
101100
}
102-
RegSlotOp = ConstantInt::get(Builder.getInt32Ty(), RegSlot);
101+
RegSlotOp = ConstantInt::get(Builder.getInt32Ty(), RegSlot.value());
103102
}
104103

105104
if (!RegSlotOp)

0 commit comments

Comments
 (0)