From ecdd3fd71f41f600623e93bad08f9e41b1d0d8cc Mon Sep 17 00:00:00 2001 From: Karthika Devi C Date: Thu, 24 Apr 2025 18:31:48 +0530 Subject: [PATCH] [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. --- polly/lib/CodeGen/BlockGenerators.cpp | 24 ++++++++++--------- polly/lib/CodeGen/CodeGeneration.cpp | 13 ++++++---- polly/lib/CodeGen/IslExprBuilder.cpp | 6 ++--- polly/lib/CodeGen/IslNodeBuilder.cpp | 30 ++++++++++++++---------- polly/lib/CodeGen/LoopGenerators.cpp | 2 +- polly/lib/CodeGen/LoopGeneratorsGOMP.cpp | 4 ++-- polly/lib/CodeGen/LoopGeneratorsKMP.cpp | 4 ++-- polly/lib/CodeGen/PerfMonitor.cpp | 4 ++-- 8 files changed, 48 insertions(+), 39 deletions(-) diff --git a/polly/lib/CodeGen/BlockGenerators.cpp b/polly/lib/CodeGen/BlockGenerators.cpp index cf2cc65e0f04..3c6909f3eb70 100644 --- a/polly/lib/CodeGen/BlockGenerators.cpp +++ b/polly/lib/CodeGen/BlockGenerators.cpp @@ -420,7 +420,7 @@ BasicBlock *BlockGenerator::copyBB(ScopStmt &Stmt, BasicBlock *BB, ValueMapT &BBMap, LoopToScevMapT <S, 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 <S, 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 <S, 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 <S, 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 <S, } // 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 <S, 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 diff --git a/polly/lib/CodeGen/CodeGeneration.cpp b/polly/lib/CodeGen/CodeGeneration.cpp index f5e29c38e290..2d8b393cc039 100644 --- a/polly/lib/CodeGen/CodeGeneration.cpp +++ b/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(); diff --git a/polly/lib/CodeGen/IslExprBuilder.cpp b/polly/lib/CodeGen/IslExprBuilder.cpp index 8c54436f295b..eecc616724b8 100644 --- a/polly/lib/CodeGen/IslExprBuilder.cpp +++ b/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(), diff --git a/polly/lib/CodeGen/IslNodeBuilder.cpp b/polly/lib/CodeGen/IslNodeBuilder.cpp index e818dab4f9c0..1217b620eed8 100644 --- a/polly/lib/CodeGen/IslNodeBuilder.cpp +++ b/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)) diff --git a/polly/lib/CodeGen/LoopGenerators.cpp b/polly/lib/CodeGen/LoopGenerators.cpp index f3975ccee44f..10d96eb10b70 100644 --- a/polly/lib/CodeGen/LoopGenerators.cpp +++ b/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. diff --git a/polly/lib/CodeGen/LoopGeneratorsGOMP.cpp b/polly/lib/CodeGen/LoopGeneratorsGOMP.cpp index 61c153d2ccfa..7b6d63a8ae18 100644 --- a/polly/lib/CodeGen/LoopGeneratorsGOMP.cpp +++ b/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 diff --git a/polly/lib/CodeGen/LoopGeneratorsKMP.cpp b/polly/lib/CodeGen/LoopGeneratorsKMP.cpp index 0cfe18b0c121..0973191e003c 100644 --- a/polly/lib/CodeGen/LoopGeneratorsKMP.cpp +++ b/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 diff --git a/polly/lib/CodeGen/PerfMonitor.cpp b/polly/lib/CodeGen/PerfMonitor.cpp index 4c1eab005084..7b559e990b7c 100644 --- a/polly/lib/CodeGen/PerfMonitor.cpp +++ b/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 =