[LCSSA] Use IRBuilder for PHI creation.

[fhahn]

Summary

Use IRBuilder instead PHINode::Create. This should not impact the
generated code, but IRBuilder provides a way to register callbacks for
inserted instructions, which is convenient for some users.

[lebedev.ri]

LGTM, but shouldn't the patch as-is should be marked as "NFC"?

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp

1781

Why is this now needed? fixupLCSSAFormFor() already preserves insertion point.

[fhahn]

In D85037#2187796, @lebedev.ri wrote:

LGTM, but shouldn't the patch as-is should be marked as "NFC"?

Now all instructions inserted as part of LCSSA PHI node creation are marked as 'inserted', whereas before only the final ones were. There weren't any binary changes on SPEC2000/SPEC2006/MultiSource with -O3 -flto, but I guess there some users of the expander could also skip the inserted PHI nodes now. Extremely unlikely to cause any changes in the end, but it slightly changes what the expander considers as 'inserted'. Hence I'd prefer it not to be marked as NFC, but it's borderline I think.

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp

1781

Yeah, I'll drop it.

llvm/include/llvm/Transforms/Utils/LoopUtils.h

 /// node.
 ///
 /// LoopInfo and DominatorTree are required and, since the routine makes no
 /// changes to CFG, preserved.
 ///
 /// Returns true if any modifications are made.
 bool formLCSSAForInstructions(SmallVectorImpl<Instruction *> &Worklist,
                               const DominatorTree &DT, const LoopInfo &LI,
-                              ScalarEvolution *SE);
+                              ScalarEvolution *SE, IRBuilderBase &Builder);
 
 /// Put loop into LCSSA form.
 ///
 /// Looks at all instructions in the loop which have uses outside of the
 /// current loop. For each, an LCSSA PHI node is inserted and the uses outside
 /// the loop are rewritten to use this node. Sub-loops must be in LCSSA form
 /// already.
 ///

llvm/lib/Transforms/Utils/LCSSA.cpp

 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/Analysis/LoopPass.h"
 #include "llvm/Analysis/MemorySSA.h"
 #include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/Analysis/ScalarEvolutionAliasAnalysis.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/PredIteratorCache.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Transforms/Utils.h"
 #include "llvm/Transforms/Utils/Local.h"
