Skip to content

Commit da3dc02

Browse files
committed
[PredicateInfo] Clean up DFS sorting (NFC)
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. Clean up the implementation to remove various dead code.
1 parent 633e740 commit da3dc02

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
@@ -94,19 +94,6 @@ struct ValueDFS {
9494
bool EdgeOnly = false;
9595
};
9696

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

124-
assert((A.DFSIn != B.DFSIn || A.DFSOut == B.DFSOut) &&
108+
// Order by block first.
109+
if (A.DFSIn != B.DFSIn)
110+
return A.DFSIn < B.DFSIn;
111+
assert(A.DFSOut == B.DFSOut &&
125112
"Equal DFS-in numbers imply equal out numbers");
126-
bool SameBlock = A.DFSIn == B.DFSIn;
113+
114+
// Then order by first/middle/last.
115+
if (A.LocalNum != B.LocalNum)
116+
return A.LocalNum < B.LocalNum;
127117

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

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

143136
// For a phi use, or a non-materialized def, return the edge it represents.
@@ -175,60 +168,32 @@ struct ValueDFS_Compare {
175168
DomTreeNode *DomBDest = DT.getNode(BDest);
176169
unsigned AIn = DomADest->getDFSNumIn();
177170
unsigned BIn = DomBDest->getDFSNumIn();
178-
bool isADef = A.Def;
179-
bool isBDef = B.Def;
180-
assert((!A.Def || !A.U) && (!B.Def || !B.U) &&
171+
bool isAUse = A.U;
172+
bool isBUse = B.U;
173+
assert((!A.PInfo || !A.U) && (!B.PInfo || !B.U) &&
181174
"Def and U cannot be set at the same time");
182175
// Now sort by edge destination and then defs before uses.
183-
return std::tie(AIn, isADef) < std::tie(BIn, isBDef);
176+
return std::tie(AIn, isAUse) < std::tie(BIn, isBUse);
184177
}
185178

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

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

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

0 commit comments

Comments
 (0)