Skip to content

Commit 3e99aa6

Browse files
authored
[PredicateInfo] Clean up DFS sorting (NFC) (#144943)
The comparison function for ValueDFS was confused in a number of ways. Most significantly, it had a bunch of logic based on Def -- however, Def is always null during sorting, it only gets set later. At this point defs only have PInfo set. Clean up the implementation to remove various dead code.
1 parent 4ec6d12 commit 3e99aa6

File tree

1 file changed

+35
-70
lines changed

1 file changed

+35
-70
lines changed

llvm/lib/Transforms/Utils/PredicateInfo.cpp

Lines changed: 35 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -92,19 +92,6 @@ struct ValueDFS {
9292
PredicateBase *PInfo = nullptr;
9393
};
9494

95-
// Perform a strict weak ordering on instructions and arguments.
96-
static bool valueComesBefore(const Value *A, const Value *B) {
97-
auto *ArgA = dyn_cast_or_null<Argument>(A);
98-
auto *ArgB = dyn_cast_or_null<Argument>(B);
99-
if (ArgA && !ArgB)
100-
return true;
101-
if (ArgB && !ArgA)
102-
return false;
103-
if (ArgA && ArgB)
104-
return ArgA->getArgNo() < ArgB->getArgNo();
105-
return cast<Instruction>(A)->comesBefore(cast<Instruction>(B));
106-
}
107-
10895
// This compares ValueDFS structures. Doing so allows us to walk the minimum
10996
// number of instructions necessary to compute our def/use ordering.
11097
struct ValueDFS_Compare {
@@ -114,28 +101,34 @@ struct ValueDFS_Compare {
114101
bool operator()(const ValueDFS &A, const ValueDFS &B) const {
115102
if (&A == &B)
116103
return false;
117-
// The only case we can't directly compare them is when they in the same
118-
// block, and both have localnum == middle. In that case, we have to use
119-
// comesbefore to see what the real ordering is, because they are in the
120-
// same basic block.
104+
assert(!A.Def && !B.Def && "Should not have Def during comparison");
121105

122-
assert((A.DFSIn != B.DFSIn || A.DFSOut == B.DFSOut) &&
106+
// Order by block first.
107+
if (A.DFSIn != B.DFSIn)
108+
return A.DFSIn < B.DFSIn;
109+
assert(A.DFSOut == B.DFSOut &&
123110
"Equal DFS-in numbers imply equal out numbers");
124-
bool SameBlock = A.DFSIn == B.DFSIn;
111+
112+
// Then order by first/middle/last.
113+
if (A.LocalNum != B.LocalNum)
114+
return A.LocalNum < B.LocalNum;
125115

126116
// We want to put the def that will get used for a given set of phi uses,
127117
// before those phi uses.
128118
// So we sort by edge, then by def.
129119
// Note that only phi nodes uses and defs can come last.
130-
if (SameBlock && A.LocalNum == LN_Last && B.LocalNum == LN_Last)
120+
if (A.LocalNum == LN_Last)
131121
return comparePHIRelated(A, B);
132122

133-
bool isADef = A.Def;
134-
bool isBDef = B.Def;
135-
if (!SameBlock || A.LocalNum != LN_Middle || B.LocalNum != LN_Middle)
136-
return std::tie(A.DFSIn, A.LocalNum, isADef) <
137-
std::tie(B.DFSIn, B.LocalNum, isBDef);
138-
return localComesBefore(A, B);
123+
// Use block-local ordering for instructions in the middle.
124+
if (A.LocalNum == LN_Middle)
125+
return localComesBefore(A, B);
126+
127+
// The order of PredicateInfo definitions at the start of the block does not
128+
// matter.
129+
assert(A.LocalNum == LN_First);
130+
assert(A.PInfo && B.PInfo && "Must be predicate info def");
131+
return false;
139132
}
140133

141134
// For a phi use, or a non-materialized def, return the edge it represents.
@@ -173,60 +166,32 @@ struct ValueDFS_Compare {
173166
DomTreeNode *DomBDest = DT.getNode(BDest);
174167
unsigned AIn = DomADest->getDFSNumIn();
175168
unsigned BIn = DomBDest->getDFSNumIn();
176-
bool isADef = A.Def;
177-
bool isBDef = B.Def;
178-
assert((!A.Def || !A.U) && (!B.Def || !B.U) &&
169+
bool isAUse = A.U;
170+
bool isBUse = B.U;
171+
assert((!A.PInfo || !A.U) && (!B.PInfo || !B.U) &&
179172
"Def and U cannot be set at the same time");
180173
// Now sort by edge destination and then defs before uses.
181-
return std::tie(AIn, isADef) < std::tie(BIn, isBDef);
174+
return std::tie(AIn, isAUse) < std::tie(BIn, isBUse);
182175
}
183176

184-
// Get the definition of an instruction that occurs in the middle of a block.
185-
Value *getMiddleDef(const ValueDFS &VD) const {
186-
if (VD.Def)
187-
return VD.Def;
188-
// It's possible for the defs and uses to be null. For branches, the local
189-
// numbering will say the placed predicaeinfos should go first (IE
190-
// LN_beginning), so we won't be in this function. For assumes, we will end
191-
// up here, beause we need to order the def we will place relative to the
192-
// assume. So for the purpose of ordering, we pretend the def is right
193-
// after the assume, because that is where we will insert the info.
194-
if (!VD.U) {
195-
assert(VD.PInfo &&
196-
"No def, no use, and no predicateinfo should not occur");
197-
assert(isa<PredicateAssume>(VD.PInfo) &&
198-
"Middle of block should only occur for assumes");
199-
return cast<PredicateAssume>(VD.PInfo)->AssumeInst->getNextNode();
200-
}
201-
return nullptr;
202-
}
177+
const Instruction *getDefOrUser(const ValueDFS &VD) const {
178+
if (VD.U)
179+
return cast<Instruction>(VD.U->getUser());
203180

204-
// Return either the Def, if it's not null, or the user of the Use, if the def
205-
// is null.
206-
const Instruction *getDefOrUser(const Value *Def, const Use *U) const {
207-
if (Def)
208-
return cast<Instruction>(Def);
209-
return cast<Instruction>(U->getUser());
181+
// For the purpose of ordering, we pretend the def is right after the
182+
// assume, because that is where we will insert the info.
183+
assert(VD.PInfo && "No use, and no predicateinfo should not occur");
184+
assert(isa<PredicateAssume>(VD.PInfo) &&
185+
"Middle of block should only occur for assumes");
186+
return cast<PredicateAssume>(VD.PInfo)->AssumeInst->getNextNode();
210187
}
211188

212189
// This performs the necessary local basic block ordering checks to tell
213190
// whether A comes before B, where both are in the same basic block.
214191
bool localComesBefore(const ValueDFS &A, const ValueDFS &B) const {
215-
auto *ADef = getMiddleDef(A);
216-
auto *BDef = getMiddleDef(B);
217-
218-
// See if we have real values or uses. If we have real values, we are
219-
// guaranteed they are instructions or arguments. No matter what, we are
220-
// guaranteed they are in the same block if they are instructions.
221-
auto *ArgA = dyn_cast_or_null<Argument>(ADef);
222-
auto *ArgB = dyn_cast_or_null<Argument>(BDef);
223-
224-
if (ArgA || ArgB)
225-
return valueComesBefore(ArgA, ArgB);
226-
227-
auto *AInst = getDefOrUser(ADef, A.U);
228-
auto *BInst = getDefOrUser(BDef, B.U);
229-
return valueComesBefore(AInst, BInst);
192+
const Instruction *AInst = getDefOrUser(A);
193+
const Instruction *BInst = getDefOrUser(B);
194+
return AInst->comesBefore(BInst);
230195
}
231196
};
232197

0 commit comments

Comments
 (0)