[llvm] StructurizeCFG: Optimize phi insertion during ssa reconstruction (PR #101301)
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 31 01:31:29 PDT 2024
================
@@ -710,10 +714,107 @@ void StructurizeCFG::findUndefBlocks(
}
}
+// Return true if two PHI nodes have compatible incoming values (for each
+// incoming block, either they have the same incoming value or only one PHI
+// node has a incoming value). And return the union of the incoming values
+// through \p Merged.
+bool StructurizeCFG::isCompatible(const BBValueVector &IncomingA,
+ const BBValueVector &IncomingB,
+ BBValueVector &Merged) {
+ MapVector<BasicBlock *, Value *> UnionSet;
+ for (auto &V : IncomingA)
+ UnionSet.insert(V);
+
+ for (auto &V : IncomingB) {
+ if (UnionSet.contains(V.first) && UnionSet[V.first] != V.second)
+ return false;
+ // Either IncomingA does not have this value or IncomingA has the same
+ // value.
+ UnionSet.insert(V);
+ }
+
+ Merged.clear();
+ Merged.append(UnionSet.takeVector());
+ return true;
+}
+
/// Add the real PHI value as soon as everything is set up
void StructurizeCFG::setPhiValues() {
SmallVector<PHINode *, 8> InsertedPhis;
SSAUpdater Updater(&InsertedPhis);
+
+ DenseMap<PHINode *, std::shared_ptr<BBValueVector>> MergedPHIMap;
+ // Find out phi nodes that have compatible incoming values (either they have
+ // the same value for the same block or one have undefined value, see example
+ // below). We only search again the phi's that are referenced by another phi,
+ // which is the cases we care.
+ //
+ // For example (-- means no incoming value):
+ // phi1 : BB1:phi2 BB2:v BB3:--
+ // phi2: BB1:-- BB2:v BB3:w
+ //
+ // Then we can merge these incoming values and let phi1, phi2 use the
+ // same set of incoming values:
+ //
+ // phi1&phi2: BB1:phi2 BB2:v BB3:w
+ //
+ // By doing this, phi1 and phi2 would share more intermediate phi nodes.
+ // This would help reducing number of phi nodes during SSA reconstruction and
+ // get less COPY instructions finally.
+ //
+ // This should be correct, because if a phi node does not have incoming
+ // value from certain block, this means the block is not the predecessor
+ // of the parent block, so we actually don't care its incoming value.
+ for (const auto &AddedPhi : AddedPhis) {
+ BasicBlock *To = AddedPhi.first;
+ if (!DeletedPhis.contains(To))
+ continue;
+ PhiMap &OldPhi = DeletedPhis[To];
+ for (const auto &PI : OldPhi) {
+ SmallVector<PHINode *> IncomingPHIs;
+ PHINode *Phi = PI.first;
+ for (const auto &VI : PI.second) {
+ // First, for each phi, check whether it has incoming value which is
+ // another phi.
+ if (PHINode *P = dyn_cast<PHINode>(VI.second))
+ IncomingPHIs.push_back(P);
+ }
+
+ auto GetUpdatedIncoming = [&](PHINode *Phi) {
+ return MergedPHIMap.contains(Phi) ? *MergedPHIMap[Phi]
+ : DeletedPhis[Phi->getParent()][Phi];
+ };
+ for (auto *OtherPhi : IncomingPHIs) {
+ // Skip phis that are not unrelated to the phi reconstruction for now.
+ if (!DeletedPhis.contains(OtherPhi->getParent()))
+ continue;
+
+ // Skip phis that were already merged with others.
+ if (MergedPHIMap.contains(Phi) && MergedPHIMap.contains(OtherPhi))
+ continue;
+
+ std::shared_ptr<BBValueVector> MergedIncomings;
+ if (MergedPHIMap.contains(Phi))
+ MergedIncomings = MergedPHIMap[Phi];
+ else if (MergedPHIMap.contains(OtherPhi))
+ MergedIncomings = MergedPHIMap[OtherPhi];
----------------
arsenm wrote:
You are doing the same map lookup twice. Use find?
https://github.com/llvm/llvm-project/pull/101301
More information about the llvm-commits
mailing list