[mlir] Use OpBuilder::createBlock in op builders and patterns
#82770
+71
−88
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matthias-springer
requested review from
aartbik,
PeimingLiu,
yinying-lisa-li,
antiagainst,
kuhar,
MaheshRavishankar,
nicolasvasilache and
dcaballe
as code owners
llvmbot
added
mlir:llvm
mlir:linalg
mlir:spirv
mlir:affine
mlir:sparse
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-linalg Author: Matthias Springer (matthias-springer) ChangesWhen creating a new block in (conversion) rewrite patterns, Note: The dialect conversion relies on listener notifications to keep track of IR modifications. Creating blocks without the builder API can lead to memory leaks during rollback. Patch is 25.57 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82770.diff 20 Files Affected:
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 3da5deeb4ec7e2..b523374f6c06b5 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1456,7 +1456,7 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
let extraClassDeclaration = [{
// Add an entry block to an empty function, and set up the block arguments
// to match the signature of the function.
- Block *addEntryBlock();
+ Block *addEntryBlock(OpBuilder &builder);
bool isVarArg() { return getFunctionType().isVarArg(); }
diff --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
index 36ad6755cab25e..991e753d1b3593 100644
--- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
+++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
@@ -285,7 +285,7 @@ def SPIRV_LoopOp : SPIRV_Op<"mlir.loop", [InFunctionScope]> {
// Adds an empty entry block and loop merge block containing one
// spirv.mlir.merge op.
- void addEntryAndMergeBlock();
+ void addEntryAndMergeBlock(OpBuilder &builder);
}];
let hasOpcode = 0;
@@ -427,7 +427,7 @@ def SPIRV_SelectionOp : SPIRV_Op<"mlir.selection", [InFunctionScope]> {
Block *getMergeBlock();
/// Adds a selection merge block containing one spirv.mlir.merge op.
- void addMergeBlock();
+ void addMergeBlock(OpBuilder &builder);
/// Creates a spirv.mlir.selection op for `if (<condition>) then { <thenBody> }`
/// with `builder`. `builder`'s insertion point will remain at after the
diff --git a/mlir/include/mlir/Interfaces/FunctionInterfaces.td b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
index 970a781c998b98..873853eba0b175 100644
--- a/mlir/include/mlir/Interfaces/FunctionInterfaces.td
+++ b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
@@ -131,6 +131,7 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
static void buildWithEntryBlock(
OpBuilder &builder, OperationState &state, StringRef name, Type type,
ArrayRef<NamedAttribute> attrs, TypeRange inputTypes) {
+ OpBuilder::InsertionGuard g(builder);
state.addAttribute(SymbolTable::getSymbolAttrName(),
builder.getStringAttr(name));
state.addAttribute(ConcreteOp::getFunctionTypeAttrName(state.name),
@@ -139,8 +140,7 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
// Add the function body.
Region *bodyRegion = state.addRegion();
- Block *body = new Block();
- bodyRegion->push_back(body);
+ Block *body = builder.createBlock(bodyRegion);
for (Type input : inputTypes)
body->addArgument(input, state.location);
}
diff --git a/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp b/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
index 0ab53ce7e3327e..77603739137614 100644
--- a/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
+++ b/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
@@ -259,7 +259,7 @@ static void addResumeFunction(ModuleOp module) {
kResume, LLVM::LLVMFunctionType::get(voidTy, {ptrType}));
resumeOp.setPrivate();
- auto *block = resumeOp.addEntryBlock();
+ auto *block = resumeOp.addEntryBlock(moduleBuilder);
auto blockBuilder = ImplicitLocOpBuilder::atBlockEnd(loc, block);
blockBuilder.create<LLVM::CoroResumeOp>(resumeOp.getArgument(0));
diff --git a/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp b/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp
index 363e5f9b8cefe7..d3ee89743da9db 100644
--- a/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp
+++ b/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp
@@ -98,12 +98,10 @@ ControlFlowToSCFTransformation::createStructuredDoWhileLoopOp(
loc, builder.create<arith::TruncIOp>(loc, builder.getI1Type(), condition),
loopVariablesNextIter);
- auto *afterBlock = new Block;
- whileOp.getAfter().push_back(afterBlock);
+ Block *afterBlock = builder.createBlock(&whileOp.getAfter());
afterBlock->addArguments(
loopVariablesInit.getTypes(),
SmallVector<Location>(loopVariablesInit.size(), loc));
- builder.setInsertionPointToEnd(afterBlock);
builder.create<scf::YieldOp>(loc, afterBlock->getArguments());
return whileOp.getOperation();
diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index bd50c67fb87958..53b44aa3241bb1 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -135,7 +135,7 @@ static void wrapForExternalCallers(OpBuilder &rewriter, Location loc,
propagateArgResAttrs(rewriter, !!resultStructType, funcOp, wrapperFuncOp);
OpBuilder::InsertionGuard guard(rewriter);
- rewriter.setInsertionPointToStart(wrapperFuncOp.addEntryBlock());
+ rewriter.setInsertionPointToStart(wrapperFuncOp.addEntryBlock(rewriter));
SmallVector<Value, 8> args;
size_t argOffset = resultStructType ? 1 : 0;
@@ -203,7 +203,7 @@ static void wrapExternalFunction(OpBuilder &builder, Location loc,
// The wrapper that we synthetize here should only be visible in this module.
newFuncOp.setLinkage(LLVM::Linkage::Private);
- builder.setInsertionPointToStart(newFuncOp.addEntryBlock());
+ builder.setInsertionPointToStart(newFuncOp.addEntryBlock(builder));
// Get a ValueRange containing arguments.
FunctionType type = cast<FunctionType>(funcOp.getFunctionType());
diff --git a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
index 2bfca303b5fd48..2dc42f0a85e669 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
@@ -520,9 +520,7 @@ struct GlobalMemrefOpLowering
global, arrayTy, global.getConstant(), linkage, global.getSymName(),
initialValue, alignment, *addressSpace);
if (!global.isExternal() && global.isUninitialized()) {
- Block *blk = new Block();
- newGlobal.getInitializerRegion().push_back(blk);
- rewriter.setInsertionPointToStart(blk);
+ rewriter.createBlock(&newGlobal.getInitializerRegion());
Value undef[] = {
rewriter.create<LLVM::UndefOp>(global.getLoc(), arrayTy)};
rewriter.create<LLVM::ReturnOp>(global.getLoc(), undef);
diff --git a/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp b/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
index febfe97f6c0a99..d90cf931385fcc 100644
--- a/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
+++ b/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
@@ -138,14 +138,13 @@ struct ForOpConversion final : SCFToSPIRVPattern<scf::ForOp> {
// from header to merge.
auto loc = forOp.getLoc();
auto loopOp = rewriter.create<spirv::LoopOp>(loc, spirv::LoopControl::None);
- loopOp.addEntryAndMergeBlock();
+ loopOp.addEntryAndMergeBlock(rewriter);
OpBuilder::InsertionGuard guard(rewriter);
// Create the block for the header.
- auto *header = new Block();
- // Insert the header.
- loopOp.getBody().getBlocks().insert(getBlockIt(loopOp.getBody(), 1),
- header);
+ Block *header = rewriter.createBlock(&loopOp.getBody(),
+ getBlockIt(loopOp.getBody(), 1));
+ rewriter.setInsertionPointAfter(loopOp);
// Create the new induction variable to use.
Value adapLowerBound = adaptor.getLowerBound();
@@ -342,7 +341,7 @@ struct WhileOpConversion final : SCFToSPIRVPattern<scf::WhileOp> {
ConversionPatternRewriter &rewriter) const override {
auto loc = whileOp.getLoc();
auto loopOp = rewriter.create<spirv::LoopOp>(loc, spirv::LoopControl::None);
- loopOp.addEntryAndMergeBlock();
+ loopOp.addEntryAndMergeBlock(rewriter);
Region &beforeRegion = whileOp.getBefore();
Region &afterRegion = whileOp.getAfter();
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index c4b13193f4e773..a4df863ab08342 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1813,6 +1813,8 @@ void AffineForOp::build(OpBuilder &builder, OperationState &result,
"upper bound operand count does not match the affine map");
assert(step > 0 && "step has to be a positive integer constant");
+ OpBuilder::InsertionGuard guard(builder);
+
// Set variadic segment sizes.
result.addAttribute(
getOperandSegmentSizeAttr(),
@@ -1841,12 +1843,11 @@ void AffineForOp::build(OpBuilder &builder, OperationState &result,
// Create a region and a block for the body. The argument of the region is
// the loop induction variable.
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block);
- Block &bodyBlock = bodyRegion->front();
+ Block *bodyBlock = builder.createBlock(bodyRegion);
Value inductionVar =
- bodyBlock.addArgument(builder.getIndexType(), result.location);
+ bodyBlock->addArgument(builder.getIndexType(), result.location);
for (Value val : iterArgs)
- bodyBlock.addArgument(val.getType(), val.getLoc());
+ bodyBlock->addArgument(val.getType(), val.getLoc());
// Create the default terminator if the builder is not provided and if the
// iteration arguments are not provided. Otherwise, leave this to the caller
@@ -1855,9 +1856,9 @@ void AffineForOp::build(OpBuilder &builder, OperationState &result,
ensureTerminator(*bodyRegion, builder, result.location);
} else if (bodyBuilder) {
OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
+ builder.setInsertionPointToStart(bodyBlock);
bodyBuilder(builder, result.location, inductionVar,
- bodyBlock.getArguments().drop_front());
+ bodyBlock->getArguments().drop_front());
}
}
@@ -2895,18 +2896,20 @@ void AffineIfOp::build(OpBuilder &builder, OperationState &result,
TypeRange resultTypes, IntegerSet set, ValueRange args,
bool withElseRegion) {
assert(resultTypes.empty() || withElseRegion);
+ OpBuilder::InsertionGuard guard(builder);
+
result.addTypes(resultTypes);
result.addOperands(args);
result.addAttribute(getConditionAttrStrName(), IntegerSetAttr::get(set));
Region *thenRegion = result.addRegion();
- thenRegion->push_back(new Block());
+ builder.createBlock(thenRegion);
if (resultTypes.empty())
AffineIfOp::ensureTerminator(*thenRegion, builder, result.location);
Region *elseRegion = result.addRegion();
if (withElseRegion) {
- elseRegion->push_back(new Block());
+ builder.createBlock(elseRegion);
if (resultTypes.empty())
AffineIfOp::ensureTerminator(*elseRegion, builder, result.location);
}
@@ -3693,6 +3696,7 @@ void AffineParallelOp::build(OpBuilder &builder, OperationState &result,
"expected upper bound maps to have as many inputs as upper bound "
"operands");
+ OpBuilder::InsertionGuard guard(builder);
result.addTypes(resultTypes);
// Convert the reductions to integer attributes.
@@ -3738,11 +3742,11 @@ void AffineParallelOp::build(OpBuilder &builder, OperationState &result,
// Create a region and a block for the body.
auto *bodyRegion = result.addRegion();
- auto *body = new Block();
+ Block *body = builder.createBlock(bodyRegion);
+
// Add all the block arguments.
for (unsigned i = 0, e = steps.size(); i < e; ++i)
body->addArgument(IndexType::get(builder.getContext()), result.location);
- bodyRegion->push_back(body);
if (resultTypes.empty())
ensureTerminator(*bodyRegion, builder, result.location);
}
diff --git a/mlir/lib/Dialect/Async/IR/Async.cpp b/mlir/lib/Dialect/Async/IR/Async.cpp
index 5f583f36cd2cb8..a3e3f80954efce 100644
--- a/mlir/lib/Dialect/Async/IR/Async.cpp
+++ b/mlir/lib/Dialect/Async/IR/Async.cpp
@@ -68,7 +68,7 @@ void ExecuteOp::getSuccessorRegions(RegionBranchPoint point,
void ExecuteOp::build(OpBuilder &builder, OperationState &result,
TypeRange resultTypes, ValueRange dependencies,
ValueRange operands, BodyBuilderFn bodyBuilder) {
-
+ OpBuilder::InsertionGuard guard(builder);
result.addOperands(dependencies);
result.addOperands(operands);
@@ -87,26 +87,21 @@ void ExecuteOp::build(OpBuilder &builder, OperationState &result,
// Add a body region with block arguments as unwrapped async value operands.
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block);
- Block &bodyBlock = bodyRegion->front();
+ Block *bodyBlock = builder.createBlock(bodyRegion);
for (Value operand : operands) {
auto valueType = llvm::dyn_cast<ValueType>(operand.getType());
- bodyBlock.addArgument(valueType ? valueType.getValueType()
- : operand.getType(),
- operand.getLoc());
+ bodyBlock->addArgument(valueType ? valueType.getValueType()
+ : operand.getType(),
+ operand.getLoc());
}
// Create the default terminator if the builder is not provided and if the
// expected result is empty. Otherwise, leave this to the caller
// because we don't know which values to return from the execute op.
if (resultTypes.empty() && !bodyBuilder) {
- OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
builder.create<async::YieldOp>(result.location, ValueRange());
} else if (bodyBuilder) {
- OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
- bodyBuilder(builder, result.location, bodyBlock.getArguments());
+ bodyBuilder(builder, result.location, bodyBlock->getArguments());
}
}
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index 0fe2c0dcfc7c53..4df8149b94c95f 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -261,20 +261,20 @@ LogicalResult ExpressionOp::verify() {
void ForOp::build(OpBuilder &builder, OperationState &result, Value lb,
Value ub, Value step, BodyBuilderFn bodyBuilder) {
+ OpBuilder::InsertionGuard g(builder);
result.addOperands({lb, ub, step});
Type t = lb.getType();
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block);
- Block &bodyBlock = bodyRegion->front();
- bodyBlock.addArgument(t, result.location);
+ Block *bodyBlock = builder.createBlock(bodyRegion);
+ bodyBlock->addArgument(t, result.location);
// Create the default terminator if the builder is not provided.
if (!bodyBuilder) {
ForOp::ensureTerminator(*bodyRegion, builder, result.location);
} else {
OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
- bodyBuilder(builder, result.location, bodyBlock.getArgument(0));
+ builder.setInsertionPointToStart(bodyBlock);
+ bodyBuilder(builder, result.location, bodyBlock->getArgument(0));
}
}
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index f4042a60541a6a..3ba6ac6ccc8142 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2165,11 +2165,10 @@ LogicalResult ShuffleVectorOp::verify() {
//===----------------------------------------------------------------------===//
// Add the entry block to the function.
-Block *LLVMFuncOp::addEntryBlock() {
+Block *LLVMFuncOp::addEntryBlock(OpBuilder &builder) {
assert(empty() && "function already has an entry block");
-
- auto *entry = new Block;
- push_back(entry);
+ OpBuilder::InsertionGuard g(builder);
+ Block *entry = builder.createBlock(&getBody());
// FIXME: Allow passing in proper locations for the entry arguments.
LLVMFunctionType type = getFunctionType();
diff --git a/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp b/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
index 559ffda4494d2b..c46e3694b70ecd 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
@@ -132,12 +132,10 @@ struct MoveInitOperandsToInput : public OpRewritePattern<GenericOp> {
newIndexingMaps, genericOp.getIteratorTypesArray(),
/*bodyBuild=*/nullptr, linalg::getPrunedAttributeList(genericOp));
+ OpBuilder::InsertionGuard guard(rewriter);
Region ®ion = newOp.getRegion();
- Block *block = new Block();
- region.push_back(block);
+ Block *block = rewriter.createBlock(®ion);
IRMapping mapper;
- OpBuilder::InsertionGuard guard(rewriter);
- rewriter.setInsertionPointToStart(block);
for (auto bbarg : genericOp.getRegionInputArgs())
mapper.map(bbarg, block->addArgument(bbarg.getType(), loc));
diff --git a/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp b/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp
index 286b07669a47f5..0d8d670904f2a8 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp
@@ -178,11 +178,9 @@ static void generateFusedElementwiseOpRegion(
// Build the region of the fused op.
Block &producerBlock = producer->getRegion(0).front();
Block &consumerBlock = consumer->getRegion(0).front();
- Block *fusedBlock = new Block();
- fusedOp.getRegion().push_back(fusedBlock);
- IRMapping mapper;
OpBuilder::InsertionGuard guard(rewriter);
- rewriter.setInsertionPointToStart(fusedBlock);
+ Block *fusedBlock = rewriter.createBlock(&fusedOp.getRegion());
+ IRMapping mapper;
// 2. Add an index operation for every fused loop dimension and use the
// `consumerToProducerLoopsMap` to map the producer indices.
diff --git a/mlir/lib/Dialect/Linalg/Utils/Utils.cpp b/mlir/lib/Dialect/Linalg/Utils/Utils.cpp
index 5d220c6cdd7e58..6392c83bde24f5 100644
--- a/mlir/lib/Dialect/Linalg/Utils/Utils.cpp
+++ b/mlir/lib/Dialect/Linalg/Utils/Utils.cpp
@@ -275,14 +275,13 @@ GenericOp makeTransposeOp(OpBuilder &b, Location loc, Value inputTensor,
auto transposeOp =
b.create<GenericOp>(loc, resultTensorType, inputTensor, outputTensor,
indexingMaps, iteratorTypes);
- Region &body = transposeOp.getRegion();
- body.push_back(new Block());
- body.front().addArguments({elementType, elementType}, {loc, loc});
// Create the body of the transpose operation.
OpBuilder::InsertionGuard g(b);
- b.setInsertionPointToEnd(&body.front());
- b.create<YieldOp>(loc, transposeOp.getRegion().front().getArgument(0));
+ Region &body = transposeOp.getRegion();
+ Block *bodyBlock = b.createBlock(&body);
+ bodyBlock->addArguments({elementType, elementType}, {loc, loc});
+ b.create<YieldOp>(loc, bodyBlock->getArgument(0));
return transposeOp;
}
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index beb7e721ca53b8..248193481acfc6 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -1420,6 +1420,7 @@ OpFoldResult ExtractStridedMetadataOp::getConstifiedMixedOffset() {
void GenericAtomicRMWOp::build(OpBuilder &builder, OperationState &result,
Value memref, ValueRange ivs) {
+ OpBuilder::InsertionGuard g(builder);
result.addOperands(memref);
result.addOperands(ivs);
@@ -1428,7 +1429,7 @@ void GenericAtomicRMWOp::build(OpBuilder &builder, OperationState &result,
result.addTypes(elementType);
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block());
+ builder.createBlock(bodyRegion);
bodyRegion->addArgument(elementType, memref.getLoc());
}
}
diff --git a/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp b/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
index 580782043c81b4..7170a899069ee3 100644
--- a/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
@@ -365,12 +365,11 @@ Block *LoopOp::getMergeBlock() {
return &getBody().back();
}
-void LoopOp::addEntryAndMergeBlock() {
+void LoopOp::addEntryAndMergeBlock(OpBuilder &builder) {
assert(getBody().empty() && "entry and merge block already exist");
- getBody().p...
[truncated]
|
|
@llvm/pr-subscribers-mlir-emitc Author: Matthias Springer (matthias-springer) ChangesWhen creating a new block in (conversion) rewrite patterns, Note: The dialect conversion relies on listener notifications to keep track of IR modifications. Creating blocks without the builder API can lead to memory leaks during rollback. Patch is 25.57 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82770.diff 20 Files Affected:
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 3da5deeb4ec7e2..b523374f6c06b5 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1456,7 +1456,7 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
let extraClassDeclaration = [{
// Add an entry block to an empty function, and set up the block arguments
// to match the signature of the function.
- Block *addEntryBlock();
+ Block *addEntryBlock(OpBuilder &builder);
bool isVarArg() { return getFunctionType().isVarArg(); }
diff --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
index 36ad6755cab25e..991e753d1b3593 100644
--- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
+++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
@@ -285,7 +285,7 @@ def SPIRV_LoopOp : SPIRV_Op<"mlir.loop", [InFunctionScope]> {
// Adds an empty entry block and loop merge block containing one
// spirv.mlir.merge op.
- void addEntryAndMergeBlock();
+ void addEntryAndMergeBlock(OpBuilder &builder);
}];
let hasOpcode = 0;
@@ -427,7 +427,7 @@ def SPIRV_SelectionOp : SPIRV_Op<"mlir.selection", [InFunctionScope]> {
Block *getMergeBlock();
/// Adds a selection merge block containing one spirv.mlir.merge op.
- void addMergeBlock();
+ void addMergeBlock(OpBuilder &builder);
/// Creates a spirv.mlir.selection op for `if (<condition>) then { <thenBody> }`
/// with `builder`. `builder`'s insertion point will remain at after the
diff --git a/mlir/include/mlir/Interfaces/FunctionInterfaces.td b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
index 970a781c998b98..873853eba0b175 100644
--- a/mlir/include/mlir/Interfaces/FunctionInterfaces.td
+++ b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
@@ -131,6 +131,7 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
static void buildWithEntryBlock(
OpBuilder &builder, OperationState &state, StringRef name, Type type,
ArrayRef<NamedAttribute> attrs, TypeRange inputTypes) {
+ OpBuilder::InsertionGuard g(builder);
state.addAttribute(SymbolTable::getSymbolAttrName(),
builder.getStringAttr(name));
state.addAttribute(ConcreteOp::getFunctionTypeAttrName(state.name),
@@ -139,8 +140,7 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
// Add the function body.
Region *bodyRegion = state.addRegion();
- Block *body = new Block();
- bodyRegion->push_back(body);
+ Block *body = builder.createBlock(bodyRegion);
for (Type input : inputTypes)
body->addArgument(input, state.location);
}
diff --git a/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp b/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
index 0ab53ce7e3327e..77603739137614 100644
--- a/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
+++ b/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
@@ -259,7 +259,7 @@ static void addResumeFunction(ModuleOp module) {
kResume, LLVM::LLVMFunctionType::get(voidTy, {ptrType}));
resumeOp.setPrivate();
- auto *block = resumeOp.addEntryBlock();
+ auto *block = resumeOp.addEntryBlock(moduleBuilder);
auto blockBuilder = ImplicitLocOpBuilder::atBlockEnd(loc, block);
blockBuilder.create<LLVM::CoroResumeOp>(resumeOp.getArgument(0));
diff --git a/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp b/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp
index 363e5f9b8cefe7..d3ee89743da9db 100644
--- a/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp
+++ b/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp
@@ -98,12 +98,10 @@ ControlFlowToSCFTransformation::createStructuredDoWhileLoopOp(
loc, builder.create<arith::TruncIOp>(loc, builder.getI1Type(), condition),
loopVariablesNextIter);
- auto *afterBlock = new Block;
- whileOp.getAfter().push_back(afterBlock);
+ Block *afterBlock = builder.createBlock(&whileOp.getAfter());
afterBlock->addArguments(
loopVariablesInit.getTypes(),
SmallVector<Location>(loopVariablesInit.size(), loc));
- builder.setInsertionPointToEnd(afterBlock);
builder.create<scf::YieldOp>(loc, afterBlock->getArguments());
return whileOp.getOperation();
diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index bd50c67fb87958..53b44aa3241bb1 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -135,7 +135,7 @@ static void wrapForExternalCallers(OpBuilder &rewriter, Location loc,
propagateArgResAttrs(rewriter, !!resultStructType, funcOp, wrapperFuncOp);
OpBuilder::InsertionGuard guard(rewriter);
- rewriter.setInsertionPointToStart(wrapperFuncOp.addEntryBlock());
+ rewriter.setInsertionPointToStart(wrapperFuncOp.addEntryBlock(rewriter));
SmallVector<Value, 8> args;
size_t argOffset = resultStructType ? 1 : 0;
@@ -203,7 +203,7 @@ static void wrapExternalFunction(OpBuilder &builder, Location loc,
// The wrapper that we synthetize here should only be visible in this module.
newFuncOp.setLinkage(LLVM::Linkage::Private);
- builder.setInsertionPointToStart(newFuncOp.addEntryBlock());
+ builder.setInsertionPointToStart(newFuncOp.addEntryBlock(builder));
// Get a ValueRange containing arguments.
FunctionType type = cast<FunctionType>(funcOp.getFunctionType());
diff --git a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
index 2bfca303b5fd48..2dc42f0a85e669 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
@@ -520,9 +520,7 @@ struct GlobalMemrefOpLowering
global, arrayTy, global.getConstant(), linkage, global.getSymName(),
initialValue, alignment, *addressSpace);
if (!global.isExternal() && global.isUninitialized()) {
- Block *blk = new Block();
- newGlobal.getInitializerRegion().push_back(blk);
- rewriter.setInsertionPointToStart(blk);
+ rewriter.createBlock(&newGlobal.getInitializerRegion());
Value undef[] = {
rewriter.create<LLVM::UndefOp>(global.getLoc(), arrayTy)};
rewriter.create<LLVM::ReturnOp>(global.getLoc(), undef);
diff --git a/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp b/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
index febfe97f6c0a99..d90cf931385fcc 100644
--- a/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
+++ b/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
@@ -138,14 +138,13 @@ struct ForOpConversion final : SCFToSPIRVPattern<scf::ForOp> {
// from header to merge.
auto loc = forOp.getLoc();
auto loopOp = rewriter.create<spirv::LoopOp>(loc, spirv::LoopControl::None);
- loopOp.addEntryAndMergeBlock();
+ loopOp.addEntryAndMergeBlock(rewriter);
OpBuilder::InsertionGuard guard(rewriter);
// Create the block for the header.
- auto *header = new Block();
- // Insert the header.
- loopOp.getBody().getBlocks().insert(getBlockIt(loopOp.getBody(), 1),
- header);
+ Block *header = rewriter.createBlock(&loopOp.getBody(),
+ getBlockIt(loopOp.getBody(), 1));
+ rewriter.setInsertionPointAfter(loopOp);
// Create the new induction variable to use.
Value adapLowerBound = adaptor.getLowerBound();
@@ -342,7 +341,7 @@ struct WhileOpConversion final : SCFToSPIRVPattern<scf::WhileOp> {
ConversionPatternRewriter &rewriter) const override {
auto loc = whileOp.getLoc();
auto loopOp = rewriter.create<spirv::LoopOp>(loc, spirv::LoopControl::None);
- loopOp.addEntryAndMergeBlock();
+ loopOp.addEntryAndMergeBlock(rewriter);
Region &beforeRegion = whileOp.getBefore();
Region &afterRegion = whileOp.getAfter();
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index c4b13193f4e773..a4df863ab08342 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1813,6 +1813,8 @@ void AffineForOp::build(OpBuilder &builder, OperationState &result,
"upper bound operand count does not match the affine map");
assert(step > 0 && "step has to be a positive integer constant");
+ OpBuilder::InsertionGuard guard(builder);
+
// Set variadic segment sizes.
result.addAttribute(
getOperandSegmentSizeAttr(),
@@ -1841,12 +1843,11 @@ void AffineForOp::build(OpBuilder &builder, OperationState &result,
// Create a region and a block for the body. The argument of the region is
// the loop induction variable.
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block);
- Block &bodyBlock = bodyRegion->front();
+ Block *bodyBlock = builder.createBlock(bodyRegion);
Value inductionVar =
- bodyBlock.addArgument(builder.getIndexType(), result.location);
+ bodyBlock->addArgument(builder.getIndexType(), result.location);
for (Value val : iterArgs)
- bodyBlock.addArgument(val.getType(), val.getLoc());
+ bodyBlock->addArgument(val.getType(), val.getLoc());
// Create the default terminator if the builder is not provided and if the
// iteration arguments are not provided. Otherwise, leave this to the caller
@@ -1855,9 +1856,9 @@ void AffineForOp::build(OpBuilder &builder, OperationState &result,
ensureTerminator(*bodyRegion, builder, result.location);
} else if (bodyBuilder) {
OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
+ builder.setInsertionPointToStart(bodyBlock);
bodyBuilder(builder, result.location, inductionVar,
- bodyBlock.getArguments().drop_front());
+ bodyBlock->getArguments().drop_front());
}
}
@@ -2895,18 +2896,20 @@ void AffineIfOp::build(OpBuilder &builder, OperationState &result,
TypeRange resultTypes, IntegerSet set, ValueRange args,
bool withElseRegion) {
assert(resultTypes.empty() || withElseRegion);
+ OpBuilder::InsertionGuard guard(builder);
+
result.addTypes(resultTypes);
result.addOperands(args);
result.addAttribute(getConditionAttrStrName(), IntegerSetAttr::get(set));
Region *thenRegion = result.addRegion();
- thenRegion->push_back(new Block());
+ builder.createBlock(thenRegion);
if (resultTypes.empty())
AffineIfOp::ensureTerminator(*thenRegion, builder, result.location);
Region *elseRegion = result.addRegion();
if (withElseRegion) {
- elseRegion->push_back(new Block());
+ builder.createBlock(elseRegion);
if (resultTypes.empty())
AffineIfOp::ensureTerminator(*elseRegion, builder, result.location);
}
@@ -3693,6 +3696,7 @@ void AffineParallelOp::build(OpBuilder &builder, OperationState &result,
"expected upper bound maps to have as many inputs as upper bound "
"operands");
+ OpBuilder::InsertionGuard guard(builder);
result.addTypes(resultTypes);
// Convert the reductions to integer attributes.
@@ -3738,11 +3742,11 @@ void AffineParallelOp::build(OpBuilder &builder, OperationState &result,
// Create a region and a block for the body.
auto *bodyRegion = result.addRegion();
- auto *body = new Block();
+ Block *body = builder.createBlock(bodyRegion);
+
// Add all the block arguments.
for (unsigned i = 0, e = steps.size(); i < e; ++i)
body->addArgument(IndexType::get(builder.getContext()), result.location);
- bodyRegion->push_back(body);
if (resultTypes.empty())
ensureTerminator(*bodyRegion, builder, result.location);
}
diff --git a/mlir/lib/Dialect/Async/IR/Async.cpp b/mlir/lib/Dialect/Async/IR/Async.cpp
index 5f583f36cd2cb8..a3e3f80954efce 100644
--- a/mlir/lib/Dialect/Async/IR/Async.cpp
+++ b/mlir/lib/Dialect/Async/IR/Async.cpp
@@ -68,7 +68,7 @@ void ExecuteOp::getSuccessorRegions(RegionBranchPoint point,
void ExecuteOp::build(OpBuilder &builder, OperationState &result,
TypeRange resultTypes, ValueRange dependencies,
ValueRange operands, BodyBuilderFn bodyBuilder) {
-
+ OpBuilder::InsertionGuard guard(builder);
result.addOperands(dependencies);
result.addOperands(operands);
@@ -87,26 +87,21 @@ void ExecuteOp::build(OpBuilder &builder, OperationState &result,
// Add a body region with block arguments as unwrapped async value operands.
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block);
- Block &bodyBlock = bodyRegion->front();
+ Block *bodyBlock = builder.createBlock(bodyRegion);
for (Value operand : operands) {
auto valueType = llvm::dyn_cast<ValueType>(operand.getType());
- bodyBlock.addArgument(valueType ? valueType.getValueType()
- : operand.getType(),
- operand.getLoc());
+ bodyBlock->addArgument(valueType ? valueType.getValueType()
+ : operand.getType(),
+ operand.getLoc());
}
// Create the default terminator if the builder is not provided and if the
// expected result is empty. Otherwise, leave this to the caller
// because we don't know which values to return from the execute op.
if (resultTypes.empty() && !bodyBuilder) {
- OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
builder.create<async::YieldOp>(result.location, ValueRange());
} else if (bodyBuilder) {
- OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
- bodyBuilder(builder, result.location, bodyBlock.getArguments());
+ bodyBuilder(builder, result.location, bodyBlock->getArguments());
}
}
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index 0fe2c0dcfc7c53..4df8149b94c95f 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -261,20 +261,20 @@ LogicalResult ExpressionOp::verify() {
void ForOp::build(OpBuilder &builder, OperationState &result, Value lb,
Value ub, Value step, BodyBuilderFn bodyBuilder) {
+ OpBuilder::InsertionGuard g(builder);
result.addOperands({lb, ub, step});
Type t = lb.getType();
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block);
- Block &bodyBlock = bodyRegion->front();
- bodyBlock.addArgument(t, result.location);
+ Block *bodyBlock = builder.createBlock(bodyRegion);
+ bodyBlock->addArgument(t, result.location);
// Create the default terminator if the builder is not provided.
if (!bodyBuilder) {
ForOp::ensureTerminator(*bodyRegion, builder, result.location);
} else {
OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
- bodyBuilder(builder, result.location, bodyBlock.getArgument(0));
+ builder.setInsertionPointToStart(bodyBlock);
+ bodyBuilder(builder, result.location, bodyBlock->getArgument(0));
}
}
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index f4042a60541a6a..3ba6ac6ccc8142 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2165,11 +2165,10 @@ LogicalResult ShuffleVectorOp::verify() {
//===----------------------------------------------------------------------===//
// Add the entry block to the function.
-Block *LLVMFuncOp::addEntryBlock() {
+Block *LLVMFuncOp::addEntryBlock(OpBuilder &builder) {
assert(empty() && "function already has an entry block");
-
- auto *entry = new Block;
- push_back(entry);
+ OpBuilder::InsertionGuard g(builder);
+ Block *entry = builder.createBlock(&getBody());
// FIXME: Allow passing in proper locations for the entry arguments.
LLVMFunctionType type = getFunctionType();
diff --git a/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp b/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
index 559ffda4494d2b..c46e3694b70ecd 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
@@ -132,12 +132,10 @@ struct MoveInitOperandsToInput : public OpRewritePattern<GenericOp> {
newIndexingMaps, genericOp.getIteratorTypesArray(),
/*bodyBuild=*/nullptr, linalg::getPrunedAttributeList(genericOp));
+ OpBuilder::InsertionGuard guard(rewriter);
Region ®ion = newOp.getRegion();
- Block *block = new Block();
- region.push_back(block);
+ Block *block = rewriter.createBlock(®ion);
IRMapping mapper;
- OpBuilder::InsertionGuard guard(rewriter);
- rewriter.setInsertionPointToStart(block);
for (auto bbarg : genericOp.getRegionInputArgs())
mapper.map(bbarg, block->addArgument(bbarg.getType(), loc));
diff --git a/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp b/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp
index 286b07669a47f5..0d8d670904f2a8 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp
@@ -178,11 +178,9 @@ static void generateFusedElementwiseOpRegion(
// Build the region of the fused op.
Block &producerBlock = producer->getRegion(0).front();
Block &consumerBlock = consumer->getRegion(0).front();
- Block *fusedBlock = new Block();
- fusedOp.getRegion().push_back(fusedBlock);
- IRMapping mapper;
OpBuilder::InsertionGuard guard(rewriter);
- rewriter.setInsertionPointToStart(fusedBlock);
+ Block *fusedBlock = rewriter.createBlock(&fusedOp.getRegion());
+ IRMapping mapper;
// 2. Add an index operation for every fused loop dimension and use the
// `consumerToProducerLoopsMap` to map the producer indices.
diff --git a/mlir/lib/Dialect/Linalg/Utils/Utils.cpp b/mlir/lib/Dialect/Linalg/Utils/Utils.cpp
index 5d220c6cdd7e58..6392c83bde24f5 100644
--- a/mlir/lib/Dialect/Linalg/Utils/Utils.cpp
+++ b/mlir/lib/Dialect/Linalg/Utils/Utils.cpp
@@ -275,14 +275,13 @@ GenericOp makeTransposeOp(OpBuilder &b, Location loc, Value inputTensor,
auto transposeOp =
b.create<GenericOp>(loc, resultTensorType, inputTensor, outputTensor,
indexingMaps, iteratorTypes);
- Region &body = transposeOp.getRegion();
- body.push_back(new Block());
- body.front().addArguments({elementType, elementType}, {loc, loc});
// Create the body of the transpose operation.
OpBuilder::InsertionGuard g(b);
- b.setInsertionPointToEnd(&body.front());
- b.create<YieldOp>(loc, transposeOp.getRegion().front().getArgument(0));
+ Region &body = transposeOp.getRegion();
+ Block *bodyBlock = b.createBlock(&body);
+ bodyBlock->addArguments({elementType, elementType}, {loc, loc});
+ b.create<YieldOp>(loc, bodyBlock->getArgument(0));
return transposeOp;
}
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index beb7e721ca53b8..248193481acfc6 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -1420,6 +1420,7 @@ OpFoldResult ExtractStridedMetadataOp::getConstifiedMixedOffset() {
void GenericAtomicRMWOp::build(OpBuilder &builder, OperationState &result,
Value memref, ValueRange ivs) {
+ OpBuilder::InsertionGuard g(builder);
result.addOperands(memref);
result.addOperands(ivs);
@@ -1428,7 +1429,7 @@ void GenericAtomicRMWOp::build(OpBuilder &builder, OperationState &result,
result.addTypes(elementType);
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block());
+ builder.createBlock(bodyRegion);
bodyRegion->addArgument(elementType, memref.getLoc());
}
}
diff --git a/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp b/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
index 580782043c81b4..7170a899069ee3 100644
--- a/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
@@ -365,12 +365,11 @@ Block *LoopOp::getMergeBlock() {
return &getBody().back();
}
-void LoopOp::addEntryAndMergeBlock() {
+void LoopOp::addEntryAndMergeBlock(OpBuilder &builder) {
assert(getBody().empty() && "entry and merge block already exist");
- getBody().p...
[truncated]
|
|
@llvm/pr-subscribers-mlir-memref Author: Matthias Springer (matthias-springer) ChangesWhen creating a new block in (conversion) rewrite patterns, Note: The dialect conversion relies on listener notifications to keep track of IR modifications. Creating blocks without the builder API can lead to memory leaks during rollback. Patch is 25.57 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82770.diff 20 Files Affected:
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 3da5deeb4ec7e2..b523374f6c06b5 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1456,7 +1456,7 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
let extraClassDeclaration = [{
// Add an entry block to an empty function, and set up the block arguments
// to match the signature of the function.
- Block *addEntryBlock();
+ Block *addEntryBlock(OpBuilder &builder);
bool isVarArg() { return getFunctionType().isVarArg(); }
diff --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
index 36ad6755cab25e..991e753d1b3593 100644
--- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
+++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
@@ -285,7 +285,7 @@ def SPIRV_LoopOp : SPIRV_Op<"mlir.loop", [InFunctionScope]> {
// Adds an empty entry block and loop merge block containing one
// spirv.mlir.merge op.
- void addEntryAndMergeBlock();
+ void addEntryAndMergeBlock(OpBuilder &builder);
}];
let hasOpcode = 0;
@@ -427,7 +427,7 @@ def SPIRV_SelectionOp : SPIRV_Op<"mlir.selection", [InFunctionScope]> {
Block *getMergeBlock();
/// Adds a selection merge block containing one spirv.mlir.merge op.
- void addMergeBlock();
+ void addMergeBlock(OpBuilder &builder);
/// Creates a spirv.mlir.selection op for `if (<condition>) then { <thenBody> }`
/// with `builder`. `builder`'s insertion point will remain at after the
diff --git a/mlir/include/mlir/Interfaces/FunctionInterfaces.td b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
index 970a781c998b98..873853eba0b175 100644
--- a/mlir/include/mlir/Interfaces/FunctionInterfaces.td
+++ b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
@@ -131,6 +131,7 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
static void buildWithEntryBlock(
OpBuilder &builder, OperationState &state, StringRef name, Type type,
ArrayRef<NamedAttribute> attrs, TypeRange inputTypes) {
+ OpBuilder::InsertionGuard g(builder);
state.addAttribute(SymbolTable::getSymbolAttrName(),
builder.getStringAttr(name));
state.addAttribute(ConcreteOp::getFunctionTypeAttrName(state.name),
@@ -139,8 +140,7 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
// Add the function body.
Region *bodyRegion = state.addRegion();
- Block *body = new Block();
- bodyRegion->push_back(body);
+ Block *body = builder.createBlock(bodyRegion);
for (Type input : inputTypes)
body->addArgument(input, state.location);
}
diff --git a/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp b/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
index 0ab53ce7e3327e..77603739137614 100644
--- a/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
+++ b/mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
@@ -259,7 +259,7 @@ static void addResumeFunction(ModuleOp module) {
kResume, LLVM::LLVMFunctionType::get(voidTy, {ptrType}));
resumeOp.setPrivate();
- auto *block = resumeOp.addEntryBlock();
+ auto *block = resumeOp.addEntryBlock(moduleBuilder);
auto blockBuilder = ImplicitLocOpBuilder::atBlockEnd(loc, block);
blockBuilder.create<LLVM::CoroResumeOp>(resumeOp.getArgument(0));
diff --git a/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp b/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp
index 363e5f9b8cefe7..d3ee89743da9db 100644
--- a/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp
+++ b/mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp
@@ -98,12 +98,10 @@ ControlFlowToSCFTransformation::createStructuredDoWhileLoopOp(
loc, builder.create<arith::TruncIOp>(loc, builder.getI1Type(), condition),
loopVariablesNextIter);
- auto *afterBlock = new Block;
- whileOp.getAfter().push_back(afterBlock);
+ Block *afterBlock = builder.createBlock(&whileOp.getAfter());
afterBlock->addArguments(
loopVariablesInit.getTypes(),
SmallVector<Location>(loopVariablesInit.size(), loc));
- builder.setInsertionPointToEnd(afterBlock);
builder.create<scf::YieldOp>(loc, afterBlock->getArguments());
return whileOp.getOperation();
diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index bd50c67fb87958..53b44aa3241bb1 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -135,7 +135,7 @@ static void wrapForExternalCallers(OpBuilder &rewriter, Location loc,
propagateArgResAttrs(rewriter, !!resultStructType, funcOp, wrapperFuncOp);
OpBuilder::InsertionGuard guard(rewriter);
- rewriter.setInsertionPointToStart(wrapperFuncOp.addEntryBlock());
+ rewriter.setInsertionPointToStart(wrapperFuncOp.addEntryBlock(rewriter));
SmallVector<Value, 8> args;
size_t argOffset = resultStructType ? 1 : 0;
@@ -203,7 +203,7 @@ static void wrapExternalFunction(OpBuilder &builder, Location loc,
// The wrapper that we synthetize here should only be visible in this module.
newFuncOp.setLinkage(LLVM::Linkage::Private);
- builder.setInsertionPointToStart(newFuncOp.addEntryBlock());
+ builder.setInsertionPointToStart(newFuncOp.addEntryBlock(builder));
// Get a ValueRange containing arguments.
FunctionType type = cast<FunctionType>(funcOp.getFunctionType());
diff --git a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
index 2bfca303b5fd48..2dc42f0a85e669 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
@@ -520,9 +520,7 @@ struct GlobalMemrefOpLowering
global, arrayTy, global.getConstant(), linkage, global.getSymName(),
initialValue, alignment, *addressSpace);
if (!global.isExternal() && global.isUninitialized()) {
- Block *blk = new Block();
- newGlobal.getInitializerRegion().push_back(blk);
- rewriter.setInsertionPointToStart(blk);
+ rewriter.createBlock(&newGlobal.getInitializerRegion());
Value undef[] = {
rewriter.create<LLVM::UndefOp>(global.getLoc(), arrayTy)};
rewriter.create<LLVM::ReturnOp>(global.getLoc(), undef);
diff --git a/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp b/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
index febfe97f6c0a99..d90cf931385fcc 100644
--- a/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
+++ b/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
@@ -138,14 +138,13 @@ struct ForOpConversion final : SCFToSPIRVPattern<scf::ForOp> {
// from header to merge.
auto loc = forOp.getLoc();
auto loopOp = rewriter.create<spirv::LoopOp>(loc, spirv::LoopControl::None);
- loopOp.addEntryAndMergeBlock();
+ loopOp.addEntryAndMergeBlock(rewriter);
OpBuilder::InsertionGuard guard(rewriter);
// Create the block for the header.
- auto *header = new Block();
- // Insert the header.
- loopOp.getBody().getBlocks().insert(getBlockIt(loopOp.getBody(), 1),
- header);
+ Block *header = rewriter.createBlock(&loopOp.getBody(),
+ getBlockIt(loopOp.getBody(), 1));
+ rewriter.setInsertionPointAfter(loopOp);
// Create the new induction variable to use.
Value adapLowerBound = adaptor.getLowerBound();
@@ -342,7 +341,7 @@ struct WhileOpConversion final : SCFToSPIRVPattern<scf::WhileOp> {
ConversionPatternRewriter &rewriter) const override {
auto loc = whileOp.getLoc();
auto loopOp = rewriter.create<spirv::LoopOp>(loc, spirv::LoopControl::None);
- loopOp.addEntryAndMergeBlock();
+ loopOp.addEntryAndMergeBlock(rewriter);
Region &beforeRegion = whileOp.getBefore();
Region &afterRegion = whileOp.getAfter();
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index c4b13193f4e773..a4df863ab08342 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1813,6 +1813,8 @@ void AffineForOp::build(OpBuilder &builder, OperationState &result,
"upper bound operand count does not match the affine map");
assert(step > 0 && "step has to be a positive integer constant");
+ OpBuilder::InsertionGuard guard(builder);
+
// Set variadic segment sizes.
result.addAttribute(
getOperandSegmentSizeAttr(),
@@ -1841,12 +1843,11 @@ void AffineForOp::build(OpBuilder &builder, OperationState &result,
// Create a region and a block for the body. The argument of the region is
// the loop induction variable.
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block);
- Block &bodyBlock = bodyRegion->front();
+ Block *bodyBlock = builder.createBlock(bodyRegion);
Value inductionVar =
- bodyBlock.addArgument(builder.getIndexType(), result.location);
+ bodyBlock->addArgument(builder.getIndexType(), result.location);
for (Value val : iterArgs)
- bodyBlock.addArgument(val.getType(), val.getLoc());
+ bodyBlock->addArgument(val.getType(), val.getLoc());
// Create the default terminator if the builder is not provided and if the
// iteration arguments are not provided. Otherwise, leave this to the caller
@@ -1855,9 +1856,9 @@ void AffineForOp::build(OpBuilder &builder, OperationState &result,
ensureTerminator(*bodyRegion, builder, result.location);
} else if (bodyBuilder) {
OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
+ builder.setInsertionPointToStart(bodyBlock);
bodyBuilder(builder, result.location, inductionVar,
- bodyBlock.getArguments().drop_front());
+ bodyBlock->getArguments().drop_front());
}
}
@@ -2895,18 +2896,20 @@ void AffineIfOp::build(OpBuilder &builder, OperationState &result,
TypeRange resultTypes, IntegerSet set, ValueRange args,
bool withElseRegion) {
assert(resultTypes.empty() || withElseRegion);
+ OpBuilder::InsertionGuard guard(builder);
+
result.addTypes(resultTypes);
result.addOperands(args);
result.addAttribute(getConditionAttrStrName(), IntegerSetAttr::get(set));
Region *thenRegion = result.addRegion();
- thenRegion->push_back(new Block());
+ builder.createBlock(thenRegion);
if (resultTypes.empty())
AffineIfOp::ensureTerminator(*thenRegion, builder, result.location);
Region *elseRegion = result.addRegion();
if (withElseRegion) {
- elseRegion->push_back(new Block());
+ builder.createBlock(elseRegion);
if (resultTypes.empty())
AffineIfOp::ensureTerminator(*elseRegion, builder, result.location);
}
@@ -3693,6 +3696,7 @@ void AffineParallelOp::build(OpBuilder &builder, OperationState &result,
"expected upper bound maps to have as many inputs as upper bound "
"operands");
+ OpBuilder::InsertionGuard guard(builder);
result.addTypes(resultTypes);
// Convert the reductions to integer attributes.
@@ -3738,11 +3742,11 @@ void AffineParallelOp::build(OpBuilder &builder, OperationState &result,
// Create a region and a block for the body.
auto *bodyRegion = result.addRegion();
- auto *body = new Block();
+ Block *body = builder.createBlock(bodyRegion);
+
// Add all the block arguments.
for (unsigned i = 0, e = steps.size(); i < e; ++i)
body->addArgument(IndexType::get(builder.getContext()), result.location);
- bodyRegion->push_back(body);
if (resultTypes.empty())
ensureTerminator(*bodyRegion, builder, result.location);
}
diff --git a/mlir/lib/Dialect/Async/IR/Async.cpp b/mlir/lib/Dialect/Async/IR/Async.cpp
index 5f583f36cd2cb8..a3e3f80954efce 100644
--- a/mlir/lib/Dialect/Async/IR/Async.cpp
+++ b/mlir/lib/Dialect/Async/IR/Async.cpp
@@ -68,7 +68,7 @@ void ExecuteOp::getSuccessorRegions(RegionBranchPoint point,
void ExecuteOp::build(OpBuilder &builder, OperationState &result,
TypeRange resultTypes, ValueRange dependencies,
ValueRange operands, BodyBuilderFn bodyBuilder) {
-
+ OpBuilder::InsertionGuard guard(builder);
result.addOperands(dependencies);
result.addOperands(operands);
@@ -87,26 +87,21 @@ void ExecuteOp::build(OpBuilder &builder, OperationState &result,
// Add a body region with block arguments as unwrapped async value operands.
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block);
- Block &bodyBlock = bodyRegion->front();
+ Block *bodyBlock = builder.createBlock(bodyRegion);
for (Value operand : operands) {
auto valueType = llvm::dyn_cast<ValueType>(operand.getType());
- bodyBlock.addArgument(valueType ? valueType.getValueType()
- : operand.getType(),
- operand.getLoc());
+ bodyBlock->addArgument(valueType ? valueType.getValueType()
+ : operand.getType(),
+ operand.getLoc());
}
// Create the default terminator if the builder is not provided and if the
// expected result is empty. Otherwise, leave this to the caller
// because we don't know which values to return from the execute op.
if (resultTypes.empty() && !bodyBuilder) {
- OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
builder.create<async::YieldOp>(result.location, ValueRange());
} else if (bodyBuilder) {
- OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
- bodyBuilder(builder, result.location, bodyBlock.getArguments());
+ bodyBuilder(builder, result.location, bodyBlock->getArguments());
}
}
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index 0fe2c0dcfc7c53..4df8149b94c95f 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -261,20 +261,20 @@ LogicalResult ExpressionOp::verify() {
void ForOp::build(OpBuilder &builder, OperationState &result, Value lb,
Value ub, Value step, BodyBuilderFn bodyBuilder) {
+ OpBuilder::InsertionGuard g(builder);
result.addOperands({lb, ub, step});
Type t = lb.getType();
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block);
- Block &bodyBlock = bodyRegion->front();
- bodyBlock.addArgument(t, result.location);
+ Block *bodyBlock = builder.createBlock(bodyRegion);
+ bodyBlock->addArgument(t, result.location);
// Create the default terminator if the builder is not provided.
if (!bodyBuilder) {
ForOp::ensureTerminator(*bodyRegion, builder, result.location);
} else {
OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPointToStart(&bodyBlock);
- bodyBuilder(builder, result.location, bodyBlock.getArgument(0));
+ builder.setInsertionPointToStart(bodyBlock);
+ bodyBuilder(builder, result.location, bodyBlock->getArgument(0));
}
}
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index f4042a60541a6a..3ba6ac6ccc8142 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2165,11 +2165,10 @@ LogicalResult ShuffleVectorOp::verify() {
//===----------------------------------------------------------------------===//
// Add the entry block to the function.
-Block *LLVMFuncOp::addEntryBlock() {
+Block *LLVMFuncOp::addEntryBlock(OpBuilder &builder) {
assert(empty() && "function already has an entry block");
-
- auto *entry = new Block;
- push_back(entry);
+ OpBuilder::InsertionGuard g(builder);
+ Block *entry = builder.createBlock(&getBody());
// FIXME: Allow passing in proper locations for the entry arguments.
LLVMFunctionType type = getFunctionType();
diff --git a/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp b/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
index 559ffda4494d2b..c46e3694b70ecd 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
@@ -132,12 +132,10 @@ struct MoveInitOperandsToInput : public OpRewritePattern<GenericOp> {
newIndexingMaps, genericOp.getIteratorTypesArray(),
/*bodyBuild=*/nullptr, linalg::getPrunedAttributeList(genericOp));
+ OpBuilder::InsertionGuard guard(rewriter);
Region ®ion = newOp.getRegion();
- Block *block = new Block();
- region.push_back(block);
+ Block *block = rewriter.createBlock(®ion);
IRMapping mapper;
- OpBuilder::InsertionGuard guard(rewriter);
- rewriter.setInsertionPointToStart(block);
for (auto bbarg : genericOp.getRegionInputArgs())
mapper.map(bbarg, block->addArgument(bbarg.getType(), loc));
diff --git a/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp b/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp
index 286b07669a47f5..0d8d670904f2a8 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp
@@ -178,11 +178,9 @@ static void generateFusedElementwiseOpRegion(
// Build the region of the fused op.
Block &producerBlock = producer->getRegion(0).front();
Block &consumerBlock = consumer->getRegion(0).front();
- Block *fusedBlock = new Block();
- fusedOp.getRegion().push_back(fusedBlock);
- IRMapping mapper;
OpBuilder::InsertionGuard guard(rewriter);
- rewriter.setInsertionPointToStart(fusedBlock);
+ Block *fusedBlock = rewriter.createBlock(&fusedOp.getRegion());
+ IRMapping mapper;
// 2. Add an index operation for every fused loop dimension and use the
// `consumerToProducerLoopsMap` to map the producer indices.
diff --git a/mlir/lib/Dialect/Linalg/Utils/Utils.cpp b/mlir/lib/Dialect/Linalg/Utils/Utils.cpp
index 5d220c6cdd7e58..6392c83bde24f5 100644
--- a/mlir/lib/Dialect/Linalg/Utils/Utils.cpp
+++ b/mlir/lib/Dialect/Linalg/Utils/Utils.cpp
@@ -275,14 +275,13 @@ GenericOp makeTransposeOp(OpBuilder &b, Location loc, Value inputTensor,
auto transposeOp =
b.create<GenericOp>(loc, resultTensorType, inputTensor, outputTensor,
indexingMaps, iteratorTypes);
- Region &body = transposeOp.getRegion();
- body.push_back(new Block());
- body.front().addArguments({elementType, elementType}, {loc, loc});
// Create the body of the transpose operation.
OpBuilder::InsertionGuard g(b);
- b.setInsertionPointToEnd(&body.front());
- b.create<YieldOp>(loc, transposeOp.getRegion().front().getArgument(0));
+ Region &body = transposeOp.getRegion();
+ Block *bodyBlock = b.createBlock(&body);
+ bodyBlock->addArguments({elementType, elementType}, {loc, loc});
+ b.create<YieldOp>(loc, bodyBlock->getArgument(0));
return transposeOp;
}
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index beb7e721ca53b8..248193481acfc6 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -1420,6 +1420,7 @@ OpFoldResult ExtractStridedMetadataOp::getConstifiedMixedOffset() {
void GenericAtomicRMWOp::build(OpBuilder &builder, OperationState &result,
Value memref, ValueRange ivs) {
+ OpBuilder::InsertionGuard g(builder);
result.addOperands(memref);
result.addOperands(ivs);
@@ -1428,7 +1429,7 @@ void GenericAtomicRMWOp::build(OpBuilder &builder, OperationState &result,
result.addTypes(elementType);
Region *bodyRegion = result.addRegion();
- bodyRegion->push_back(new Block());
+ builder.createBlock(bodyRegion);
bodyRegion->addArgument(elementType, memref.getLoc());
}
}
diff --git a/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp b/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
index 580782043c81b4..7170a899069ee3 100644
--- a/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
@@ -365,12 +365,11 @@ Block *LoopOp::getMergeBlock() {
return &getBody().back();
}
-void LoopOp::addEntryAndMergeBlock() {
+void LoopOp::addEntryAndMergeBlock(OpBuilder &builder) {
assert(getBody().empty() && "entry and merge block already exist");
- getBody().p...
[truncated]
|
ftynse
approved these changes
Feb 23, 2024
| @@ -299,8 +299,7 @@ struct FuseSparseMultiplyOverAdd : public OpRewritePattern<GenericOp> { | |||
| Block &prodBlock = prod.getRegion().front(); | |||
| Block &consBlock = op.getRegion().front(); | |||
| IRMapping mapper; | |||
| Block *fusedBlock = new Block(); | |||
| fusedOp.getRegion().push_back(fusedBlock); | |||
| Block *fusedBlock = rewriter.createBlock(&fusedOp.getRegion()); | |||
| unsigned num = prodBlock.getNumArguments(); | |||
| for (unsigned i = 0; i < num - 1; i++) | |||
| addArg(mapper, fusedBlock, prodBlock.getArgument(i)); | |||
There was a problem hiding this comment.
I suspect this may be also unhappy if the underlying implementation adds arguments to fusedBlock.
There was a problem hiding this comment.
We have no notifyBlockModified callback at the moment, so I think there is no expectation that a listener be notified about adding/removing/changing block arguments. I have plans to add RewriterBase::Listener::notifyBlockModified and generalize modifyOpInPlace to modifyInPlace (accepting both ops and blocks).
When creating a new block in (conversion) rewrite patterns, `OpBuilder::createBlock` must be used. Otherwise, no `notifyBlockInserted` notification is sent to the listener. Note: The dialect conversion relies on listener notifications to keep track of IR modifications. Creating blocks without the builder API can lead to memory leaks during rollback.
matthias-springer
force-pushed
the
users/matthias-springer/create_block_api
branch
from
dffe3a9 to
ac01c30
Compare
joker-eph
reviewed
Feb 24, 2024
| @@ -520,9 +520,7 @@ struct GlobalMemrefOpLowering | |||
| global, arrayTy, global.getConstant(), linkage, global.getSymName(), | |||
| initialValue, alignment, *addressSpace); | |||
| if (!global.isExternal() && global.isUninitialized()) { | |||
| Block *blk = new Block(); | |||
There was a problem hiding this comment.
Oh these raw uses of new always irked me deeply! Glad to see more that are gone
joker-eph
approved these changes
Feb 24, 2024
matthias-springer
deleted the
users/matthias-springer/create_block_api
branch
mgehre-amd
pushed a commit
to Xilinx/llvm-project
that referenced
this pull request
Mar 11, 2024
…#82770) When creating a new block in (conversion) rewrite patterns, `OpBuilder::createBlock` must be used. Otherwise, no `notifyBlockInserted` notification is sent to the listener. Note: The dialect conversion relies on listener notifications to keep track of IR modifications. Creating blocks without the builder API can lead to memory leaks during rollback.
mgehre-amd
added a commit
to Xilinx/llvm-project
that referenced
this pull request
Mar 14, 2024
…ns (llvm#82770)" This reverts commit 65d0767.
mgehre-amd
added a commit
to Xilinx/llvm-project
that referenced
this pull request
Mar 14, 2024
Revert "[mlir] Use `OpBuilder::createBlock` in op builders and patterns (llvm#82770)"
mgehre-amd
added a commit
to Xilinx/llvm-project
that referenced
this pull request
Mar 20, 2024
…rns (llvm#82770)" This reverts commit a3c8ffb.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When creating a new block in (conversion) rewrite patterns,
OpBuilder::createBlockmust be used. Otherwise, nonotifyBlockInsertednotification is sent to the listener.Note: The dialect conversion relies on listener notifications to keep track of IR modifications. Creating blocks without the builder API can lead to memory leaks during rollback.