static bool isExitBlock(BasicBlock *BB,
   return is_contained(ExitBlocks, BB);
 }
 
 /// For every instruction from the worklist, check to see if it has any uses
 /// that are outside the current loop.  If so, insert LCSSA PHI nodes and
 /// rewrite the uses.
 bool llvm::formLCSSAForInstructions(SmallVectorImpl<Instruction *> &Worklist,
                                     const DominatorTree &DT, const LoopInfo &LI,
-                                    ScalarEvolution *SE) {
+                                    ScalarEvolution *SE,
+                                    IRBuilderBase &Builder) {
   SmallVector<Use *, 16> UsesToRewrite;
   SmallSetVector<PHINode *, 16> PHIsToRemove;
   PredIteratorCache PredCache;
   bool Changed = false;
 
+  IRBuilderBase::InsertPointGuard InsertPtGuard(Builder);
+
   // Cache the Loop ExitBlocks across this loop.  We expect to get a lot of
   // instructions within the same loops, computing the exit blocks is
   // expensive, and we're not mutating the loop structure.
   SmallDenseMap<Loop*, SmallVector<BasicBlock *,1>> LoopExitBlocks;
 
   while (!Worklist.empty()) {
     UsesToRewrite.clear();
 
while (!Worklist.empty()) {
     // value, and add them to the Phi's map.
     for (BasicBlock *ExitBB : ExitBlocks) {
       if (!DT.dominates(DomNode, DT.getNode(ExitBB)))
         continue;
 
       // If we already inserted something for this BB, don't reprocess it.
       if (SSAUpdate.HasValueForBlock(ExitBB))
         continue;
-
-      PHINode *PN = PHINode::Create(I->getType(), PredCache.size(ExitBB),
-                                    I->getName() + ".lcssa", &ExitBB->front());
+      Builder.SetInsertPoint(&ExitBB->front());
+      PHINode *PN = Builder.CreatePHI(I->getType(), PredCache.size(ExitBB),
+                                      I->getName() + ".lcssa");
       // Get the debug location from the original instruction.
       PN->setDebugLoc(I->getDebugLoc());
       // Add inputs from inside the loop for this PHI.
       for (BasicBlock *Pred : PredCache.get(ExitBB)) {
         PN->addIncoming(I, Pred);
 
         // If the exit block has a predecessor not within the loop, arrange for
         // the incoming value use corresponding to that predecessor to be
for (Instruction &I : *BB) {
       // instructions in Windows EH if the catchswitch has one catchpad which
       // is inside the loop and another which is not.
       if (I.getType()->isTokenTy())
         continue;
 
       Worklist.push_back(&I);
     }
   }
-  Changed = formLCSSAForInstructions(Worklist, DT, *LI, SE);
+
+  IRBuilder<> Builder(L.getHeader()->getContext());
+  Changed = formLCSSAForInstructions(Worklist, DT, *LI, SE, Builder);
 
   // If we modified the code, remove any caches about the loop from SCEV to
   // avoid dangling entries.
   // FIXME: This is a big hammer, can we clear the cache more selectively?
   if (SE && Changed)
     SE->forgetLoop(&L);
 
   assert(L.isLCSSAForm(DT));

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp

if (auto *Inst = dyn_cast<Instruction>(V)) {
       // Create a temporary instruction to at the current insertion point, so we
       // can hand it off to the helper to create LCSSA PHIs if required for the
       // new use.
       // FIXME: Ideally formLCSSAForInstructions (used in fixupLCSSAFormFor)
       // would accept a insertion point and return an LCSSA phi for that
       // insertion point, so there is no need to insert & remove the temporary
       // instruction.
       Instruction *Tmp;
       if (Inst->getType()->isIntegerTy())
         Tmp =
             cast<Instruction>(Builder.CreateAdd(Inst, Inst, "tmp.lcssa.user"));
       else {
         assert(Inst->getType()->isPointerTy());
         Tmp = cast<Instruction>(
             Builder.CreateGEP(Inst, Builder.getInt32(1), "tmp.lcssa.user"));
       }
       V = fixupLCSSAFormFor(Tmp, 0);
void SCEVExpander::rememberInstruction(Value *I) {
   if (!PreserveLCSSA)
     return;
 
   if (auto *Inst = dyn_cast<Instruction>(I)) {
     // A new instruction has been added, which might introduce new uses outside
     // a defining loop. Fix LCSSA from for each operand of the new instruction,
     // if required.
     for (unsigned OpIdx = 0, OpEnd = Inst->getNumOperands(); OpIdx != OpEnd;
-         OpIdx++) {
-      auto *V = fixupLCSSAFormFor(Inst, OpIdx);
-      if (V != I)
-        DoInsert(V);
-    }
+         OpIdx++)
+      fixupLCSSAFormFor(Inst, OpIdx);
   }
 }
 
 /// getOrInsertCanonicalInductionVariable - This method returns the
 /// canonical induction variable of the specified type for the specified
 /// loop (inserting one if there is none).  A canonical induction variable
 /// starts at zero and steps by one on each iteration.
 PHINode *
if (!OpI)
     return OpV;
 
   Loop *DefLoop = SE.LI.getLoopFor(OpI->getParent());
   Loop *UseLoop = SE.LI.getLoopFor(User->getParent());
   if (!DefLoop || UseLoop == DefLoop || DefLoop->contains(UseLoop))
     return OpV;
 
   ToUpdate.push_back(OpI);
-  formLCSSAForInstructions(ToUpdate, SE.DT, SE.LI, &SE);
+  formLCSSAForInstructions(ToUpdate, SE.DT, SE.LI, &SE, Builder);
   return User->getOperand(OpIdx);
 }
 
 namespace {
 // Search for a SCEV subexpression that is not safe to expand.  Any expression
 // that may expand to a !isSafeToSpeculativelyExecute value is unsafe, namely
 // UDiv expressions. We don't know if the UDiv is derived from an IR divide
 // instruction, but the important thing is that we prove the denominator is