diff options
author | Matthias Springer <me@m-sp.org> | 2024-02-23 14:28:57 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-02-23 14:28:57 +0100 |
commit | 492e8ba0384b038596e6b4a97313b7bdced5e868 (patch) | |
tree | f1ad90fb45f6428164762f1b80d1db66fcce0c2f | |
parent | ad49fe3e89c3b3950956548f14cdb5c159ba0aec (diff) |
[mlir] Fix memory leaks after #81759 (#82762)
This commit fixes memory leaks that were introduced by #81759. The way
ops and blocks are erased changed slightly.
The leaks were caused by an incorrect implementation of op builders:
blocks must be created with the supplied builder object. Otherwise, they
are not properly tracked by the dialect conversion and can leak during
rollback.
-rw-r--r-- | mlir/lib/Dialect/GPU/IR/GPUDialect.cpp | 11 | ||||
-rw-r--r-- | mlir/lib/Dialect/SCF/IR/SCF.cpp | 15 |
2 files changed, 14 insertions, 12 deletions
diff --git a/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp b/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp index 30b6cd74147e..33ce5c159db4 100644 --- a/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp +++ b/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp @@ -648,6 +648,8 @@ void LaunchOp::build(OpBuilder &builder, OperationState &result, TypeRange workgroupAttributions, TypeRange privateAttributions, Value clusterSizeX, Value clusterSizeY, Value clusterSizeZ) { + OpBuilder::InsertionGuard g(builder); + // Add a WorkGroup attribution attribute. This attribute is required to // identify private attributions in the list of block argguments. result.addAttribute(getNumWorkgroupAttributionsAttrName(), @@ -674,7 +676,7 @@ void LaunchOp::build(OpBuilder &builder, OperationState &result, // attributions, where the first kNumConfigRegionAttributes arguments have // `index` type and the rest have the same types as the data operands. Region *kernelRegion = result.addRegion(); - Block *body = new Block(); + Block *body = builder.createBlock(kernelRegion); // TODO: Allow passing in proper locations here. for (unsigned i = 0; i < kNumConfigRegionAttributes; ++i) body->addArgument(builder.getIndexType(), result.location); @@ -683,7 +685,6 @@ void LaunchOp::build(OpBuilder &builder, OperationState &result, body->addArgument(argTy, result.location); for (Type argTy : privateAttributions) body->addArgument(argTy, result.location); - kernelRegion->push_back(body); // Fill OperandSegmentSize Attribute. SmallVector<int32_t, 11> segmentSizes(11, 1); segmentSizes.front() = asyncDependencies.size(); @@ -1325,6 +1326,8 @@ void GPUFuncOp::build(OpBuilder &builder, OperationState &result, TypeRange workgroupAttributions, TypeRange privateAttributions, ArrayRef<NamedAttribute> attrs) { + OpBuilder::InsertionGuard g(builder); + result.addAttribute(SymbolTable::getSymbolAttrName(), builder.getStringAttr(name)); result.addAttribute(getFunctionTypeAttrName(result.name), @@ -1333,7 +1336,7 @@ void GPUFuncOp::build(OpBuilder &builder, OperationState &result, builder.getI64IntegerAttr(workgroupAttributions.size())); result.addAttributes(attrs); Region *body = result.addRegion(); - Block *entryBlock = new Block; + Block *entryBlock = builder.createBlock(body); // TODO: Allow passing in proper locations here. for (Type argTy : type.getInputs()) @@ -1342,8 +1345,6 @@ void GPUFuncOp::build(OpBuilder &builder, OperationState &result, entryBlock->addArgument(argTy, result.location); for (Type argTy : privateAttributions) entryBlock->addArgument(argTy, result.location); - - body->getBlocks().push_back(entryBlock); } /// Parses a GPU function memory attribution. diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp index 119df9acd9e9..233e702dbb22 100644 --- a/mlir/lib/Dialect/SCF/IR/SCF.cpp +++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp @@ -306,17 +306,18 @@ void ConditionOp::getSuccessorRegions( void ForOp::build(OpBuilder &builder, OperationState &result, Value lb, Value ub, Value step, ValueRange iterArgs, BodyBuilderFn bodyBuilder) { + OpBuilder::InsertionGuard guard(builder); + result.addOperands({lb, ub, step}); result.addOperands(iterArgs); for (Value v : iterArgs) result.addTypes(v.getType()); 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); for (Value v : iterArgs) - bodyBlock.addArgument(v.getType(), v.getLoc()); + bodyBlock->addArgument(v.getType(), v.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 @@ -325,9 +326,9 @@ void ForOp::build(OpBuilder &builder, OperationState &result, Value lb, ForOp::ensureTerminator(*bodyRegion, builder, result.location); } else if (bodyBuilder) { OpBuilder::InsertionGuard guard(builder); - builder.setInsertionPointToStart(&bodyBlock); - bodyBuilder(builder, result.location, bodyBlock.getArgument(0), - bodyBlock.getArguments().drop_front()); + builder.setInsertionPointToStart(bodyBlock); + bodyBuilder(builder, result.location, bodyBlock->getArgument(0), + bodyBlock->getArguments().drop_front()); } } |