Skip to content

Commit 1e58e9c

Browse files
authored
[PredicateInfo] Don't store Def in ValueDFS (NFC) (#145022)
Def is only actually used during renaming, and having it in ValueDFS causes unnecessary confusion. Remove it from ValueDFS and instead use a separate StackEntry structure for renaming, which holds the ValueDFS and the Def.
1 parent c6be4ff commit 1e58e9c

File tree

1 file changed

+22
-18
lines changed

1 file changed

+22
-18
lines changed

llvm/lib/Transforms/Utils/PredicateInfo.cpp

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,13 @@ enum LocalNum {
8080
LN_Last
8181
};
8282

83-
// Associate global and local DFS info with defs and uses, so we can sort them
84-
// into a global domination ordering.
83+
// Associate global and local DFS info with defs (PInfo set) and uses (U set),
84+
// so we can sort them into a global domination ordering.
8585
struct ValueDFS {
8686
int DFSIn = 0;
8787
int DFSOut = 0;
8888
unsigned int LocalNum = LN_Middle;
89-
// Only one of Def or Use will be set.
90-
Value *Def = nullptr;
89+
// Only one of U or PInfo will be set.
9190
Use *U = nullptr;
9291
PredicateBase *PInfo = nullptr;
9392
};
@@ -101,7 +100,6 @@ struct ValueDFS_Compare {
101100
bool operator()(const ValueDFS &A, const ValueDFS &B) const {
102101
if (&A == &B)
103102
return false;
104-
assert(!A.Def && !B.Def && "Should not have Def during comparison");
105103

106104
// Order by block first.
107105
if (A.DFSIn != B.DFSIn)
@@ -133,7 +131,7 @@ struct ValueDFS_Compare {
133131

134132
// For a phi use, or a non-materialized def, return the edge it represents.
135133
std::pair<BasicBlock *, BasicBlock *> getBlockEdge(const ValueDFS &VD) const {
136-
if (!VD.Def && VD.U) {
134+
if (VD.U) {
137135
auto *PHI = cast<PHINode>(VD.U->getUser());
138136
return std::make_pair(PHI->getIncomingBlock(*VD.U), PHI->getParent());
139137
}
@@ -229,7 +227,14 @@ class PredicateInfoBuilder {
229227
void addInfoFor(SmallVectorImpl<Value *> &OpsToRename, Value *Op,
230228
PredicateBase *PB);
231229

232-
typedef SmallVectorImpl<ValueDFS> ValueDFSStack;
230+
struct StackEntry {
231+
const ValueDFS *V;
232+
Value *Def = nullptr;
233+
234+
StackEntry(const ValueDFS *V) : V(V) {}
235+
};
236+
237+
using ValueDFSStack = SmallVectorImpl<StackEntry>;
233238
void convertUsesToDFSOrdered(Value *, SmallVectorImpl<ValueDFS> &);
234239
Value *materializeStack(unsigned int &, ValueDFSStack &, Value *);
235240
bool stackIsInScope(const ValueDFSStack &, const ValueDFS &) const;
@@ -254,7 +259,7 @@ bool PredicateInfoBuilder::stackIsInScope(const ValueDFSStack &Stack,
254259
// a LN_Last def, we need to pop the stack. We deliberately sort phi uses
255260
// next to the defs they must go with so that we can know it's time to pop
256261
// the stack when we hit the end of the phi uses for a given def.
257-
const ValueDFS &Top = Stack.back();
262+
const ValueDFS &Top = *Stack.back().V;
258263
if (Top.LocalNum == LN_Last && Top.PInfo) {
259264
if (!VDUse.U)
260265
return false;
@@ -496,8 +501,8 @@ Value *PredicateInfoBuilder::materializeStack(unsigned int &Counter,
496501
RenameIter != RenameStack.end(); ++RenameIter) {
497502
auto *Op =
498503
RenameIter == RenameStack.begin() ? OrigOp : (RenameIter - 1)->Def;
499-
ValueDFS &Result = *RenameIter;
500-
auto *ValInfo = Result.PInfo;
504+
StackEntry &Result = *RenameIter;
505+
auto *ValInfo = Result.V->PInfo;
501506
ValInfo->RenamedOp = (RenameStack.end() - Start) == RenameStack.begin()
502507
? OrigOp
503508
: (RenameStack.end() - Start - 1)->Def;
@@ -625,19 +630,18 @@ void PredicateInfoBuilder::renameUses(SmallVectorImpl<Value *> &OpsToRename) {
625630
// currently and will be considered equal. We could get rid of the
626631
// stable sort by creating one if we wanted.
627632
llvm::stable_sort(OrderedUses, Compare);
628-
SmallVector<ValueDFS, 8> RenameStack;
633+
SmallVector<StackEntry, 8> RenameStack;
629634
// For each use, sorted into dfs order, push values and replaces uses with
630635
// top of stack, which will represent the reaching def.
631-
for (auto &VD : OrderedUses) {
636+
for (const ValueDFS &VD : OrderedUses) {
632637
// We currently do not materialize copy over copy, but we should decide if
633638
// we want to.
634-
bool PossibleCopy = VD.PInfo != nullptr;
635639
if (RenameStack.empty()) {
636640
LLVM_DEBUG(dbgs() << "Rename Stack is empty\n");
637641
} else {
638642
LLVM_DEBUG(dbgs() << "Rename Stack Top DFS numbers are ("
639-
<< RenameStack.back().DFSIn << ","
640-
<< RenameStack.back().DFSOut << ")\n");
643+
<< RenameStack.back().V->DFSIn << ","
644+
<< RenameStack.back().V->DFSOut << ")\n");
641645
}
642646

643647
LLVM_DEBUG(dbgs() << "Current DFS numbers are (" << VD.DFSIn << ","
@@ -646,8 +650,8 @@ void PredicateInfoBuilder::renameUses(SmallVectorImpl<Value *> &OpsToRename) {
646650
// Sync to our current scope.
647651
popStackUntilDFSScope(RenameStack, VD);
648652

649-
if (VD.Def || PossibleCopy) {
650-
RenameStack.push_back(VD);
653+
if (VD.PInfo) {
654+
RenameStack.push_back(&VD);
651655
continue;
652656
}
653657

@@ -659,7 +663,7 @@ void PredicateInfoBuilder::renameUses(SmallVectorImpl<Value *> &OpsToRename) {
659663
LLVM_DEBUG(dbgs() << "Skipping execution due to debug counter\n");
660664
continue;
661665
}
662-
ValueDFS &Result = RenameStack.back();
666+
StackEntry &Result = RenameStack.back();
663667

664668
// If the possible copy dominates something, materialize our stack up to
665669
// this point. This ensures every comparison that affects our operation

0 commit comments

Comments
 (0)