Skip to content

[mlir] Use OpBuilder::createBlock in op builders and patterns #82770

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }

Expand Down
4 changes: 2 additions & 2 deletions mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions mlir/include/mlir/Interfaces/FunctionInterfaces.td
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
4 changes: 1 addition & 3 deletions mlir/lib/Conversion/ControlFlowToSCF/ControlFlowToSCF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
4 changes: 1 addition & 3 deletions mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh these raw uses of new always irked me deeply! Glad to see more that are gone

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);
Expand Down
11 changes: 5 additions & 6 deletions mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
24 changes: 14 additions & 10 deletions mlir/lib/Dialect/Affine/IR/AffineOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand All @@ -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());
}
}

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
Expand Down
17 changes: 6 additions & 11 deletions mlir/lib/Dialect/Async/IR/Async.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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());
}
}

Expand Down
10 changes: 5 additions & 5 deletions mlir/lib/Dialect/EmitC/IR/EmitC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down
7 changes: 3 additions & 4 deletions mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 2 additions & 4 deletions mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,10 @@ struct MoveInitOperandsToInput : public OpRewritePattern<GenericOp> {
newIndexingMaps, genericOp.getIteratorTypesArray(),
/*bodyBuild=*/nullptr, linalg::getPrunedAttributeList(genericOp));

OpBuilder::InsertionGuard guard(rewriter);
Region &region = newOp.getRegion();
Block *block = new Block();
region.push_back(block);
Block *block = rewriter.createBlock(&region);
IRMapping mapper;
OpBuilder::InsertionGuard guard(rewriter);
rewriter.setInsertionPointToStart(block);
for (auto bbarg : genericOp.getRegionInputArgs())
mapper.map(bbarg, block->addArgument(bbarg.getType(), loc));

Expand Down
6 changes: 2 additions & 4 deletions mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 4 additions & 5 deletions mlir/lib/Dialect/Linalg/Utils/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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, /*insertPt=*/{},
{elementType, elementType}, {loc, loc});
b.create<YieldOp>(loc, bodyBlock->getArgument(0));
return transposeOp;
}

Expand Down
3 changes: 2 additions & 1 deletion mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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());
}
}
Expand Down
18 changes: 8 additions & 10 deletions mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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().push_back(new Block());
auto *mergeBlock = new Block();
getBody().push_back(mergeBlock);
OpBuilder builder = OpBuilder::atBlockEnd(mergeBlock);
OpBuilder::InsertionGuard g(builder);
builder.createBlock(&getBody());
builder.createBlock(&getBody());

// Add a spirv.mlir.merge op into the merge block.
builder.create<spirv::MergeOp>(getLoc());
Expand Down Expand Up @@ -525,11 +524,10 @@ Block *SelectionOp::getMergeBlock() {
return &getBody().back();
}

void SelectionOp::addMergeBlock() {
void SelectionOp::addMergeBlock(OpBuilder &builder) {
assert(getBody().empty() && "entry and merge block already exist");
auto *mergeBlock = new Block();
getBody().push_back(mergeBlock);
OpBuilder builder = OpBuilder::atBlockEnd(mergeBlock);
OpBuilder::InsertionGuard guard(builder);
builder.createBlock(&getBody());

// Add a spirv.mlir.merge op into the merge block.
builder.create<spirv::MergeOp>(getLoc());
Expand All @@ -542,7 +540,7 @@ SelectionOp::createIfThen(Location loc, Value condition,
auto selectionOp =
builder.create<spirv::SelectionOp>(loc, spirv::SelectionControl::None);

selectionOp.addMergeBlock();
selectionOp.addMergeBlock(builder);
Block *mergeBlock = selectionOp.getMergeBlock();
Block *thenBlock = nullptr;

Expand Down
Loading