-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[NFC] Remove goto in PromoteMem2Reg::RenamePass #142454
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NFC] Remove goto in PromoteMem2Reg::RenamePass #142454
Conversation
Created using spr 1.3.6
@llvm/pr-subscribers-llvm-transforms Author: Vitaly Buka (vitalybuka) Changes'goto' is essentially a shortcut for push/pop for worklist. Without 'goto' it's easier to reason about the There is out of order processing of the first Full diff: https://github.com/llvm/llvm-project/pull/142454.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
index 6ba64968193cf..d84b07bd1457c 100644
--- a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
+++ b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
@@ -424,8 +424,8 @@ struct PromoteMem2Reg {
const SmallPtrSetImpl<BasicBlock *> &DefBlocks,
SmallPtrSetImpl<BasicBlock *> &LiveInBlocks);
void RenamePass(BasicBlock *BB, BasicBlock *Pred,
- RenamePassData::ValVector &IncVals,
- RenamePassData::LocationVector &IncLocs,
+ RenamePassData::ValVector IncVals,
+ RenamePassData::LocationVector IncLocs,
std::vector<RenamePassData> &Worklist);
bool QueuePhiNode(BasicBlock *BB, unsigned AllocaIdx, unsigned &Version);
@@ -869,7 +869,8 @@ void PromoteMem2Reg::run() {
RenamePassData RPD = std::move(RenamePassWorkList.back());
RenamePassWorkList.pop_back();
// RenamePass may add new worklist entries.
- RenamePass(RPD.BB, RPD.Pred, RPD.Values, RPD.Locations, RenamePassWorkList);
+ RenamePass(RPD.BB, RPD.Pred, std::move(RPD.Values),
+ std::move(RPD.Locations), RenamePassWorkList);
} while (!RenamePassWorkList.empty());
// Remove the allocas themselves from the function.
@@ -1096,10 +1097,9 @@ static void updateForIncomingValueLocation(PHINode *PN, DebugLoc DL,
/// IncomingVals indicates what value each Alloca contains on exit from the
/// predecessor block Pred.
void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
- RenamePassData::ValVector &IncomingVals,
- RenamePassData::LocationVector &IncomingLocs,
+ RenamePassData::ValVector IncomingVals,
+ RenamePassData::LocationVector IncomingLocs,
std::vector<RenamePassData> &Worklist) {
-NextIteration:
// If we are inserting any phi nodes into this BB, they will already be in the
// block.
if (PHINode *APN = dyn_cast<PHINode>(BB->begin())) {
@@ -1222,17 +1222,17 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
// Keep track of the successors so we don't visit the same successor twice
SmallPtrSet<BasicBlock *, 8> VisitedSuccs;
- // Handle the first successor without using the worklist.
+ // Handle the first successor after the rest, to mimic legacy behaviour.
+ // FIXME: Handle them in regular order.
VisitedSuccs.insert(*I);
- Pred = BB;
- BB = *I;
++I;
for (; I != E; ++I)
if (VisitedSuccs.insert(*I).second)
- Worklist.emplace_back(*I, Pred, IncomingVals, IncomingLocs);
+ Worklist.emplace_back(*I, BB, IncomingVals, IncomingLocs);
- goto NextIteration;
+ Worklist.emplace_back(*succ_begin(BB), BB, std::move(IncomingVals),
+ std::move(IncomingLocs));
}
void llvm::PromoteMemToReg(ArrayRef<AllocaInst *> Allocas, DominatorTree &DT,
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
'goto' is essentially a shortcut for push/pop for worklist. It can be expensive if we copy vectors, but if we move them, it should be an issue. Without 'goto' it's easier to reason about the code, and it `PromoteMem2Reg::RenamePass` process exactly one edge at a time. There is out of order processing of the first successor, just to make this path pure NFC. I'll remove this in follow up patch. Pull Request: llvm#142454
'goto' is essentially a shortcut for push/pop for worklist. It can be expensive if we copy vectors, but if we move them, it should be an issue. Without 'goto' it's easier to reason about the code, and it `PromoteMem2Reg::RenamePass` process exactly one edge at a time. There is out of order processing of the first successor, just to make this path pure NFC. I'll remove this in follow up patch. Pull Request: llvm#142454
'goto' is essentially a shortcut for push/pop for worklist. It can be expensive if we copy vectors, but if we move them, it should be an issue. Without 'goto' it's easier to reason about the code, and it `PromoteMem2Reg::RenamePass` process exactly one edge at a time. There is out of order processing of the first successor, just to make this path pure NFC. I'll remove this in follow up patch. Pull Request: llvm#142454
Created using spr 1.3.6
'goto' is essentially a shortcut for push/pop for worklist.
It can be expensive if we copy vectors, but if we move them, it
should not be an issue.
Without 'goto' it's easier to reason about the
code, when
PromoteMem2Reg::RenamePass
processesexactly one edge at a time.
There is out of order processing of the first
successor, I keep it just to make this patch pure NFC. I'll
remove this in follow up patches.
For #142461