[llvm] [Inline][Cloning] Defer constant propagation after phi-nodes resolution (PR #87963)
Yingwei Zheng via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 10 08:37:49 PDT 2024
================
@@ -823,52 +804,45 @@ void llvm::CloneAndPruneIntoFromInst(Function *NewFunc, const Function *OldFunc,
}
}
- // Make a second pass over the PHINodes now that all of them have been
- // remapped into the new function, simplifying the PHINode and performing any
- // recursive simplifications exposed. This will transparently update the
- // WeakTrackingVH in the VMap. Notably, we rely on that so that if we coalesce
- // two PHINodes, the iteration over the old PHIs remains valid, and the
- // mapping will just map us to the new node (which may not even be a PHI
- // node).
- const DataLayout &DL = NewFunc->getParent()->getDataLayout();
- SmallSetVector<const Value *, 8> Worklist;
- for (unsigned Idx = 0, Size = PHIToResolve.size(); Idx != Size; ++Idx)
- if (isa<PHINode>(VMap[PHIToResolve[Idx]]))
- Worklist.insert(PHIToResolve[Idx]);
-
- // Note that we must test the size on each iteration, the worklist can grow.
- for (unsigned Idx = 0; Idx != Worklist.size(); ++Idx) {
- const Value *OrigV = Worklist[Idx];
- auto *I = dyn_cast_or_null<Instruction>(VMap.lookup(OrigV));
- if (!I)
- continue;
-
- // Skip over non-intrinsic callsites, we don't want to remove any nodes from
- // the CGSCC.
- CallBase *CB = dyn_cast<CallBase>(I);
- if (CB && CB->getCalledFunction() &&
- !CB->getCalledFunction()->isIntrinsic())
- continue;
-
- // See if this instruction simplifies.
- Value *SimpleV = simplifyInstruction(I, DL);
- if (!SimpleV)
- continue;
-
- // Stash away all the uses of the old instruction so we can check them for
- // recursive simplifications after a RAUW. This is cheaper than checking all
- // uses of To on the recursive step in most cases.
- for (const User *U : OrigV->users())
- Worklist.insert(cast<Instruction>(U));
-
- // Replace the instruction with its simplified value.
- I->replaceAllUsesWith(SimpleV);
+ auto GetOriginalValueInVMap = [&](const auto &I) -> Value * {
+ for (const auto &[OrigI, NewI] : VMap) {
+ if (NewI == I)
+ return const_cast<Value *>(OrigI);
+ }
+ return nullptr;
+ };
- // If the original instruction had no side effects, remove it.
- if (isInstructionTriviallyDead(I))
- I->eraseFromParent();
- else
- VMap[OrigV] = I;
+ // As phi-nodes have been now remapped, allow incremental simplification of
+ // newly-cloned instructions.
+ const DataLayout &DL = NewFunc->getParent()->getDataLayout();
+ auto *NewEntryBlock = cast<BasicBlock>(VMap[StartingBB]);
+ ReversePostOrderTraversal<BasicBlock *> RPOT(NewEntryBlock);
+ for (auto &BB : RPOT) {
+ for (BasicBlock::iterator II = BB->begin(), IE = BB->end(); II != IE;) {
+ auto *NewI = &*II++;
+ if (isa<DbgInfoIntrinsic>(NewI))
+ continue;
+
+ CallBase *CB = dyn_cast<CallBase>(NewI);
----------------
dtcxzyw wrote:
```suggestion
// Skip over non-intrinsic callsites, we don't want to remove any nodes from
// the CGSCC.
CallBase *CB = dyn_cast<CallBase>(NewI);
```
Should we keep the comment?
https://github.com/llvm/llvm-project/pull/87963
More information about the llvm-commits
mailing list