[Mlir-commits] [llvm] [mlir] [IRBuilder] Remove insertion block (PR #234245)
Nikita Popov
llvmlistbot at llvm.org
Thu Oct 22 02:26:16 PDT 2026
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/234245
>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 1/2] [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)))
>From 9324cd38ec8477f301835ab97ac57499af60b781 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Thu, 22 Oct 2026 10:25:53 +0200
Subject: [PATCH 2/2] Drop assert that doesn't make sense
---
llvm/include/llvm/IR/IRBuilder.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/llvm/include/llvm/IR/IRBuilder.h b/llvm/include/llvm/IR/IRBuilder.h
index 4d545a7b5c50c2..f01e66954f4560 100644
--- a/llvm/include/llvm/IR/IRBuilder.h
+++ b/llvm/include/llvm/IR/IRBuilder.h
@@ -199,8 +199,6 @@ class IRBuilderBase {
/// the specified instruction.
void SetInsertPoint(Instruction *I) {
InsertPt = I->getIterator();
- assert(InsertPt != I->getParent()->end() &&
- "Can't read debug loc from end()");
SetCurrentDebugLocation(I->getDebugLoc());
}
More information about the Mlir-commits
mailing list