[Mlir-commits] [llvm] [mlir] [IRBuilder] Remove insertion block (PR #234245)
Nikita Popov
llvmlistbot at llvm.org
Wed Oct 21 08:50:48 PDT 2026
https://github.com/nikic created https://github.com/llvm/llvm-project/pull/234245
The insertion point already stores the block it belongs to, so we should not be storing a separate insertion block.
Removing this requires some fixes, because there are a few places where the insertion point gets invalidated, and the insertion block is still accessed afterwards. In cases like this, we should save the insertion block before iterator invalidation occurs.
Completes the work started in https://github.com/llvm/llvm-project/pull/228117.
>From a9365fc0fe3f5ad225ab27586d8cc236b594feb1 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 7 Oct 2026 15:24:59 +0200
Subject: [PATCH] [IRBuilder] Remove insertion block
The insertion point already stores the block it belongs to, so we
should not be storing a separate insertion block.
Removing this requires some fixes, because there are a few places
where the insertion point gets invalidated, and the insertion block
is still accessed afterwards. In cases like this, we should save
the insertion block before iterator invalidation occurs.
---
llvm/include/llvm/IR/IRBuilder.h | 37 ++++++++-----------
llvm/lib/CodeGen/AtomicExpandPass.cpp | 3 +-
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 10 +++--
llvm/lib/IR/IRBuilder.cpp | 1 +
.../Instrumentation/HWAddressSanitizer.cpp | 24 ++++++------
.../OpenMP/OpenMPToLLVMIRTranslation.cpp | 13 +++++--
6 files changed, 46 insertions(+), 42 deletions(-)
diff --git a/llvm/include/llvm/IR/IRBuilder.h b/llvm/include/llvm/IR/IRBuilder.h
index 2613998413a3b0..4d545a7b5c50c2 100644
--- a/llvm/include/llvm/IR/IRBuilder.h
+++ b/llvm/include/llvm/IR/IRBuilder.h
@@ -117,9 +117,6 @@ class IRBuilderBase {
using InsertFn = void (*)(const IRBuilderBase &, Instruction *, const Twine &,
BasicBlock::iterator);
- // TODO: Remove this in favor of InsertPt.getNodeParent(), so they cannot
- // go out of sync.
- BasicBlock *BB;
BasicBlock::iterator InsertPt;
LLVMContext &Context;
const IRBuilderFolder &Folder;
@@ -171,42 +168,39 @@ class IRBuilderBase {
/// Clear the insertion point: created instructions will not be
/// inserted into a block.
- void ClearInsertionPoint() {
- BB = nullptr;
- InsertPt = BasicBlock::iterator();
- }
+ void ClearInsertionPoint() { InsertPt = BasicBlock::iterator(); }
- BasicBlock *GetInsertBlock() const { return BB; }
+ BasicBlock *GetInsertBlock() const {
+ return InsertPt.isValid() ? InsertPt.getNodeParent() : nullptr;
+ }
BasicBlock::iterator GetInsertPoint() const { return InsertPt; }
LLVMContext &getContext() const { return Context; }
/// Get data layout. Requires that an insertion point is set and connected
/// to a module.
const DataLayout &getDataLayout() const {
- assert(BB && "Must have insertion point to get data layout");
- return BB->getDataLayout();
+ assert(InsertPt.isValid() &&
+ "Must have insertion point to get data layout");
+ return InsertPt.getNodeParent()->getDataLayout();
}
/// Get the module. Requires that an insertion point is set and connected
/// to a module.
Module *getModule() const {
- assert(BB && "Must have insertion point to get module");
- return BB->getModule();
+ assert(InsertPt.isValid() && "Must have insertion point to get module");
+ return InsertPt.getNodeParent()->getModule();
}
/// This specifies that created instructions should be appended to the
/// end of the specified block.
- void SetInsertPoint(BasicBlock *TheBB) {
- BB = TheBB;
- InsertPt = BB->end();
- }
+ void SetInsertPoint(BasicBlock *TheBB) { InsertPt = TheBB->end(); }
/// This specifies that created instructions should be inserted before
/// the specified instruction.
void SetInsertPoint(Instruction *I) {
- BB = I->getParent();
InsertPt = I->getIterator();
- assert(InsertPt != BB->end() && "Can't read debug loc from end()");
+ assert(InsertPt != I->getParent()->end() &&
+ "Can't read debug loc from end()");
SetCurrentDebugLocation(I->getDebugLoc());
}
@@ -220,9 +214,8 @@ class IRBuilderBase {
/// This specifies that created instructions should be inserted at
/// the specified point.
void SetInsertPoint(BasicBlock::iterator IP) {
- BB = IP.getNodeParent();
InsertPt = IP;
- if (IP != BB->end())
+ if (IP != IP.getNodeParent()->end())
SetCurrentDebugLocation(IP->getDebugLoc());
}
@@ -230,8 +223,7 @@ class IRBuilderBase {
/// end of the specified function, but after already existing static alloca
/// instructions that are at the start.
void SetInsertPointPastAllocas(Function *F) {
- BB = &F->getEntryBlock();
- InsertPt = BB->getFirstNonPHIOrAlloca();
+ InsertPt = F->getEntryBlock().getFirstNonPHIOrAlloca();
}
/// Set location information used by debugging information.
@@ -337,6 +329,7 @@ class IRBuilderBase {
}
void setConstrainedFPFunctionAttr() {
+ BasicBlock *BB = GetInsertBlock();
assert(BB && "Must have a basic block to set any function attributes!");
Function *F = BB->getParent();
diff --git a/llvm/lib/CodeGen/AtomicExpandPass.cpp b/llvm/lib/CodeGen/AtomicExpandPass.cpp
index b5ed03aa479897..768550ca342297 100644
--- a/llvm/lib/CodeGen/AtomicExpandPass.cpp
+++ b/llvm/lib/CodeGen/AtomicExpandPass.cpp
@@ -187,7 +187,8 @@ struct ReplacementIRBuilder
: IRBuilder(
I->getIterator(), InstSimplifyFolder(DL),
IRBuilderCallbackInserter([this](Instruction *I) { addMD(I); })) {
- if (BB->getParent()->getAttributes().hasFnAttr(Attribute::StrictFP))
+ if (GetInsertBlock()->getParent()->getAttributes().hasFnAttr(
+ Attribute::StrictFP))
this->setIsFPConstrained(true);
MMRAMD = I->getMetadata(LLVMContext::MD_mmra);
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index c8695bf2b57a9a..37d5d7dece0b20 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -425,11 +425,12 @@ BasicBlock *llvm::splitBB(IRBuilderBase::InsertPoint IP, bool CreateBranch,
BasicBlock *llvm::splitBB(IRBuilderBase &Builder, bool CreateBranch,
llvm::Twine Name) {
DebugLoc DebugLoc = Builder.getCurrentDebugLocation();
+ BasicBlock *Old = Builder.GetInsertBlock();
BasicBlock *New = splitBB(Builder.saveIP(), CreateBranch, DebugLoc, Name);
if (CreateBranch)
- Builder.SetInsertPoint(Builder.GetInsertBlock()->getTerminator());
+ Builder.SetInsertPoint(Old->getTerminator());
else
- Builder.SetInsertPoint(Builder.GetInsertBlock());
+ Builder.SetInsertPoint(Old);
// SetInsertPoint also updates the Builder's debug location, but we want to
// keep the one the Builder was configured to use.
Builder.SetCurrentDebugLocation(DebugLoc);
@@ -439,11 +440,12 @@ BasicBlock *llvm::splitBB(IRBuilderBase &Builder, bool CreateBranch,
BasicBlock *llvm::splitBB(IRBuilder<> &Builder, bool CreateBranch,
llvm::Twine Name) {
DebugLoc DebugLoc = Builder.getCurrentDebugLocation();
+ BasicBlock *Old = Builder.GetInsertBlock();
BasicBlock *New = splitBB(Builder.saveIP(), CreateBranch, DebugLoc, Name);
if (CreateBranch)
- Builder.SetInsertPoint(Builder.GetInsertBlock()->getTerminator());
+ Builder.SetInsertPoint(Old->getTerminator());
else
- Builder.SetInsertPoint(Builder.GetInsertBlock());
+ Builder.SetInsertPoint(Old);
// SetInsertPoint also updates the Builder's debug location, but we want to
// keep the one the Builder was configured to use.
Builder.SetCurrentDebugLocation(DebugLoc);
diff --git a/llvm/lib/IR/IRBuilder.cpp b/llvm/lib/IR/IRBuilder.cpp
index cf4b81bad842cd..b42516b006c7c9 100644
--- a/llvm/lib/IR/IRBuilder.cpp
+++ b/llvm/lib/IR/IRBuilder.cpp
@@ -58,6 +58,7 @@ GlobalVariable *IRBuilderBase::CreateGlobalString(StringRef Str,
}
Type *IRBuilderBase::getCurrentFunctionReturnType() const {
+ BasicBlock *BB = GetInsertBlock();
assert(BB && BB->getParent() && "No current function!");
return BB->getParent()->getReturnType();
}
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 0097f333c2ef63..bdab6c771483f0 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1539,19 +1539,21 @@ void HWAddressSanitizer::sanitizeFunction(Function &F,
const LoopInfo &LI = FAM.getResult<LoopAnalysis>(F);
Value *StackTag = getStackBaseTag(EntryIRB);
Value *UARTag = getUARTag(EntryIRB);
+ // instrumentStack may replace allocas, invalidating EntryIRB's insert
+ // point. Save the insert block before calling it.
+ BasicBlock *EntryBB = EntryIRB.GetInsertBlock();
instrumentStack(ORE, SInfo, StackTag, UARTag, DT, PDT, LI);
- }
- // If we split the entry block, move any allocas that were originally in the
- // entry block back into the entry block so that they aren't treated as
- // dynamic allocas.
- if (EntryIRB.GetInsertBlock() != &F.getEntryBlock()) {
- InsertPt = F.getEntryBlock().begin();
- for (Instruction &I :
- llvm::make_early_inc_range(*EntryIRB.GetInsertBlock())) {
- if (auto *AI = dyn_cast<AllocaInst>(&I))
- if (isa<ConstantInt>(AI->getArraySize()))
- I.moveBefore(F.getEntryBlock(), InsertPt);
+ // If we split the entry block, move any allocas that were originally in the
+ // entry block back into the entry block so that they aren't treated as
+ // dynamic allocas.
+ if (EntryBB != &F.getEntryBlock()) {
+ InsertPt = F.getEntryBlock().begin();
+ for (Instruction &I : llvm::make_early_inc_range(*EntryBB)) {
+ if (auto *AI = dyn_cast<AllocaInst>(&I))
+ if (isa<ConstantInt>(AI->getArraySize()))
+ I.moveBefore(F.getEntryBlock(), InsertPt);
+ }
}
}
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index b10cf42717945b..bf990b44131d83 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -1187,13 +1187,18 @@ static LogicalResult inlineConvertOmpRegions(
// Special case for single-block regions that don't create additional blocks:
// insert operations without creating additional blocks.
if (region.hasOneBlock()) {
+ llvm::BasicBlock *insertBlock = builder.GetInsertBlock();
llvm::Instruction *potentialTerminator =
- builder.GetInsertBlock()->empty() ? nullptr
- : &builder.GetInsertBlock()->back();
+ insertBlock->empty() ? nullptr : &insertBlock->back();
- if (potentialTerminator && potentialTerminator->isTerminator())
+ if (potentialTerminator && potentialTerminator->isTerminator()) {
+ assert(builder.GetInsertPoint() == potentialTerminator->getIterator() &&
+ "Insertion point should be the terminator");
potentialTerminator->removeFromParent();
- moduleTranslation.mapBlock(®ion.front(), builder.GetInsertBlock());
+ // Reset insertion point to the end of the block.
+ builder.SetInsertPoint(insertBlock);
+ }
+ moduleTranslation.mapBlock(®ion.front(), insertBlock);
if (failed(moduleTranslation.convertBlock(
region.front(), /*ignoreArguments=*/true, builder)))
More information about the Mlir-commits
mailing list