-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[SSAUpdaterBulk] Add PHI simplification pass. #132004
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
Changes from all commits
8596961
10e99d6
09f1e89
241b300
fba85b4
6264060
23f244e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,13 +11,15 @@ | |
//===----------------------------------------------------------------------===// | ||
|
||
#include "llvm/Transforms/Utils/SSAUpdaterBulk.h" | ||
#include "llvm/Analysis/InstructionSimplify.h" | ||
#include "llvm/Analysis/IteratedDominanceFrontier.h" | ||
#include "llvm/IR/BasicBlock.h" | ||
#include "llvm/IR/Dominators.h" | ||
#include "llvm/IR/IRBuilder.h" | ||
#include "llvm/IR/Instructions.h" | ||
#include "llvm/IR/Use.h" | ||
#include "llvm/IR/Value.h" | ||
#include "llvm/Transforms/Utils/Local.h" | ||
|
||
using namespace llvm; | ||
|
||
|
@@ -109,8 +111,8 @@ struct BBValueInfo { | |
|
||
/// Perform all the necessary updates, including new PHI-nodes insertion and the | ||
/// requested uses update. | ||
void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT, | ||
SmallVectorImpl<PHINode *> *InsertedPHIs) { | ||
void SSAUpdaterBulk::createPHIsAndRewrite( | ||
DominatorTree *DT, SmallVectorImpl<PHINode *> &InsertedPHIs) { | ||
DenseMap<BasicBlock *, BBValueInfo> BBInfos; | ||
for (auto &R : Rewrites) { | ||
BBInfos.clear(); | ||
|
@@ -150,8 +152,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT, | |
IRBuilder<> B(FrontierBB, FrontierBB->begin()); | ||
PHINode *PN = B.CreatePHI(R.Ty, 0, R.Name); | ||
BBInfos[FrontierBB].LiveInValue = PN; | ||
if (InsertedPHIs) | ||
InsertedPHIs->push_back(PN); | ||
InsertedPHIs.push_back(PN); | ||
} | ||
|
||
// IsLiveOut indicates whether we are computing live-out values (true) or | ||
|
@@ -223,3 +224,78 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT, | |
} | ||
} | ||
} | ||
|
||
bool SSAUpdaterBulk::simplifyPass(SmallVectorImpl<PHINode *> &Worklist) { | ||
if (Worklist.empty()) | ||
return false; | ||
|
||
const DataLayout &DL = Worklist.front()->getParent()->getDataLayout(); | ||
bool Change = false; | ||
for (PHINode *&PHI : Worklist) { | ||
if (Value *Replacement = simplifyInstruction(PHI, DL)) { | ||
PHI->replaceAllUsesWith(Replacement); | ||
PHI->eraseFromParent(); | ||
PHI = nullptr; // Mark as removed | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could also iterate over Worklist using erase_if instead to avoid leaving behind nullptrs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, but I don't see much sense since this is internal routine and we're know what we're doing after that :) |
||
Change = true; | ||
} | ||
} | ||
return Change; | ||
} | ||
|
||
bool SSAUpdaterBulk::deduplicatePass(const SmallVectorImpl<PHINode *> &Worklist, | ||
SmallPtrSetImpl<PHINode *> &PHIToRemove) { | ||
|
||
auto FindFirstNonPHIIt = [](BasicBlock *BB) { | ||
vpykhtin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
PHINode *LastPHI = nullptr; | ||
for (auto &PHI : BB->phis()) | ||
LastPHI = &PHI; | ||
assert(LastPHI); | ||
return std::next(LastPHI->getIterator()); | ||
}; | ||
|
||
// BB -> First BB's Non-PHI iterator map | ||
SmallDenseMap<BasicBlock *, BasicBlock::iterator> Blocks; | ||
// Reverse Worklist to preserve the order of new phis. | ||
for (PHINode *PHI : reverse(Worklist)) { | ||
if (!PHI) | ||
continue; | ||
auto *BB = PHI->getParent(); | ||
auto [I, Inserted] = Blocks.try_emplace(BB); | ||
if (Inserted) | ||
I->second = FindFirstNonPHIIt(BB); | ||
|
||
// Move newly inserted PHIs to the end to ensure that | ||
// EliminateDuplicatePHINodes prioritizes removing the newly created PHIs | ||
// over the existing ones, preserving the original PHI nodes. | ||
PHI->moveBefore(I->second); | ||
} | ||
|
||
for (auto [BB, I] : Blocks) | ||
EliminateDuplicatePHINodes(BB, PHIToRemove); | ||
|
||
for (PHINode *PHI : PHIToRemove) | ||
PHI->eraseFromParent(); | ||
|
||
return !PHIToRemove.empty(); | ||
} | ||
|
||
void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT, | ||
SmallVectorImpl<PHINode *> *InsertedPHIs) { | ||
SmallVector<PHINode *, 4> PHIs; | ||
createPHIsAndRewrite(DT, PHIs); | ||
simplifyPass(PHIs); | ||
|
||
SmallPtrSet<PHINode *, 8> PHIToRemove; | ||
deduplicatePass(PHIs, PHIToRemove); | ||
|
||
if (!InsertedPHIs) | ||
return; | ||
|
||
for (auto *&PHI : PHIs) | ||
if (PHI && PHIToRemove.count(PHI)) | ||
PHI = nullptr; // Mark as removed. | ||
vpykhtin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
InsertedPHIs->reserve(PHIs.size() - PHIToRemove.size()); | ||
std::copy_if(PHIs.begin(), PHIs.end(), std::back_inserter(*InsertedPHIs), | ||
[](PHINode *PHI) { return PHI != nullptr; }); | ||
} |
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.
nit: More typical naming for this flag.
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.
Will address this in the new stacker PR.