[RemoveDI][Polly] Use iterators instead of instruction pointers to SetInsertPoint (#135336)

As part of the effort to transition to using Debug Records instead of
Debug intrinsics, some API/argument changes are necessary to achieve the
desired behavior from Debug Records. This particular fix involves
passing iterators instead of instruction pointers to the SetInsertPoint
function. While this is crucial in certain areas, it may be more than
needed in others, but it does not cause any harm.

Author: kartcq

polly/lib/CodeGen/BlockGenerators.cpp

@@ -420,7 +420,7 @@ BasicBlock *BlockGenerator::copyBB(ScopStmt &Stmt, BasicBlock *BB,
                                    ValueMapT &BBMap, LoopToScevMapT &LTS,
                                    isl_id_to_ast_expr *NewAccesses) {
   BasicBlock *CopyBB = splitBB(BB);
-  Builder.SetInsertPoint(&CopyBB->front());
+  Builder.SetInsertPoint(CopyBB, CopyBB->begin());
   generateScalarLoads(Stmt, LTS, BBMap, NewAccesses);
   generateBeginStmtTrace(Stmt, LTS, BBMap);
 
@@ -795,7 +795,7 @@ void BlockGenerator::createScalarInitialization(Scop &S) {
   BasicBlock *ExitBB = S.getExit();
   BasicBlock *PreEntryBB = S.getEnteringBlock();
 
-  Builder.SetInsertPoint(&*StartBlock->begin());
+  Builder.SetInsertPoint(StartBlock, StartBlock->begin());
 
   for (auto &Array : S.arrays()) {
     if (Array->getNumberOfDimensions() != 0)
@@ -850,7 +850,7 @@ void BlockGenerator::createScalarFinalization(Scop &S) {
   if (OptExitBB == ExitBB)
     OptExitBB = *(++pred_begin(MergeBB));
 
-  Builder.SetInsertPoint(OptExitBB->getTerminator());
+  Builder.SetInsertPoint(OptExitBB, OptExitBB->getTerminator()->getIterator());
   for (const auto &EscapeMapping : EscapeMap) {
     // Extract the escaping instruction and the escaping users as well as the
     // alloca the instruction was demoted to.
@@ -921,7 +921,7 @@ void BlockGenerator::createExitPHINodeMerges(Scop &S) {
   if (OptExitBB == ExitBB)
     OptExitBB = *(++pred_begin(MergeBB));
 
-  Builder.SetInsertPoint(OptExitBB->getTerminator());
+  Builder.SetInsertPoint(OptExitBB, OptExitBB->getTerminator()->getIterator());
 
   for (auto &SAI : S.arrays()) {
     auto *Val = SAI->getBasePtr();
@@ -1072,7 +1072,7 @@ void RegionGenerator::copyStmt(ScopStmt &Stmt, LoopToScevMapT &LTS,
   BasicBlock *EntryBBCopy = SplitBlock(
       Builder.GetInsertBlock(), &*Builder.GetInsertPoint(), GenDT, GenLI);
   EntryBBCopy->setName("polly.stmt." + EntryBB->getName() + ".entry");
-  Builder.SetInsertPoint(&EntryBBCopy->front());
+  Builder.SetInsertPoint(EntryBBCopy, EntryBBCopy->begin());
 
   ValueMapT &EntryBBMap = RegionMaps[EntryBBCopy];
   generateScalarLoads(Stmt, LTS, EntryBBMap, IdToAstExp);
@@ -1112,7 +1112,7 @@ void RegionGenerator::copyStmt(ScopStmt &Stmt, LoopToScevMapT &LTS,
     ValueMapT &RegionMap = Inserted.first->second;
 
     // Copy the block with the BlockGenerator.
-    Builder.SetInsertPoint(&BBCopy->front());
+    Builder.SetInsertPoint(BBCopy, BBCopy->begin());
     copyBB(Stmt, BB, BBCopy, RegionMap, LTS, IdToAstExp);
 
     // In order to remap PHI nodes we store also basic block mappings.
@@ -1166,7 +1166,7 @@ void RegionGenerator::copyStmt(ScopStmt &Stmt, LoopToScevMapT &LTS,
     ValueMapT &RegionMap = RegionMaps[BBCopyStart];
     RegionMap.insert_range(StartBlockMap);
 
-    Builder.SetInsertPoint(BICopy);
+    Builder.SetInsertPoint(BBCopyEnd, BICopy->getIterator());
     copyInstScalar(Stmt, TI, RegionMap, LTS);
     BICopy->eraseFromParent();
   }
@@ -1204,7 +1204,7 @@ void RegionGenerator::copyStmt(ScopStmt &Stmt, LoopToScevMapT &LTS,
   }
 
   // Continue generating code in the exit block.
-  Builder.SetInsertPoint(&*ExitBBCopy->getFirstInsertionPt());
+  Builder.SetInsertPoint(ExitBBCopy, ExitBBCopy->getFirstInsertionPt());
 
   // Write values visible to other statements.
   generateScalarStores(Stmt, LTS, ValueMap, IdToAstExp);
@@ -1241,7 +1241,8 @@ PHINode *RegionGenerator::buildExitPHI(MemoryAccess *MA, LoopToScevMapT &LTS,
     BasicBlock *OrigIncomingBlock = Pair.first;
     BasicBlock *NewIncomingBlockStart = StartBlockMap.lookup(OrigIncomingBlock);
     BasicBlock *NewIncomingBlockEnd = EndBlockMap.lookup(OrigIncomingBlock);
-    Builder.SetInsertPoint(NewIncomingBlockEnd->getTerminator());
+    Builder.SetInsertPoint(NewIncomingBlockEnd,
+                           NewIncomingBlockEnd->getTerminator()->getIterator());
     assert(RegionMaps.count(NewIncomingBlockStart));
     assert(RegionMaps.count(NewIncomingBlockEnd));
     ValueMapT *LocalBBMap = &RegionMaps[NewIncomingBlockStart];
@@ -1358,10 +1359,11 @@ void RegionGenerator::addOperandToPHI(ScopStmt &Stmt, PHINode *PHI,
     // change it, otherwise do not.
     auto IP = Builder.GetInsertPoint();
     if (IP->getParent() != BBCopyEnd)
-      Builder.SetInsertPoint(BBCopyEnd->getTerminator());
+      Builder.SetInsertPoint(BBCopyEnd,
+                             BBCopyEnd->getTerminator()->getIterator());
     OpCopy = getNewValue(Stmt, Op, BBCopyMap, LTS, getLoopForStmt(Stmt));
     if (IP->getParent() != BBCopyEnd)
-      Builder.SetInsertPoint(&*IP);
+      Builder.SetInsertPoint(IP);
   } else {
     // All edges from outside the non-affine region become a single edge
     // in the new copy of the non-affine region. Make sure to only add the

polly/lib/CodeGen/CodeGeneration.cpp

@@ -77,8 +77,8 @@ namespace polly {
 /// Marks the basic block @p Block unreachable by equipping it with an
 /// UnreachableInst.
 void markBlockUnreachable(BasicBlock &Block, PollyIRBuilder &Builder) {
-  auto *OrigTerminator = Block.getTerminator();
-  Builder.SetInsertPoint(OrigTerminator);
+  auto OrigTerminator = Block.getTerminator()->getIterator();
+  Builder.SetInsertPoint(&Block, OrigTerminator);
   Builder.CreateUnreachable();
   OrigTerminator->eraseFromParent();
 }
@@ -211,7 +211,8 @@ static bool generateCode(Scop &S, IslAstInfo &AI, LoopInfo &LI,
   assert(EnteringBB);
   PollyIRBuilder Builder(EnteringBB->getContext(), ConstantFolder(),
                          IRInserter(Annotator));
-  Builder.SetInsertPoint(EnteringBB->getTerminator());
+  Builder.SetInsertPoint(EnteringBB,
+                         EnteringBB->getTerminator()->getIterator());
 
   // Only build the run-time condition and parameters _after_ having
   // introduced the conditional branch. This is important as the conditional
@@ -257,7 +258,8 @@ static bool generateCode(Scop &S, IslAstInfo &AI, LoopInfo &LI,
   // might reference the hoisted loads. Finally, build the runtime check
   // that might reference both hoisted loads as well as parameters.
   // If the hoisting fails we have to bail and execute the original code.
-  Builder.SetInsertPoint(SplitBlock->getTerminator());
+  Builder.SetInsertPoint(SplitBlock,
+                         SplitBlock->getTerminator()->getIterator());
   if (!NodeBuilder.preloadInvariantLoads()) {
     // Patch the introduced branch condition to ensure that we always execute
     // the original SCoP.
@@ -289,7 +291,8 @@ static bool generateCode(Scop &S, IslAstInfo &AI, LoopInfo &LI,
     // Ideally we would just split the block during allocation of the new
     // arrays, but this would break the assumption that there are no blocks
     // between polly.start and polly.exiting (at this point).
-    Builder.SetInsertPoint(StartBlock->getTerminator());
+    Builder.SetInsertPoint(StartBlock,
+                           StartBlock->getTerminator()->getIterator());
 
     NodeBuilder.create(AstRoot.release());
     NodeBuilder.finalize();

polly/lib/CodeGen/IslExprBuilder.cpp

@@ -625,7 +625,7 @@ IslExprBuilder::createOpBooleanConditional(__isl_take isl_ast_expr *Expr) {
   Builder.SetInsertPoint(CondBB);
   Builder.CreateBr(NextBB);
 
-  Builder.SetInsertPoint(InsertBB->getTerminator());
+  Builder.SetInsertPoint(InsertBB, InsertBB->getTerminator()->getIterator());
 
   LHS = create(isl_ast_expr_get_op_arg(Expr, 0));
   if (!LHS->getType()->isIntegerTy(1))
@@ -637,13 +637,13 @@ IslExprBuilder::createOpBooleanConditional(__isl_take isl_ast_expr *Expr) {
   else
     BR->setCondition(LHS);
 
-  Builder.SetInsertPoint(CondBB->getTerminator());
+  Builder.SetInsertPoint(CondBB, CondBB->getTerminator()->getIterator());
   RHS = create(isl_ast_expr_get_op_arg(Expr, 1));
   if (!RHS->getType()->isIntegerTy(1))
     RHS = Builder.CreateIsNotNull(RHS);
   auto RightBB = Builder.GetInsertBlock();
 
-  Builder.SetInsertPoint(NextBB->getTerminator());
+  Builder.SetInsertPoint(NextBB, NextBB->getTerminator()->getIterator());
   auto PHI = Builder.CreatePHI(Builder.getInt1Ty(), 2);
   PHI->addIncoming(OpType == isl_ast_op_and_then ? Builder.getFalse()
                                                  : Builder.getTrue(),

polly/lib/CodeGen/IslNodeBuilder.cpp

@@ -488,7 +488,7 @@ void IslNodeBuilder::createForSequential(isl::ast_node_for For,
 
   IDToValue.erase(IDToValue.find(IteratorID.get()));
 
-  Builder.SetInsertPoint(&ExitBlock->front());
+  Builder.SetInsertPoint(ExitBlock, ExitBlock->begin());
 
   SequentialLoops++;
 }
@@ -508,7 +508,7 @@ void IslNodeBuilder::createForParallel(__isl_take isl_ast_node *For) {
   BasicBlock *ParBB = SplitBlock(Builder.GetInsertBlock(),
                                  &*Builder.GetInsertPoint(), &DT, &LI);
   ParBB->setName("polly.parallel.for");
-  Builder.SetInsertPoint(&ParBB->front());
+  Builder.SetInsertPoint(ParBB, ParBB->begin());
 
   Body = isl_ast_node_for_get_body(For);
   Init = isl_ast_node_for_get_init(For);
@@ -612,7 +612,7 @@ void IslNodeBuilder::createForParallel(__isl_take isl_ast_node *For) {
   BlockGen.switchGeneratedFunc(SubFn, GenDT, GenLI, GenSE);
   RegionGen.switchGeneratedFunc(SubFn, GenDT, GenLI, GenSE);
   ExprBuilder.switchGeneratedFunc(SubFn, GenDT, GenLI, GenSE);
-  Builder.SetInsertPoint(&*LoopBody);
+  Builder.SetInsertPoint(LoopBody);
 
   // Update the ValueMap to use instructions in the subfunction. Note that
   // "GlobalMap" used in BlockGenerator/IslExprBuilder is a reference to this
@@ -682,7 +682,7 @@ void IslNodeBuilder::createForParallel(__isl_take isl_ast_node *For) {
   ExprBuilder.switchGeneratedFunc(CallerFn, CallerDT, CallerLI, CallerSE);
   RegionGen.switchGeneratedFunc(CallerFn, CallerDT, CallerLI, CallerSE);
   BlockGen.switchGeneratedFunc(CallerFn, CallerDT, CallerLI, CallerSE);
-  Builder.SetInsertPoint(&*AfterLoop);
+  Builder.SetInsertPoint(AfterLoop);
 
   for (const Loop *L : Loops)
     OutsideLoopIterations.erase(L);
@@ -737,16 +737,16 @@ void IslNodeBuilder::createIf(__isl_take isl_ast_node *If) {
   Builder.CreateBr(MergeBB);
   Builder.SetInsertPoint(ElseBB);
   Builder.CreateBr(MergeBB);
-  Builder.SetInsertPoint(&ThenBB->front());
+  Builder.SetInsertPoint(ThenBB, ThenBB->begin());
 
   create(isl_ast_node_if_get_then(If));
 
-  Builder.SetInsertPoint(&ElseBB->front());
+  Builder.SetInsertPoint(ElseBB, ElseBB->begin());
 
   if (isl_ast_node_if_has_else(If))
     create(isl_ast_node_if_get_else(If));
 
-  Builder.SetInsertPoint(&MergeBB->front());
+  Builder.SetInsertPoint(MergeBB, MergeBB->begin());
 
   isl_ast_node_free(If);
 
@@ -1126,16 +1126,16 @@ Value *IslNodeBuilder::preloadInvariantLoad(const MemoryAccess &MA,
     L->addBasicBlockToLoop(ExecBB, *GenLI);
 
   auto *CondBBTerminator = CondBB->getTerminator();
-  Builder.SetInsertPoint(CondBBTerminator);
+  Builder.SetInsertPoint(CondBB, CondBBTerminator->getIterator());
   Builder.CreateCondBr(Cond, ExecBB, MergeBB);
   CondBBTerminator->eraseFromParent();
 
   Builder.SetInsertPoint(ExecBB);
   Builder.CreateBr(MergeBB);
 
-  Builder.SetInsertPoint(ExecBB->getTerminator());
+  Builder.SetInsertPoint(ExecBB, ExecBB->getTerminator()->getIterator());
   Value *PreAccInst = preloadUnconditionally(AccessRange, Build, AccInst);
-  Builder.SetInsertPoint(MergeBB->getTerminator());
+  Builder.SetInsertPoint(MergeBB, MergeBB->getTerminator()->getIterator());
   auto *MergePHI = Builder.CreatePHI(
       AccInstTy, 2, "polly.preload." + AccInst->getName() + ".merge");
   PreloadVal = MergePHI;
@@ -1315,7 +1315,9 @@ void IslNodeBuilder::allocateNewArrays(BBPair StartExitBlocks) {
       unsigned Size = SAI->getElemSizeInBytes();
 
       // Insert the malloc call at polly.start
-      Builder.SetInsertPoint(std::get<0>(StartExitBlocks)->getTerminator());
+      BasicBlock *StartBlock = std::get<0>(StartExitBlocks);
+      Builder.SetInsertPoint(StartBlock,
+                             StartBlock->getTerminator()->getIterator());
       auto *CreatedArray = Builder.CreateMalloc(
           IntPtrTy, SAI->getElementType(),
           ConstantInt::get(Type::getInt64Ty(Ctx), Size),
@@ -1325,7 +1327,9 @@ void IslNodeBuilder::allocateNewArrays(BBPair StartExitBlocks) {
       SAI->setBasePtr(CreatedArray);
 
       // Insert the free call at polly.exiting
-      Builder.SetInsertPoint(std::get<1>(StartExitBlocks)->getTerminator());
+      BasicBlock *ExitingBlock = std::get<1>(StartExitBlocks);
+      Builder.SetInsertPoint(ExitingBlock,
+                             ExitingBlock->getTerminator()->getIterator());
       Builder.CreateFree(CreatedArray);
     } else {
       auto InstIt = Builder.GetInsertBlock()
@@ -1351,7 +1355,7 @@ bool IslNodeBuilder::preloadInvariantLoads() {
   BasicBlock *PreLoadBB = SplitBlock(Builder.GetInsertBlock(),
                                      &*Builder.GetInsertPoint(), GenDT, GenLI);
   PreLoadBB->setName("polly.preload.begin");
-  Builder.SetInsertPoint(&PreLoadBB->front());
+  Builder.SetInsertPoint(PreLoadBB, PreLoadBB->begin());
 
   for (auto &IAClass : InvariantEquivClasses)
     if (!preloadInvariantEquivClass(IAClass))

polly/lib/CodeGen/LoopGenerators.cpp

@@ -200,7 +200,7 @@ Value *ParallelLoopGenerator::createParallelLoop(
   Function *SubFn;
   std::tie(IV, SubFn) = createSubFn(Stride, Struct, UsedValues, Map);
   *LoopBody = Builder.GetInsertPoint();
-  Builder.SetInsertPoint(&*BeforeLoop);
+  Builder.SetInsertPoint(BeforeLoop);
 
   // Add one as the upper bound provided by OpenMP is a < comparison
   // whereas the codegenForSequential function creates a <= comparison.

polly/lib/CodeGen/LoopGeneratorsGOMP.cpp

@@ -148,7 +148,7 @@ ParallelLoopGeneratorGOMP::createSubFn(Value *Stride, AllocaInst *StructData,
                          "polly.par.UBAdjusted");
 
   Builder.CreateBr(CheckNextBB);
-  Builder.SetInsertPoint(&*--Builder.GetInsertPoint());
+  Builder.SetInsertPoint(--Builder.GetInsertPoint());
   BasicBlock *AfterBB;
   Value *IV =
       createLoop(LB, UB, Stride, Builder, *SubFnLI, *SubFnDT, AfterBB,
@@ -161,7 +161,7 @@ ParallelLoopGeneratorGOMP::createSubFn(Value *Stride, AllocaInst *StructData,
   createCallCleanupThread();
   Builder.CreateRetVoid();
 
-  Builder.SetInsertPoint(&*LoopBody);
+  Builder.SetInsertPoint(LoopBody);
 
   // FIXME: Call SubFnDT->verify() and SubFnLI->verify() to check that the
   // DominatorTree/LoopInfo has been created correctly. Alternatively, recreate

polly/lib/CodeGen/LoopGeneratorsKMP.cpp

@@ -282,7 +282,7 @@ ParallelLoopGeneratorKMP::createSubFn(Value *SequentialLoopStride,
   }
 
   Builder.CreateBr(CheckNextBB);
-  Builder.SetInsertPoint(&*--Builder.GetInsertPoint());
+  Builder.SetInsertPoint(--Builder.GetInsertPoint());
   BasicBlock *AfterBB;
   Value *IV = createLoop(LB, UB, SequentialLoopStride, Builder, *SubFnLI,
                          *SubFnDT, AfterBB, ICmpInst::ICMP_SLE, nullptr, true,
@@ -298,7 +298,7 @@ ParallelLoopGeneratorKMP::createSubFn(Value *SequentialLoopStride,
     createCallStaticFini(ID);
   }
   Builder.CreateRetVoid();
-  Builder.SetInsertPoint(&*LoopBody);
+  Builder.SetInsertPoint(LoopBody);
 
   // FIXME: Call SubFnDT->verify() and SubFnLI->verify() to check that the
   // DominatorTree/LoopInfo has been created correctly. Alternatively, recreate

polly/lib/CodeGen/PerfMonitor.cpp

@@ -267,7 +267,7 @@ void PerfMonitor::insertRegionStart(Instruction *InsertBefore) {
   if (!Supported)
     return;
 
-  Builder.SetInsertPoint(InsertBefore);
+  Builder.SetInsertPoint(InsertBefore->getIterator());
   Function *RDTSCPFn = getRDTSCP();
   Value *CurrentCycles =
       Builder.CreateExtractValue(Builder.CreateCall(RDTSCPFn), {0});
@@ -278,7 +278,7 @@ void PerfMonitor::insertRegionEnd(Instruction *InsertBefore) {
   if (!Supported)
     return;
 
-  Builder.SetInsertPoint(InsertBefore);
+  Builder.SetInsertPoint(InsertBefore->getIterator());
   Function *RDTSCPFn = getRDTSCP();
   Type *Int64Ty = Builder.getInt64Ty();
   LoadInst *CyclesStart =