Skip to content

Commit ca70746

Browse files
committed
[PredicateInfo] Don't store Def in ValueDFS (NFC)
Def is only actually used during renaming, and having it in ValueDFS causes unnecessary confusing. Remove it from ValueDFS and instead use a separate StackEntry structure for renaming, which holds the ValueDFS and the Def.
1 parent 3e99aa6 commit ca70746

File tree

1 file changed

+20
-16
lines changed

1 file changed

+20
-16
lines changed

llvm/lib/Transforms/Utils/PredicateInfo.cpp

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ 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+
typedef SmallVectorImpl<StackEntry> ValueDFSStack;
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;
@@ -494,8 +499,8 @@ Value *PredicateInfoBuilder::materializeStack(unsigned int &Counter,
494499
RenameIter != RenameStack.end(); ++RenameIter) {
495500
auto *Op =
496501
RenameIter == RenameStack.begin() ? OrigOp : (RenameIter - 1)->Def;
497-
ValueDFS &Result = *RenameIter;
498-
auto *ValInfo = Result.PInfo;
502+
StackEntry &Result = *RenameIter;
503+
auto *ValInfo = Result.V->PInfo;
499504
ValInfo->RenamedOp = (RenameStack.end() - Start) == RenameStack.begin()
500505
? OrigOp
501506
: (RenameStack.end() - Start - 1)->Def;
@@ -623,19 +628,18 @@ void PredicateInfoBuilder::renameUses(SmallVectorImpl<Value *> &OpsToRename) {
623628
// currently and will be considered equal. We could get rid of the
624629
// stable sort by creating one if we wanted.
625630
llvm::stable_sort(OrderedUses, Compare);
626-
SmallVector<ValueDFS, 8> RenameStack;
631+
SmallVector<StackEntry, 8> RenameStack;
627632
// For each use, sorted into dfs order, push values and replaces uses with
628633
// top of stack, which will represent the reaching def.
629-
for (auto &VD : OrderedUses) {
634+
for (const ValueDFS &VD : OrderedUses) {
630635
// We currently do not materialize copy over copy, but we should decide if
631636
// we want to.
632-
bool PossibleCopy = VD.PInfo != nullptr;
633637
if (RenameStack.empty()) {
634638
LLVM_DEBUG(dbgs() << "Rename Stack is empty\n");
635639
} else {
636640
LLVM_DEBUG(dbgs() << "Rename Stack Top DFS numbers are ("
637-
<< RenameStack.back().DFSIn << ","
638-
<< RenameStack.back().DFSOut << ")\n");
641+
<< RenameStack.back().V->DFSIn << ","
642+
<< RenameStack.back().V->DFSOut << ")\n");
639643
}
640644

641645
LLVM_DEBUG(dbgs() << "Current DFS numbers are (" << VD.DFSIn << ","
@@ -644,8 +648,8 @@ void PredicateInfoBuilder::renameUses(SmallVectorImpl<Value *> &OpsToRename) {
644648
// Sync to our current scope.
645649
popStackUntilDFSScope(RenameStack, VD);
646650

647-
if (VD.Def || PossibleCopy) {
648-
RenameStack.push_back(VD);
651+
if (VD.PInfo) {
652+
RenameStack.push_back(&VD);
649653
continue;
650654
}
651655

@@ -657,7 +661,7 @@ void PredicateInfoBuilder::renameUses(SmallVectorImpl<Value *> &OpsToRename) {
657661
LLVM_DEBUG(dbgs() << "Skipping execution due to debug counter\n");
658662
continue;
659663
}
660-
ValueDFS &Result = RenameStack.back();
664+
StackEntry &Result = RenameStack.back();
661665

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

0 commit comments

Comments
 (0)