[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
@@ -321,15 +323,15 @@ class RandomizedWorklist : public Worklist {
/// to the worklist in the beginning.
class GreedyPatternRewriteDriver : public RewriterBase::Listener {
protected:
- explicit GreedyPatternRewriteDriver(MLIRContext *ctx,
+ explicit GreedyPatternRewriteDriver(PatternRewriter &rewriter,
const FrozenRewritePatternSet &patterns,
const GreedyRewriteConfig &config);
/// Add the given operation to the worklist.
void addSingleOpToWorklist(Operation *op);
/// Add the given operation and its ancestors to the worklist.
- void addToWorklist(Operation *op);
+ virtual void addToWorklist(Operation *op);
matthias-springer wrote:
We can avoid this virtual function call by templatizing the driver class.
https://github.com/llvm/llvm-project/pull/93412
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
@@ -1053,3 +1055,241 @@ LogicalResult mlir::applyOpPatternsAndFold(
});
return converged;
}
+
+//===--===//
+// One-Shot Dialect Conversion Infrastructure
+//===--===//
+
+namespace {
+/// A conversion rewriter for the One-Shot Dialect Conversion. This rewriter
+/// immediately materializes all IR changes. It derives from
+/// `ConversionPatternRewriter` so that the existing conversion patterns can
+/// be used with the One-Shot Dialect Conversion.
+class OneShotConversionPatternRewriter : public ConversionPatternRewriter {
+public:
+ OneShotConversionPatternRewriter(MLIRContext *ctx)
+ : ConversionPatternRewriter(ctx) {}
+
+ bool canRecoverFromRewriteFailure() const override { return false; }
+
+ void replaceOp(Operation *op, ValueRange newValues) override;
+
+ void replaceOp(Operation *op, Operation *newOp) override {
+replaceOp(op, newOp->getResults());
+ }
+
+ void eraseOp(Operation *op) override { PatternRewriter::eraseOp(op); }
+
+ void eraseBlock(Block *block) override { PatternRewriter::eraseBlock(block);
}
+
+ void inlineBlockBefore(Block *source, Block *dest, Block::iterator before,
+ ValueRange argValues = std::nullopt) override {
+PatternRewriter::inlineBlockBefore(source, dest, before, argValues);
+ }
+ using PatternRewriter::inlineBlockBefore;
+
+ void startOpModification(Operation *op) override {
+PatternRewriter::startOpModification(op);
+ }
+
+ void finalizeOpModification(Operation *op) override {
+PatternRewriter::finalizeOpModification(op);
+ }
+
+ void cancelOpModification(Operation *op) override {
+PatternRewriter::cancelOpModification(op);
+ }
matthias-springer wrote:
The only one that would still be necessary is `replaceOp` because it must
insert unrealized_conversion_casts. All the others could be removed (because
the intermediate `ConversionPatternRewriter` class would be gone).
https://github.com/llvm/llvm-project/pull/93412
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
@@ -1053,3 +1055,241 @@ LogicalResult mlir::applyOpPatternsAndFold(
});
return converged;
}
+
+//===--===//
+// One-Shot Dialect Conversion Infrastructure
+//===--===//
+
+namespace {
+/// A conversion rewriter for the One-Shot Dialect Conversion. This rewriter
+/// immediately materializes all IR changes. It derives from
+/// `ConversionPatternRewriter` so that the existing conversion patterns can
+/// be used with the One-Shot Dialect Conversion.
+class OneShotConversionPatternRewriter : public ConversionPatternRewriter {
+public:
+ OneShotConversionPatternRewriter(MLIRContext *ctx)
+ : ConversionPatternRewriter(ctx) {}
+
+ bool canRecoverFromRewriteFailure() const override { return false; }
+
+ void replaceOp(Operation *op, ValueRange newValues) override;
+
+ void replaceOp(Operation *op, Operation *newOp) override {
+replaceOp(op, newOp->getResults());
+ }
+
+ void eraseOp(Operation *op) override { PatternRewriter::eraseOp(op); }
+
+ void eraseBlock(Block *block) override { PatternRewriter::eraseBlock(block);
}
+
+ void inlineBlockBefore(Block *source, Block *dest, Block::iterator before,
+ ValueRange argValues = std::nullopt) override {
+PatternRewriter::inlineBlockBefore(source, dest, before, argValues);
+ }
+ using PatternRewriter::inlineBlockBefore;
+
+ void startOpModification(Operation *op) override {
+PatternRewriter::startOpModification(op);
+ }
+
+ void finalizeOpModification(Operation *op) override {
+PatternRewriter::finalizeOpModification(op);
+ }
+
+ void cancelOpModification(Operation *op) override {
+PatternRewriter::cancelOpModification(op);
+ }
+
+ void setCurrentTypeConverter(const TypeConverter *converter) override {
+typeConverter = converter;
+ }
+
+ const TypeConverter *getCurrentTypeConverter() const override {
+return typeConverter;
+ }
+
+ LogicalResult getAdapterOperands(StringRef valueDiagTag,
+ std::optional inputLoc,
+ ValueRange values,
+ SmallVector &remapped) override;
+
+private:
+ /// Build an unrealized_conversion_cast op or look it up in the cache.
+ Value buildUnrealizedConversionCast(Location loc, Type type, Value value);
+
+ /// The current type converter.
+ const TypeConverter *typeConverter;
+
+ /// A cache for unrealized_conversion_casts. To ensure that identical casts
+ /// are not built multiple times.
+ DenseMap, Value> castCache;
matthias-springer wrote:
I think this can happen during `getAdapterOperands`. Each conversion pattern
can have its own type converter. (That's why there is `currentTypeConverter`
etc.)
```
func.func @foo(%arg0: i1) {
"op1"(%arg0)
"op2"(%arg0)
}
```
Let's assume that there is no conversion pattern for `func.func`. I.e., the
block argument is not converted. We could have two different patterns for "op1"
and "op2" with different type converters that convert `i1` to different types.
Now we have to insert two unrealized_conversion_casts with different result
types.
(This probably does not happen very often.)
https://github.com/llvm/llvm-project/pull/93412
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
@@ -1053,3 +1055,241 @@ LogicalResult mlir::applyOpPatternsAndFold(
});
return converged;
}
+
+//===--===//
+// One-Shot Dialect Conversion Infrastructure
+//===--===//
+
+namespace {
+/// A conversion rewriter for the One-Shot Dialect Conversion. This rewriter
+/// immediately materializes all IR changes. It derives from
+/// `ConversionPatternRewriter` so that the existing conversion patterns can
+/// be used with the One-Shot Dialect Conversion.
+class OneShotConversionPatternRewriter : public ConversionPatternRewriter {
+public:
+ OneShotConversionPatternRewriter(MLIRContext *ctx)
+ : ConversionPatternRewriter(ctx) {}
+
+ bool canRecoverFromRewriteFailure() const override { return false; }
+
+ void replaceOp(Operation *op, ValueRange newValues) override;
+
+ void replaceOp(Operation *op, Operation *newOp) override {
+replaceOp(op, newOp->getResults());
+ }
+
+ void eraseOp(Operation *op) override { PatternRewriter::eraseOp(op); }
+
+ void eraseBlock(Block *block) override { PatternRewriter::eraseBlock(block);
}
+
+ void inlineBlockBefore(Block *source, Block *dest, Block::iterator before,
+ ValueRange argValues = std::nullopt) override {
+PatternRewriter::inlineBlockBefore(source, dest, before, argValues);
+ }
+ using PatternRewriter::inlineBlockBefore;
+
+ void startOpModification(Operation *op) override {
+PatternRewriter::startOpModification(op);
+ }
+
+ void finalizeOpModification(Operation *op) override {
+PatternRewriter::finalizeOpModification(op);
+ }
+
+ void cancelOpModification(Operation *op) override {
+PatternRewriter::cancelOpModification(op);
+ }
+
+ void setCurrentTypeConverter(const TypeConverter *converter) override {
+typeConverter = converter;
+ }
+
+ const TypeConverter *getCurrentTypeConverter() const override {
+return typeConverter;
+ }
+
+ LogicalResult getAdapterOperands(StringRef valueDiagTag,
+ std::optional inputLoc,
+ ValueRange values,
+ SmallVector &remapped) override;
+
+private:
+ /// Build an unrealized_conversion_cast op or look it up in the cache.
+ Value buildUnrealizedConversionCast(Location loc, Type type, Value value);
+
+ /// The current type converter.
+ const TypeConverter *typeConverter;
+
+ /// A cache for unrealized_conversion_casts. To ensure that identical casts
+ /// are not built multiple times.
+ DenseMap, Value> castCache;
+};
+
+void OneShotConversionPatternRewriter::replaceOp(Operation *op,
+ ValueRange newValues) {
+ assert(op->getNumResults() == newValues.size());
+ for (auto [orig, repl] : llvm::zip_equal(op->getResults(), newValues)) {
+if (orig.getType() != repl.getType()) {
+ // Type mismatch: insert unrealized_conversion cast.
+ replaceAllUsesWith(orig, buildUnrealizedConversionCast(
+ op->getLoc(), orig.getType(), repl));
+} else {
+ // Same type: use replacement value directly.
+ replaceAllUsesWith(orig, repl);
+}
+ }
+ eraseOp(op);
+}
+
+Value OneShotConversionPatternRewriter::buildUnrealizedConversionCast(
+Location loc, Type type, Value value) {
+ auto it = castCache.find(std::make_pair(value, type));
+ if (it != castCache.end())
+return it->second;
+
+ // Insert cast at the beginning of the block (for block arguments) or right
+ // after the defining op.
+ OpBuilder::InsertionGuard g(*this);
+ Block *insertBlock = value.getParentBlock();
+ Block::iterator insertPt = insertBlock->begin();
+ if (OpResult inputRes = dyn_cast(value))
+insertPt = ++inputRes.getOwner()->getIterator();
+ setInsertionPoint(insertBlock, insertPt);
+ auto castOp = create(loc, type, value);
+ castCache[std::make_pair(value, type)] = castOp.getOutputs()[0];
+ return castOp.getOutputs()[0];
+}
+
+class ConversionPatternRewriteDriver : public GreedyPatternRewriteDriver {
matthias-springer wrote:
"One Shot" may be a misnomer. I really meant "no rollback".
I am reluctant to build something that is not compatible with the large number
of existing conversion patterns. I would rather gradually improve the situation
in way where users have a painless way to migrate to the new driver. Otherwise,
is anybody going to use the new infrastructure?
https://github.com/llvm/llvm-project/pull/93412
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
@@ -1053,3 +1055,241 @@ LogicalResult mlir::applyOpPatternsAndFold(
});
return converged;
}
+
+//===--===//
+// One-Shot Dialect Conversion Infrastructure
+//===--===//
+
+namespace {
+/// A conversion rewriter for the One-Shot Dialect Conversion. This rewriter
+/// immediately materializes all IR changes. It derives from
+/// `ConversionPatternRewriter` so that the existing conversion patterns can
+/// be used with the One-Shot Dialect Conversion.
+class OneShotConversionPatternRewriter : public ConversionPatternRewriter {
+public:
+ OneShotConversionPatternRewriter(MLIRContext *ctx)
+ : ConversionPatternRewriter(ctx) {}
+
+ bool canRecoverFromRewriteFailure() const override { return false; }
+
+ void replaceOp(Operation *op, ValueRange newValues) override;
+
+ void replaceOp(Operation *op, Operation *newOp) override {
+replaceOp(op, newOp->getResults());
+ }
+
+ void eraseOp(Operation *op) override { PatternRewriter::eraseOp(op); }
+
+ void eraseBlock(Block *block) override { PatternRewriter::eraseBlock(block);
}
+
+ void inlineBlockBefore(Block *source, Block *dest, Block::iterator before,
+ ValueRange argValues = std::nullopt) override {
+PatternRewriter::inlineBlockBefore(source, dest, before, argValues);
+ }
+ using PatternRewriter::inlineBlockBefore;
+
+ void startOpModification(Operation *op) override {
+PatternRewriter::startOpModification(op);
+ }
+
+ void finalizeOpModification(Operation *op) override {
+PatternRewriter::finalizeOpModification(op);
+ }
+
+ void cancelOpModification(Operation *op) override {
+PatternRewriter::cancelOpModification(op);
+ }
+
+ void setCurrentTypeConverter(const TypeConverter *converter) override {
+typeConverter = converter;
+ }
+
+ const TypeConverter *getCurrentTypeConverter() const override {
+return typeConverter;
+ }
+
+ LogicalResult getAdapterOperands(StringRef valueDiagTag,
+ std::optional inputLoc,
+ ValueRange values,
+ SmallVector &remapped) override;
+
+private:
+ /// Build an unrealized_conversion_cast op or look it up in the cache.
+ Value buildUnrealizedConversionCast(Location loc, Type type, Value value);
+
+ /// The current type converter.
+ const TypeConverter *typeConverter;
+
+ /// A cache for unrealized_conversion_casts. To ensure that identical casts
+ /// are not built multiple times.
+ DenseMap, Value> castCache;
+};
+
+void OneShotConversionPatternRewriter::replaceOp(Operation *op,
+ ValueRange newValues) {
+ assert(op->getNumResults() == newValues.size());
+ for (auto [orig, repl] : llvm::zip_equal(op->getResults(), newValues)) {
+if (orig.getType() != repl.getType()) {
+ // Type mismatch: insert unrealized_conversion cast.
+ replaceAllUsesWith(orig, buildUnrealizedConversionCast(
+ op->getLoc(), orig.getType(), repl));
+} else {
+ // Same type: use replacement value directly.
+ replaceAllUsesWith(orig, repl);
+}
+ }
+ eraseOp(op);
+}
+
+Value OneShotConversionPatternRewriter::buildUnrealizedConversionCast(
+Location loc, Type type, Value value) {
+ auto it = castCache.find(std::make_pair(value, type));
+ if (it != castCache.end())
+return it->second;
+
+ // Insert cast at the beginning of the block (for block arguments) or right
+ // after the defining op.
+ OpBuilder::InsertionGuard g(*this);
+ Block *insertBlock = value.getParentBlock();
+ Block::iterator insertPt = insertBlock->begin();
+ if (OpResult inputRes = dyn_cast(value))
+insertPt = ++inputRes.getOwner()->getIterator();
+ setInsertionPoint(insertBlock, insertPt);
+ auto castOp = create(loc, type, value);
+ castCache[std::make_pair(value, type)] = castOp.getOutputs()[0];
+ return castOp.getOutputs()[0];
+}
+
+class ConversionPatternRewriteDriver : public GreedyPatternRewriteDriver {
matthias-springer wrote:
What's the main issue that you see with a worklist-based approach? Imo, the
main complexity of the dialect conversion is because of rollback and late
materialization. I never had issues issues debugging a greedy pattern rewrite
with `-debug` so far.
https://github.com/llvm/llvm-project/pull/93412
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
@@ -1053,3 +1055,241 @@ LogicalResult mlir::applyOpPatternsAndFold(
});
return converged;
}
+
+//===--===//
+// One-Shot Dialect Conversion Infrastructure
+//===--===//
+
+namespace {
+/// A conversion rewriter for the One-Shot Dialect Conversion. This rewriter
+/// immediately materializes all IR changes. It derives from
+/// `ConversionPatternRewriter` so that the existing conversion patterns can
+/// be used with the One-Shot Dialect Conversion.
+class OneShotConversionPatternRewriter : public ConversionPatternRewriter {
+public:
+ OneShotConversionPatternRewriter(MLIRContext *ctx)
+ : ConversionPatternRewriter(ctx) {}
+
+ bool canRecoverFromRewriteFailure() const override { return false; }
+
+ void replaceOp(Operation *op, ValueRange newValues) override;
+
+ void replaceOp(Operation *op, Operation *newOp) override {
+replaceOp(op, newOp->getResults());
+ }
+
+ void eraseOp(Operation *op) override { PatternRewriter::eraseOp(op); }
+
+ void eraseBlock(Block *block) override { PatternRewriter::eraseBlock(block);
}
+
+ void inlineBlockBefore(Block *source, Block *dest, Block::iterator before,
+ ValueRange argValues = std::nullopt) override {
+PatternRewriter::inlineBlockBefore(source, dest, before, argValues);
+ }
+ using PatternRewriter::inlineBlockBefore;
+
+ void startOpModification(Operation *op) override {
+PatternRewriter::startOpModification(op);
+ }
+
+ void finalizeOpModification(Operation *op) override {
+PatternRewriter::finalizeOpModification(op);
+ }
+
+ void cancelOpModification(Operation *op) override {
+PatternRewriter::cancelOpModification(op);
+ }
+
+ void setCurrentTypeConverter(const TypeConverter *converter) override {
+typeConverter = converter;
+ }
+
+ const TypeConverter *getCurrentTypeConverter() const override {
+return typeConverter;
+ }
+
+ LogicalResult getAdapterOperands(StringRef valueDiagTag,
+ std::optional inputLoc,
+ ValueRange values,
+ SmallVector &remapped) override;
+
+private:
+ /// Build an unrealized_conversion_cast op or look it up in the cache.
+ Value buildUnrealizedConversionCast(Location loc, Type type, Value value);
+
+ /// The current type converter.
+ const TypeConverter *typeConverter;
+
+ /// A cache for unrealized_conversion_casts. To ensure that identical casts
+ /// are not built multiple times.
+ DenseMap, Value> castCache;
+};
+
+void OneShotConversionPatternRewriter::replaceOp(Operation *op,
+ ValueRange newValues) {
+ assert(op->getNumResults() == newValues.size());
+ for (auto [orig, repl] : llvm::zip_equal(op->getResults(), newValues)) {
+if (orig.getType() != repl.getType()) {
+ // Type mismatch: insert unrealized_conversion cast.
+ replaceAllUsesWith(orig, buildUnrealizedConversionCast(
+ op->getLoc(), orig.getType(), repl));
+} else {
+ // Same type: use replacement value directly.
+ replaceAllUsesWith(orig, repl);
+}
+ }
+ eraseOp(op);
+}
+
+Value OneShotConversionPatternRewriter::buildUnrealizedConversionCast(
+Location loc, Type type, Value value) {
+ auto it = castCache.find(std::make_pair(value, type));
+ if (it != castCache.end())
+return it->second;
+
+ // Insert cast at the beginning of the block (for block arguments) or right
+ // after the defining op.
+ OpBuilder::InsertionGuard g(*this);
+ Block *insertBlock = value.getParentBlock();
+ Block::iterator insertPt = insertBlock->begin();
+ if (OpResult inputRes = dyn_cast(value))
+insertPt = ++inputRes.getOwner()->getIterator();
+ setInsertionPoint(insertBlock, insertPt);
+ auto castOp = create(loc, type, value);
+ castCache[std::make_pair(value, type)] = castOp.getOutputs()[0];
+ return castOp.getOutputs()[0];
+}
+
+class ConversionPatternRewriteDriver : public GreedyPatternRewriteDriver {
matthias-springer wrote:
> I don't have problem with dialect conversion complexity actually, other than
> replaceAllUsesWith not doing what it says it does
I tried to fix that [here](https://github.com/llvm/llvm-project/pull/84725). It
sounded like the rollback logic was getting too complex. (I would agree with
that.)
> Without rollback you're incompatible anyway, people can't "just adopt it".
I expect that most dialect conversions do not actually need the rollback. I am
not aware of any passes in MLIR that require it. (But users may have their own
passes that rely on it, that's why I asked in the RFC.) So my hope is that the
existing patterns just work with the new driver. I only tried it with
`NVGPUToNVVM.cpp` and `ComplexToStandard.cpp` so far, and have to try migrating
a few more passe
[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/93412 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/93412 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Simplify handling of dropped arguments (PR #96207)
https://github.com/matthias-springer created https://github.com/llvm/llvm-project/pull/96207 This commit simplifies the handling of dropped arguments and updates some dialect conversion documentation that is outdated. When converting a block signature, a `BlockTypeConversionRewrite` object and potentially multiple `ReplaceBlockArgRewrite` are created. During the "commit" phase, uses of the old block arguments are replaced with the new block arguments, but the old implementation was written in an inconsistent way: some block arguments were replaced in `BlockTypeConversionRewrite::commit` and some were replaced in `ReplaceBlockArgRewrite::commit`. The new `BlockTypeConversionRewrite::commit` implementation is much simpler and no longer modifies any IR; that is done only in `ReplaceBlockArgRewrite` now. The `ConvertedArgInfo` data structure is no longer needed. To that end, materializations of dropped arguments are now built in `applySignatureConversion` instead of `materializeLiveConversions`; the latter function no longer has to deal with dropped arguments. Other minor improvements: - Improve variable name: `origOutputType` -> `origArgType`. Add an assertion to check that this field is only used for argument materializations. - Add more comments to `applySignatureConversion`. Note: Error messages around failed materializations for dropped basic block arguments changed slightly. That is because those materializations are now built in `legalizeUnresolvedMaterialization` instead of `legalizeConvertedArgumentTypes`. This commit is in preparation of decoupling argument/source/target materializations from the dialect conversion. >From ed7bb706dc8eaec2f45a8f63c98d9b9b7fac23a8 Mon Sep 17 00:00:00 2001 From: Matthias Springer Date: Thu, 20 Jun 2024 11:18:44 +0200 Subject: [PATCH] [mlir][Transforms] Dialect conversion: Simplify handling of dropped arguments This commit simplifies the handling of dropped arguments and updates some dialect conversion documentation that is outdated. When converting a block signature, a `BlockTypeConversionRewrite` object and potentially multiple `ReplaceBlockArgRewrite` are created. During the "commit" phase, uses of the old block arguments are replaced with the new block arguments, but the old implementation was written in an inconsistent way: some block arguments were replaced in `BlockTypeConversionRewrite::commit` and some were replaced in `ReplaceBlockArgRewrite::commit`. The new `BlockTypeConversionRewrite::commit` implementation is much simpler and no longer modifies any IR; that is done only in `ReplaceBlockArgRewrite` now. The `ConvertedArgInfo` data structure is no longer needed. To that end, materializations of dropped arguments are now built in `applySignatureConversion` instead of `materializeLiveConversions`; the latter function no longer has to deal with dropped arguments. Other minor improvements: - Improve variable name: `origOutputType` -> `origArgType`. Add an assertion to check that this field is only used for argument materializations. - Add more comments to `applySignatureConversion`. Note: Error messages around failed materializations for dropped basic block arguments changed slightly. That is because those materializations are now built in `legalizeUnresolvedMaterialization` instead of `legalizeConvertedArgumentTypes`. --- mlir/docs/DialectConversion.md| 37 +++- .../mlir/Transforms/DialectConversion.h | 10 +- .../Transforms/Utils/DialectConversion.cpp| 208 +++--- .../test-legalize-type-conversion.mlir| 6 +- 4 files changed, 111 insertions(+), 150 deletions(-) diff --git a/mlir/docs/DialectConversion.md b/mlir/docs/DialectConversion.md index 69781bb868bbf..f722974a9a1e5 100644 --- a/mlir/docs/DialectConversion.md +++ b/mlir/docs/DialectConversion.md @@ -246,6 +246,13 @@ depending on the situation. - An argument materialization is used when converting the type of a block argument during a [signature conversion](#region-signature-conversion). +The new block argument types are specified in a `SignatureConversion` +object. An original block argument can be converted into multiple +block arguments, which is not supported everywhere in the dialect +conversion. (E.g., adaptors support only a single replacement value for +each original value.) Therefore, an argument materialization is used to +convert potentially multiple new block arguments back into a single SSA +value. * Source Materialization @@ -259,6 +266,9 @@ depending on the situation. * When a block argument has been converted to a different type, but the original argument still has users that will remain live after the conversion process has finished. +* When a block argument has been dropped, but the argument still has +users that will remain live after the conversion pro
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Dialect Conversion: Move argument materialization logic (PR #96329)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/96329
This commit moves the argument materialization logic from
`legalizeConvertedArgumentTypes` to `legalizeUnresolvedMaterializations`.
Before this change:
- Argument materializations were created in `legalizeConvertedArgumentTypes`
(which used to call `materializeLiveConversions`).
After this change:
- `legalizeConvertedArgumentTypes` creates a "placeholder"
`unrealized_conversion_cast`.
- The placeholder `unrealized_conversion_cast` is replaced with an argument
materialization (using the type converter) in
`legalizeUnresolvedMaterializations`.
- All argument and target materializations now take place in the same location
(`legalizeUnresolvedMaterializations`).
This commit brings us closer towards creating all source/target/argument
materializations in one central step, which can then be made optional (and
delegated to the user) in the future. (There is one more source materialization
step that has not been moved yet.)
This commit also consolidates all `build*UnresolvedMaterialization` functions
into a single `buildUnresolvedMaterialization` function.
>From d2b0b9ef97c626fc48b0c00ce2ec8e5573599f2b Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Thu, 20 Jun 2024 17:40:07 +0200
Subject: [PATCH] remove materializeLiveConversions
---
.../Transforms/Utils/DialectConversion.cpp| 133 +++---
1 file changed, 52 insertions(+), 81 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 07ebd687ee2b3..47e03383304af 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -53,6 +53,16 @@ static void logFailure(llvm::ScopedPrinter &os, StringRef
fmt, Args &&...args) {
});
}
+/// Helper function that computes an insertion point where the given value is
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(Value value) {
+ Block *insertBlock = value.getParentBlock();
+ Block::iterator insertPt = insertBlock->begin();
+ if (OpResult inputRes = dyn_cast(value))
+insertPt = ++inputRes.getOwner()->getIterator();
+ return OpBuilder::InsertPoint(insertBlock, insertPt);
+}
+
//===--===//
// ConversionValueMapping
//===--===//
@@ -445,11 +455,9 @@ class BlockTypeConversionRewrite : public BlockRewrite {
return rewrite->getKind() == Kind::BlockTypeConversion;
}
- /// Materialize any necessary conversions for converted arguments that have
- /// live users, using the provided `findLiveUser` to search for a user that
- /// survives the conversion process.
- LogicalResult
- materializeLiveConversions(function_ref findLiveUser);
+ Block *getOrigBlock() const { return origBlock; }
+
+ const TypeConverter *getConverter() const { return converter; }
void commit(RewriterBase &rewriter) override;
@@ -841,14 +849,10 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
/// Build an unresolved materialization operation given an output type and
set
/// of input operands.
Value buildUnresolvedMaterialization(MaterializationKind kind,
- Block *insertBlock,
- Block::iterator insertPt, Location loc,
+ OpBuilder::InsertPoint ip, Location loc,
ValueRange inputs, Type outputType,
Type origOutputType,
const TypeConverter *converter);
- Value buildUnresolvedTargetMaterialization(Location loc, Value input,
- Type outputType,
- const TypeConverter *converter);
//======//
// Rewriter Notification Hooks
@@ -981,49 +985,6 @@ void BlockTypeConversionRewrite::rollback() {
block->replaceAllUsesWith(origBlock);
}
-LogicalResult BlockTypeConversionRewrite::materializeLiveConversions(
-function_ref findLiveUser) {
- // Process the remapping for each of the original arguments.
- for (auto it : llvm::enumerate(origBlock->getArguments())) {
-BlockArgument origArg = it.value();
-// Note: `block` may be detached, so OpBuilder::atBlockBegin cannot be
used.
-OpBuilder builder(it.value().getContext(), /*listener=*/&rewriterImpl);
-builder.setInsertionPointToStart(block);
-
-// If the type of this argument changed and the argument is still live, we
-// need to materialize a conversion.
-if (rewriterImpl.mapping.lookupOrNull(origArg, origArg.getType()))
- continue;
-Operation *liveUser = findLiveUser(
[llvm-branch-commits] [mlir] [mlir][Conversion] `FuncToLLVM`: Simplify bare-pointer handling (PR #96393)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/96393
Before this commit, there used to be a workaround in the `func.func`/`gpu.func`
op lowering when the bare-pointer calling convention is enabled. This
workaround "patched up" the argument materializations for memref arguments.
This can be done directly in the argument materialization functions (as the
TODOs in the code base indicate).
This commit effectively reverts back to the old implementation
(a664c14001fa2359604527084c91d0864aa131a4) and adds additional checks to make
sure that bare pointers are used only for function entry block arguments.
>From f65911a2b08c538d24a9b2044123390ceae5b4b5 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Sat, 22 Jun 2024 14:54:21 +0200
Subject: [PATCH] [mlir][Conversion] `FuncToLLVM`: Simplify bare-pointer
handling
Before this commit, there used to be a workaround in the `func.func`/`gpu.func`
op lowering when the bare-pointer calling convention was enabled. This
workaround "patched up" the argument materializations for memref arguments.
This can be done directly in the argument materialization functions (as the
TODOs in the code base indicate).
---
mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp | 53
.../Conversion/GPUCommon/GPUOpsLowering.cpp | 128 +++---
.../Conversion/LLVMCommon/TypeConverter.cpp | 22 ++-
3 files changed, 66 insertions(+), 137 deletions(-)
diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index 744236692fbb6..efb80467369a2 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -268,55 +268,6 @@ static void wrapExternalFunction(OpBuilder &builder,
Location loc,
}
}
-/// Modifies the body of the function to construct the `MemRefDescriptor` from
-/// the bare pointer calling convention lowering of `memref` types.
-static void modifyFuncOpToUseBarePtrCallingConv(
-ConversionPatternRewriter &rewriter, Location loc,
-const LLVMTypeConverter &typeConverter, LLVM::LLVMFuncOp funcOp,
-TypeRange oldArgTypes) {
- if (funcOp.getBody().empty())
-return;
-
- // Promote bare pointers from memref arguments to memref descriptors at the
- // beginning of the function so that all the memrefs in the function have a
- // uniform representation.
- Block *entryBlock = &funcOp.getBody().front();
- auto blockArgs = entryBlock->getArguments();
- assert(blockArgs.size() == oldArgTypes.size() &&
- "The number of arguments and types doesn't match");
-
- OpBuilder::InsertionGuard guard(rewriter);
- rewriter.setInsertionPointToStart(entryBlock);
- for (auto it : llvm::zip(blockArgs, oldArgTypes)) {
-BlockArgument arg = std::get<0>(it);
-Type argTy = std::get<1>(it);
-
-// Unranked memrefs are not supported in the bare pointer calling
-// convention. We should have bailed out before in the presence of
-// unranked memrefs.
-assert(!isa(argTy) &&
- "Unranked memref is not supported");
-auto memrefTy = dyn_cast(argTy);
-if (!memrefTy)
- continue;
-
-// Replace barePtr with a placeholder (undef), promote barePtr to a ranked
-// or unranked memref descriptor and replace placeholder with the last
-// instruction of the memref descriptor.
-// TODO: The placeholder is needed to avoid replacing barePtr uses in the
-// MemRef descriptor instructions. We may want to have a utility in the
-// rewriter to properly handle this use case.
-Location loc = funcOp.getLoc();
-auto placeholder = rewriter.create(
-loc, typeConverter.convertType(memrefTy));
-rewriter.replaceUsesOfBlockArgument(arg, placeholder);
-
-Value desc = MemRefDescriptor::fromStaticShape(rewriter, loc,
typeConverter,
- memrefTy, arg);
-rewriter.replaceOp(placeholder, {desc});
- }
-}
-
FailureOr
mlir::convertFuncOpToLLVMFuncOp(FunctionOpInterface funcOp,
ConversionPatternRewriter &rewriter,
@@ -462,10 +413,6 @@ mlir::convertFuncOpToLLVMFuncOp(FunctionOpInterface funcOp,
wrapForExternalCallers(rewriter, funcOp->getLoc(), converter, funcOp,
newFuncOp);
}
- } else {
-modifyFuncOpToUseBarePtrCallingConv(
-rewriter, funcOp->getLoc(), converter, newFuncOp,
-llvm::cast(funcOp.getFunctionType()).getInputs());
}
return newFuncOp;
diff --git a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
index 7ea05b7e7f6c1..6053e34f30a41 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
@@ -182,35 +182,6 @@ GPUFuncOpLowering::matchAndRewrite(gpu::GPUFuncOp
gpuFuncOp, OpAdaptor adaptor,
&signatureConversion)))
return failure();
- /
[llvm-branch-commits] [mlir] [mlir][Conversion] `FuncToLLVM`: Simplify bare-pointer handling (PR #96393)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/96393
>From 2d838580bf8c17ea7a17d73415b3c64c1775b37d Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Sat, 22 Jun 2024 14:54:21 +0200
Subject: [PATCH] [mlir][Conversion] `FuncToLLVM`: Simplify bare-pointer
handling
Before this commit, there used to be a workaround in the `func.func`/`gpu.func`
op lowering when the bare-pointer calling convention was enabled. This
workaround "patched up" the argument materializations for memref arguments.
This can be done directly in the argument materialization functions (as the
TODOs in the code base indicate).
---
mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp | 53 ---
.../Conversion/GPUCommon/GPUOpsLowering.cpp | 29 --
.../Conversion/LLVMCommon/TypeConverter.cpp | 22 ++--
3 files changed, 17 insertions(+), 87 deletions(-)
diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index 744236692fbb6..efb80467369a2 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -268,55 +268,6 @@ static void wrapExternalFunction(OpBuilder &builder,
Location loc,
}
}
-/// Modifies the body of the function to construct the `MemRefDescriptor` from
-/// the bare pointer calling convention lowering of `memref` types.
-static void modifyFuncOpToUseBarePtrCallingConv(
-ConversionPatternRewriter &rewriter, Location loc,
-const LLVMTypeConverter &typeConverter, LLVM::LLVMFuncOp funcOp,
-TypeRange oldArgTypes) {
- if (funcOp.getBody().empty())
-return;
-
- // Promote bare pointers from memref arguments to memref descriptors at the
- // beginning of the function so that all the memrefs in the function have a
- // uniform representation.
- Block *entryBlock = &funcOp.getBody().front();
- auto blockArgs = entryBlock->getArguments();
- assert(blockArgs.size() == oldArgTypes.size() &&
- "The number of arguments and types doesn't match");
-
- OpBuilder::InsertionGuard guard(rewriter);
- rewriter.setInsertionPointToStart(entryBlock);
- for (auto it : llvm::zip(blockArgs, oldArgTypes)) {
-BlockArgument arg = std::get<0>(it);
-Type argTy = std::get<1>(it);
-
-// Unranked memrefs are not supported in the bare pointer calling
-// convention. We should have bailed out before in the presence of
-// unranked memrefs.
-assert(!isa(argTy) &&
- "Unranked memref is not supported");
-auto memrefTy = dyn_cast(argTy);
-if (!memrefTy)
- continue;
-
-// Replace barePtr with a placeholder (undef), promote barePtr to a ranked
-// or unranked memref descriptor and replace placeholder with the last
-// instruction of the memref descriptor.
-// TODO: The placeholder is needed to avoid replacing barePtr uses in the
-// MemRef descriptor instructions. We may want to have a utility in the
-// rewriter to properly handle this use case.
-Location loc = funcOp.getLoc();
-auto placeholder = rewriter.create(
-loc, typeConverter.convertType(memrefTy));
-rewriter.replaceUsesOfBlockArgument(arg, placeholder);
-
-Value desc = MemRefDescriptor::fromStaticShape(rewriter, loc,
typeConverter,
- memrefTy, arg);
-rewriter.replaceOp(placeholder, {desc});
- }
-}
-
FailureOr
mlir::convertFuncOpToLLVMFuncOp(FunctionOpInterface funcOp,
ConversionPatternRewriter &rewriter,
@@ -462,10 +413,6 @@ mlir::convertFuncOpToLLVMFuncOp(FunctionOpInterface funcOp,
wrapForExternalCallers(rewriter, funcOp->getLoc(), converter, funcOp,
newFuncOp);
}
- } else {
-modifyFuncOpToUseBarePtrCallingConv(
-rewriter, funcOp->getLoc(), converter, newFuncOp,
-llvm::cast(funcOp.getFunctionType()).getInputs());
}
return newFuncOp;
diff --git a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
index 3e6fcc076fb4d..6053e34f30a41 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
@@ -182,35 +182,6 @@ GPUFuncOpLowering::matchAndRewrite(gpu::GPUFuncOp
gpuFuncOp, OpAdaptor adaptor,
&signatureConversion)))
return failure();
- // If bare memref pointers are being used, remap them back to memref
- // descriptors This must be done after signature conversion to get rid of the
- // unrealized casts.
- if (getTypeConverter()->getOptions().useBarePtrCallConv) {
-OpBuilder::InsertionGuard guard(rewriter);
-rewriter.setInsertionPointToStart(&llvmFuncOp.getBody().front());
-for (const auto [idx, argTy] :
- llvm::enumerate(gpuFuncOp.getArgumentTypes())) {
- auto memrefTy = dyn_cast(argTy);
- if (!memrefTy)
-continue;
- assert(memrefTy.has
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Fix missing source materialization (PR #97903)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/97903 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Fix missing source materialization (PR #97903)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/97903 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Simplify handling of dropped arguments (PR #97213)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/97213 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Simplify handling of dropped arguments (PR #97213)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/97213 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Fix missing source materialization (PR #97903)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/97903
>From 577317640c0f5800650157f5c4c6fbe6220eba38 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Sat, 6 Jul 2024 14:28:41 +0200
Subject: [PATCH] fix test
---
mlir/docs/DialectConversion.md| 3 +-
.../SCF/TransformOps/SCFTransformOps.td | 11 +++
.../mlir/Transforms/DialectConversion.h | 10 +--
.../Conversion/LLVMCommon/TypeConverter.cpp | 28 --
.../Dialect/SCF/TransformOps/CMakeLists.txt | 1 +
.../SCF/TransformOps/SCFTransformOps.cpp | 13 ++-
.../Transforms/Utils/DialectConversion.cpp| 87 ++-
.../FuncToLLVM/func-memref-return.mlir| 4 +-
.../Transforms/test-block-legalization.mlir | 44 ++
9 files changed, 141 insertions(+), 60 deletions(-)
create mode 100644 mlir/test/Transforms/test-block-legalization.mlir
diff --git a/mlir/docs/DialectConversion.md b/mlir/docs/DialectConversion.md
index db26e6477d5fc..23e74470a835f 100644
--- a/mlir/docs/DialectConversion.md
+++ b/mlir/docs/DialectConversion.md
@@ -352,7 +352,8 @@ class TypeConverter {
/// This method registers a materialization that will be called when
/// converting (potentially multiple) block arguments that were the result of
- /// a signature conversion of a single block argument, to a single SSA value.
+ /// a signature conversion of a single block argument, to a single SSA value
+ /// with the old argument type.
template ::template
arg_t<1>>
void addArgumentMaterialization(FnT &&callback) {
diff --git a/mlir/include/mlir/Dialect/SCF/TransformOps/SCFTransformOps.td
b/mlir/include/mlir/Dialect/SCF/TransformOps/SCFTransformOps.td
index 7bf914f6456ce..20880d94a83ca 100644
--- a/mlir/include/mlir/Dialect/SCF/TransformOps/SCFTransformOps.td
+++ b/mlir/include/mlir/Dialect/SCF/TransformOps/SCFTransformOps.td
@@ -38,6 +38,17 @@ def ApplySCFStructuralConversionPatternsOp :
Op]> {
+ let description = [{
+Collects patterns that lower structured control flow ops to unstructured
+control flow.
+ }];
+
+ let assemblyFormat = "attr-dict";
+}
+
def Transform_ScfForOp : Transform_ConcreteOpType<"scf.for">;
def ForallToForOp : Op>::template arg_t<1>>
void addArgumentMaterialization(FnT &&callback) {
diff --git a/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
b/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
index f5620a6a7cd91..32d02d5e438bd 100644
--- a/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
+++ b/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp
@@ -153,9 +153,11 @@ LLVMTypeConverter::LLVMTypeConverter(MLIRContext *ctx,
type.isVarArg());
});
- // Materialization for memrefs creates descriptor structs from individual
- // values constituting them, when descriptors are used, i.e. more than one
- // value represents a memref.
+ // Argument materializations convert from the new block argument types
+ // (multiple SSA values that make up a memref descriptor) back to the
+ // original block argument type. The dialect conversion framework will then
+ // insert a target materialization from the original block argument type to
+ // a legal type.
addArgumentMaterialization(
[&](OpBuilder &builder, UnrankedMemRefType resultType, ValueRange inputs,
Location loc) -> std::optional {
@@ -164,12 +166,18 @@ LLVMTypeConverter::LLVMTypeConverter(MLIRContext *ctx,
// memref descriptor cannot be built just from a bare pointer.
return std::nullopt;
}
-return UnrankedMemRefDescriptor::pack(builder, loc, *this, resultType,
- inputs);
+Value desc = UnrankedMemRefDescriptor::pack(builder, loc, *this,
+resultType, inputs);
+// An argument materialization must return a value of type
+// `resultType`, so insert a cast from the memref descriptor type
+// (!llvm.struct) to the original memref type.
+return builder.create(loc, resultType,
desc)
+.getResult(0);
});
addArgumentMaterialization([&](OpBuilder &builder, MemRefType resultType,
ValueRange inputs,
Location loc) -> std::optional {
+Value desc;
if (inputs.size() == 1) {
// This is a bare pointer. We allow bare pointers only for function entry
// blocks.
@@ -180,10 +188,16 @@ LLVMTypeConverter::LLVMTypeConverter(MLIRContext *ctx,
if (!block->isEntryBlock() ||
!isa(block->getParentOp()))
return std::nullopt;
- return MemRefDescriptor::fromStaticShape(builder, loc, *this, resultType,
+ desc = MemRefDescriptor::fromStaticShape(builder, loc, *this, resultType,
inputs[0]);
+} else {
+ desc = MemRefDescriptor::pack(builder, loc, *this, resultType, inputs);
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Simplify handling of dropped arguments (PR #97213)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/97213
>From b0b781318e5f76f28b9340efbdcc3f91c2c1cf31 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Sat, 13 Jul 2024 17:36:37 +0200
Subject: [PATCH] [mlir][Transforms] Dialect conversion: Simplify handling of
dropped arguments
This commit simplifies the handling of dropped arguments and updates some
dialect conversion documentation that is outdated.
When converting a block signature, a BlockTypeConversionRewrite object and
potentially multiple ReplaceBlockArgRewrite are created. During the "commit"
phase, uses of the old block arguments are replaced with the new block
arguments, but the old implementation was written in an inconsistent way: some
block arguments were replaced in BlockTypeConversionRewrite::commit and some
were replaced in ReplaceBlockArgRewrite::commit. The new
BlockTypeConversionRewrite::commit implementation is much simpler and no longer
modifies any IR; that is done only in ReplaceBlockArgRewrite now. The
ConvertedArgInfo data structure is no longer needed.
To that end, materializations of dropped arguments are now built in
applySignatureConversion instead of materializeLiveConversions; the latter
function no longer has to deal with dropped arguments.
Other minor improvements:
Improve variable name: origOutputType -> origArgType. Add an assertion to check
that this field is only used for argument materializations.
Add more comments to applySignatureConversion.
Note: Error messages around failed materializations for dropped basic block
arguments changed slightly. That is because those materializations are now
built in legalizeUnresolvedMaterialization instead of
legalizeConvertedArgumentTypes.
This commit is in preparation of decoupling argument/source/target
materializations from the dialect conversion.
This is a re-upload of #96207.
---
.../Transforms/Utils/DialectConversion.cpp| 173 ++
.../test-legalize-type-conversion.mlir| 6 +-
2 files changed, 57 insertions(+), 122 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 1e0afee2373a9..0b552a7e1ca3b 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -432,34 +432,14 @@ class MoveBlockRewrite : public BlockRewrite {
Block *insertBeforeBlock;
};
-/// This structure contains the information pertaining to an argument that has
-/// been converted.
-struct ConvertedArgInfo {
- ConvertedArgInfo(unsigned newArgIdx, unsigned newArgSize,
- Value castValue = nullptr)
- : newArgIdx(newArgIdx), newArgSize(newArgSize), castValue(castValue) {}
-
- /// The start index of in the new argument list that contains arguments that
- /// replace the original.
- unsigned newArgIdx;
-
- /// The number of arguments that replaced the original argument.
- unsigned newArgSize;
-
- /// The cast value that was created to cast from the new arguments to the
- /// old. This only used if 'newArgSize' > 1.
- Value castValue;
-};
-
/// Block type conversion. This rewrite is partially reflected in the IR.
class BlockTypeConversionRewrite : public BlockRewrite {
public:
- BlockTypeConversionRewrite(
- ConversionPatternRewriterImpl &rewriterImpl, Block *block,
- Block *origBlock, SmallVector, 1>
argInfo,
- const TypeConverter *converter)
+ BlockTypeConversionRewrite(ConversionPatternRewriterImpl &rewriterImpl,
+ Block *block, Block *origBlock,
+ const TypeConverter *converter)
: BlockRewrite(Kind::BlockTypeConversion, rewriterImpl, block),
-origBlock(origBlock), argInfo(argInfo), converter(converter) {}
+origBlock(origBlock), converter(converter) {}
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() == Kind::BlockTypeConversion;
@@ -479,10 +459,6 @@ class BlockTypeConversionRewrite : public BlockRewrite {
/// The original block that was requested to have its signature converted.
Block *origBlock;
- /// The conversion information for each of the arguments. The information is
- /// std::nullopt if the argument was dropped during conversion.
- SmallVector, 1> argInfo;
-
/// The type converter used to convert the arguments.
const TypeConverter *converter;
};
@@ -691,12 +667,16 @@ class CreateOperationRewrite : public OperationRewrite {
/// The type of materialization.
enum MaterializationKind {
/// This materialization materializes a conversion for an illegal block
- /// argument type, to a legal one.
+ /// argument type, to the original one.
Argument,
/// This materialization materializes a conversion from an illegal type to a
/// legal one.
- Target
+ Target,
+
+ /// This materialization materializes a conversion from a legal type back to
+ /// an illegal one.
+ Source
};
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Simplify handling of dropped arguments (PR #97213)
https://github.com/matthias-springer ready_for_review https://github.com/llvm/llvm-project/pull/97213 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Dialect Conversion: Move argument materialization logic (PR #98805)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/98805
This commit moves the argument materialization logic from
`legalizeConvertedArgumentTypes` to `legalizeUnresolvedMaterializations`.
Before this change:
- Argument materializations were created in `legalizeConvertedArgumentTypes`
(which used to call `materializeLiveConversions`).
After this change:
- `legalizeConvertedArgumentTypes` creates a "placeholder"
`unrealized_conversion_cast`.
- The placeholder `unrealized_conversion_cast` is replaced with an argument
materialization (using the type converter) in
`legalizeUnresolvedMaterializations`.
- All argument and target materializations now take place in the same location
(`legalizeUnresolvedMaterializations`).
This commit brings us closer towards creating all source/target/argument
materializations in one central step, which can then be made optional (and
delegated to the user) in the future. (There is one more source materialization
step that has not been moved yet.)
This commit also consolidates all `build*UnresolvedMaterialization` functions
into a single `buildUnresolvedMaterialization` function.
This is a re-upload of #96329.
>From dac43d6ee0b2231aa64d49650f9b052ced49d3e4 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Thu, 20 Jun 2024 17:40:07 +0200
Subject: [PATCH] [mlir][Transforms][NFC] Dialect Conversion: Move argument
materialization logic
This commit moves the argument materialization logic from
`legalizeConvertedArgumentTypes` to `legalizeUnresolvedMaterializations`.
Before this change:
- Argument materializations were created in `legalizeConvertedArgumentTypes`
(which used to call `materializeLiveConversions`).
After this change:
- `legalizeConvertedArgumentTypes` creates a "placeholder"
`unrealized_conversion_cast`.
- The placeholder `unrealized_conversion_cast` is replaced with an argument
materialization (using the type converter) in
`legalizeUnresolvedMaterializations`.
- All argument and target materializations now take place in the same location
(`legalizeUnresolvedMaterializations`).
This commit brings us closer towards creating all source/target/argument
materializations in one central step, which can then be made optional (and
delegated to the user) in the future. (There is one more source materialization
step that has not been moved yet.)
This commit also consolidates all `build*UnresolvedMaterialization` functions
into a single `buildUnresolvedMaterialization` function.
This is a re-upload of #96329.
---
.../Transforms/Utils/DialectConversion.cpp| 138 +++---
1 file changed, 54 insertions(+), 84 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 0b552a7e1ca3b..a2e7570380351 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -53,6 +53,16 @@ static void logFailure(llvm::ScopedPrinter &os, StringRef
fmt, Args &&...args) {
});
}
+/// Helper function that computes an insertion point where the given value is
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(Value value) {
+ Block *insertBlock = value.getParentBlock();
+ Block::iterator insertPt = insertBlock->begin();
+ if (OpResult inputRes = dyn_cast(value))
+insertPt = ++inputRes.getOwner()->getIterator();
+ return OpBuilder::InsertPoint(insertBlock, insertPt);
+}
+
//===--===//
// ConversionValueMapping
//===--===//
@@ -445,11 +455,9 @@ class BlockTypeConversionRewrite : public BlockRewrite {
return rewrite->getKind() == Kind::BlockTypeConversion;
}
- /// Materialize any necessary conversions for converted arguments that have
- /// live users, using the provided `findLiveUser` to search for a user that
- /// survives the conversion process.
- LogicalResult
- materializeLiveConversions(function_ref findLiveUser);
+ Block *getOrigBlock() const { return origBlock; }
+
+ const TypeConverter *getConverter() const { return converter; }
void commit(RewriterBase &rewriter) override;
@@ -830,15 +838,10 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
/// Build an unresolved materialization operation given an output type and
set
/// of input operands.
Value buildUnresolvedMaterialization(MaterializationKind kind,
- Block *insertBlock,
- Block::iterator insertPt, Location loc,
+ OpBuilder::InsertPoint ip, Location loc,
ValueRange inputs, Type outputType,
const TypeConverter *converter);
- Value buildUnresolvedTargetMaterialization(Locat
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Dialect conversion: Eagerly build reverse mapping (PR #101476)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/101476 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Build unresolved materialization for replaced ops (PR #101514)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/101514
When inserting an argument/source/target materialization, the dialect
conversion framework first inserts a "dummy" `unrealized_conversion_cast` op
(during the rewrite process) and then (in the "finialize" phase) replaces these
cast ops with the IR generated by the type converter callback.
This is the case for all materializations, except when ops are being replaced
with values that have a different type. In that case, the dialect conversion
currently directly emits a source materialization. This commit changes the
implementation, such that a temporary `unrealized_conversion_cast` is also
inserted in that case.
This commit simplifies the code base: all materializations now happen in
`legalizeUnresolvedMaterialization`. This commit makes it possible to decouple
source/target/argument materializations from the dialect conversion (to reduce
the complexity of the code base). Such materializations can then also be
optional. This will be implemented in a follow-up commit.
Depends on #101476.
>From 6f771c12fdc1459da867a89bf89c32a38251a4a1 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Thu, 1 Aug 2024 18:24:56 +0200
Subject: [PATCH] [mlir][Transforms] Dialect conversion: Build unresolved
materialization for replaced ops
When inserting an argument/source/target materialization, the dialect
conversion framework first inserts a "dummy" `unrealized_conversion_cast` op
(during the rewrite process) and then (in the "finialize" phase) replaces these
cast ops with the IR generated by the type converter callback.
This is the case for all materializations, except when ops are being replaced
with values that have a different type. In that case, the dialect conversion
currently directly emits a source materialization. This commit changes the
implementation, such that a temporary `unrealized_conversion_cast` is also
inserted in this case.
This commit simplifies the code base: all materializations now happen in
`legalizeUnresolvedMaterialization`. This commit makes it possible to decouple
source/target/argument materializations from the dialect conversion (to reduce
the complexity of the code base). Such materializations can then also be
optional. This will be implemented in a follow-up commit.
---
.../Transforms/Utils/DialectConversion.cpp| 126 +++---
.../VectorToSPIRV/vector-to-spirv.mlir| 4 +-
.../Transforms/finalizing-bufferize.mlir | 3 +-
.../test-legalize-type-conversion.mlir| 11 +-
4 files changed, 57 insertions(+), 87 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 8f9b21b7ee1e5..b0b5a8247b53f 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -2348,6 +2348,12 @@ struct OperationConverter {
legalizeConvertedArgumentTypes(ConversionPatternRewriter &rewriter,
ConversionPatternRewriterImpl &rewriterImpl);
+ /// Legalize the types of converted op results.
+ LogicalResult legalizeConvertedOpResultTypes(
+ ConversionPatternRewriter &rewriter,
+ ConversionPatternRewriterImpl &rewriterImpl,
+ DenseMap> &inverseMapping);
+
/// Legalize any unresolved type materializations.
LogicalResult legalizeUnresolvedMaterializations(
ConversionPatternRewriter &rewriter,
@@ -2359,14 +2365,6 @@ struct OperationConverter {
legalizeErasedResult(Operation *op, OpResult result,
ConversionPatternRewriterImpl &rewriterImpl);
- /// Legalize an operation result that was replaced with a value of a
different
- /// type.
- LogicalResult legalizeChangedResultType(
- Operation *op, OpResult result, Value newValue,
- const TypeConverter *replConverter, ConversionPatternRewriter &rewriter,
- ConversionPatternRewriterImpl &rewriterImpl,
- const DenseMap> &inverseMapping);
-
/// Dialect conversion configuration.
ConversionConfig config;
@@ -2459,10 +2457,42 @@ OperationConverter::finalize(ConversionPatternRewriter
&rewriter) {
return failure();
DenseMap> inverseMapping =
rewriterImpl.mapping.getInverse();
+ if (failed(legalizeConvertedOpResultTypes(rewriter, rewriterImpl,
+inverseMapping)))
+return failure();
if (failed(legalizeUnresolvedMaterializations(rewriter, rewriterImpl,
inverseMapping)))
return failure();
+ return success();
+}
+/// Finds a user of the given value, or of any other value that the given value
+/// replaced, that was not replaced in the conversion process.
+static Operation *findLiveUserOfReplaced(
+Value initialValue, ConversionPatternRewriterImpl &rewriterImpl,
+const DenseMap> &inverseMapping) {
+ SmallVector worklist(1, initialValue);
+ while (!w
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Build unresolved materialization for replaced ops (PR #101514)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/101514
>From 2d9d1281db99061b212259a60a8bffdfdc800447 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Thu, 1 Aug 2024 18:24:56 +0200
Subject: [PATCH] [mlir][Transforms] Dialect conversion: Build unresolved
materialization for replaced ops
When inserting an argument/source/target materialization, the dialect
conversion framework first inserts a "dummy" `unrealized_conversion_cast` op
(during the rewrite process) and then (in the "finialize" phase) replaces these
cast ops with the IR generated by the type converter callback.
This is the case for all materializations, except when ops are being replaced
with values that have a different type. In that case, the dialect conversion
currently directly emits a source materialization. This commit changes the
implementation, such that a temporary `unrealized_conversion_cast` is also
inserted in this case.
This commit simplifies the code base: all materializations now happen in
`legalizeUnresolvedMaterialization`. This commit makes it possible to decouple
source/target/argument materializations from the dialect conversion (to reduce
the complexity of the code base). Such materializations can then also be
optional. This will be implemented in a follow-up commit.
---
.../Transforms/Utils/DialectConversion.cpp| 126 +++---
.../VectorToSPIRV/vector-to-spirv.mlir| 4 +-
.../Transforms/finalizing-bufferize.mlir | 3 +-
.../test-legalize-type-conversion.mlir| 11 +-
4 files changed, 57 insertions(+), 87 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 8f9b21b7ee1e5..b0b5a8247b53f 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -2348,6 +2348,12 @@ struct OperationConverter {
legalizeConvertedArgumentTypes(ConversionPatternRewriter &rewriter,
ConversionPatternRewriterImpl &rewriterImpl);
+ /// Legalize the types of converted op results.
+ LogicalResult legalizeConvertedOpResultTypes(
+ ConversionPatternRewriter &rewriter,
+ ConversionPatternRewriterImpl &rewriterImpl,
+ DenseMap> &inverseMapping);
+
/// Legalize any unresolved type materializations.
LogicalResult legalizeUnresolvedMaterializations(
ConversionPatternRewriter &rewriter,
@@ -2359,14 +2365,6 @@ struct OperationConverter {
legalizeErasedResult(Operation *op, OpResult result,
ConversionPatternRewriterImpl &rewriterImpl);
- /// Legalize an operation result that was replaced with a value of a
different
- /// type.
- LogicalResult legalizeChangedResultType(
- Operation *op, OpResult result, Value newValue,
- const TypeConverter *replConverter, ConversionPatternRewriter &rewriter,
- ConversionPatternRewriterImpl &rewriterImpl,
- const DenseMap> &inverseMapping);
-
/// Dialect conversion configuration.
ConversionConfig config;
@@ -2459,10 +2457,42 @@ OperationConverter::finalize(ConversionPatternRewriter
&rewriter) {
return failure();
DenseMap> inverseMapping =
rewriterImpl.mapping.getInverse();
+ if (failed(legalizeConvertedOpResultTypes(rewriter, rewriterImpl,
+inverseMapping)))
+return failure();
if (failed(legalizeUnresolvedMaterializations(rewriter, rewriterImpl,
inverseMapping)))
return failure();
+ return success();
+}
+/// Finds a user of the given value, or of any other value that the given value
+/// replaced, that was not replaced in the conversion process.
+static Operation *findLiveUserOfReplaced(
+Value initialValue, ConversionPatternRewriterImpl &rewriterImpl,
+const DenseMap> &inverseMapping) {
+ SmallVector worklist(1, initialValue);
+ while (!worklist.empty()) {
+Value value = worklist.pop_back_val();
+
+// Walk the users of this value to see if there are any live users that
+// weren't replaced during conversion.
+auto liveUserIt = llvm::find_if_not(value.getUsers(), [&](Operation *user)
{
+ return rewriterImpl.isOpIgnored(user);
+});
+if (liveUserIt != value.user_end())
+ return *liveUserIt;
+auto mapIt = inverseMapping.find(value);
+if (mapIt != inverseMapping.end())
+ worklist.append(mapIt->second);
+ }
+ return nullptr;
+}
+
+LogicalResult OperationConverter::legalizeConvertedOpResultTypes(
+ConversionPatternRewriter &rewriter,
+ConversionPatternRewriterImpl &rewriterImpl,
+DenseMap> &inverseMapping) {
// Process requested operation replacements.
for (unsigned i = 0; i < rewriterImpl.rewrites.size(); ++i) {
auto *opReplacement =
@@ -2485,14 +2515,21 @@ OperationConverter::finalize(ConversionPatternRewriter
&rewriter) {
if (result.getType() == newValue.getT
[llvm-branch-commits] [mlir] [mlir][IR] Auto-generate element type verification for VectorType (PR #102449)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/102449
#102326 enables verification of type parameters that are type constraints. The
element type verification for `VectorType` (and maybe other builtin types in
the future) can now be auto-generated.
Also remove redundant error checking in the vector type parser: element type
and dimensions are already checked by the verifier (which is called from
`getChecked`).
Depends on #102326.
>From 73980b05339b57ff770d3b9aa5e82a569d84f91d Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Thu, 8 Aug 2024 12:28:23 +0200
Subject: [PATCH] [mlir][IR] Auto-generate element type verification for
VectorType
---
mlir/include/mlir/IR/BuiltinTypes.td| 4 +++-
mlir/lib/AsmParser/TypeParser.cpp | 13 +++--
mlir/test/IR/invalid-builtin-types.mlir | 6 +++---
3 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/mlir/include/mlir/IR/BuiltinTypes.td
b/mlir/include/mlir/IR/BuiltinTypes.td
index 365edcf68d8b94..4b3add2035263c 100644
--- a/mlir/include/mlir/IR/BuiltinTypes.td
+++ b/mlir/include/mlir/IR/BuiltinTypes.td
@@ -17,6 +17,7 @@
include "mlir/IR/AttrTypeBase.td"
include "mlir/IR/BuiltinDialect.td"
include "mlir/IR/BuiltinTypeInterfaces.td"
+include "mlir/IR/CommonTypeConstraints.td"
// TODO: Currently the types defined in this file are prefixed with `Builtin_`.
// This is to differentiate the types here with the ones in OpBase.td. We
should
@@ -1146,7 +1147,7 @@ def Builtin_Vector : Builtin_Type<"Vector", "vector",
}];
let parameters = (ins
ArrayRefParameter<"int64_t">:$shape,
-"Type":$elementType,
+AnyTypeOf<[AnyInteger, Index, AnyFloat]>:$elementType,
ArrayRefParameter<"bool">:$scalableDims
);
let builders = [
@@ -1173,6 +1174,7 @@ def Builtin_Vector : Builtin_Type<"Vector", "vector",
/// type. In particular, vectors can consist of integer, index, or float
/// primitives.
static bool isValidElementType(Type t) {
+ // TODO: Auto-generate this function from $elementType.
return ::llvm::isa(t);
}
diff --git a/mlir/lib/AsmParser/TypeParser.cpp
b/mlir/lib/AsmParser/TypeParser.cpp
index 542eaeefe57f12..f070c072c43296 100644
--- a/mlir/lib/AsmParser/TypeParser.cpp
+++ b/mlir/lib/AsmParser/TypeParser.cpp
@@ -458,31 +458,24 @@ Type Parser::parseTupleType() {
/// static-dim-list ::= decimal-literal (`x` decimal-literal)*
///
VectorType Parser::parseVectorType() {
+ SMLoc loc = getToken().getLoc();
consumeToken(Token::kw_vector);
if (parseToken(Token::less, "expected '<' in vector type"))
return nullptr;
+ // Parse the dimensions.
SmallVector dimensions;
SmallVector scalableDims;
if (parseVectorDimensionList(dimensions, scalableDims))
return nullptr;
- if (any_of(dimensions, [](int64_t i) { return i <= 0; }))
-return emitError(getToken().getLoc(),
- "vector types must have positive constant sizes"),
- nullptr;
// Parse the element type.
- auto typeLoc = getToken().getLoc();
auto elementType = parseType();
if (!elementType || parseToken(Token::greater, "expected '>' in vector
type"))
return nullptr;
- if (!VectorType::isValidElementType(elementType))
-return emitError(typeLoc, "vector elements must be int/index/float type"),
- nullptr;
-
- return VectorType::get(dimensions, elementType, scalableDims);
+ return getChecked(loc, dimensions, elementType, scalableDims);
}
/// Parse a dimension list in a vector type. This populates the dimension list.
diff --git a/mlir/test/IR/invalid-builtin-types.mlir
b/mlir/test/IR/invalid-builtin-types.mlir
index 9884212e916c1f..07854a25000feb 100644
--- a/mlir/test/IR/invalid-builtin-types.mlir
+++ b/mlir/test/IR/invalid-builtin-types.mlir
@@ -120,17 +120,17 @@ func.func @illegaltype(i21312312323120) // expected-error
{{invalid integer widt
// -
// Test no nested vector.
-// expected-error@+1 {{vector elements must be int/index/float type}}
+// expected-error@+1 {{failed to verify 'elementType': integer or index or
floating-point}}
func.func @vectors(vector<1 x vector<1xi32>>, vector<2x4xf32>)
// -
-// expected-error @+1 {{vector types must have positive constant sizes}}
+// expected-error @+1 {{vector types must have positive constant sizes but got
0}}
func.func @zero_vector_type() -> vector<0xi32>
// -
-// expected-error @+1 {{vector types must have positive constant sizes}}
+// expected-error @+1 {{vector types must have positive constant sizes but got
1, 0}}
func.func @zero_in_vector_type() -> vector<1x0xi32>
// -
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][IR] Auto-generate element type verification for VectorType (PR #102449)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/102449
>From 1e3c58ae8903d58c2a10c634e859c8f1e4df9812 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Thu, 8 Aug 2024 12:28:23 +0200
Subject: [PATCH] [mlir][IR] Auto-generate element type verification for
VectorType
---
mlir/include/mlir/IR/BuiltinTypes.td| 4 +++-
mlir/lib/AsmParser/TypeParser.cpp | 13 +++--
mlir/test/IR/invalid-builtin-types.mlir | 6 +++---
mlir/test/python/ir/builtin_types.py| 2 +-
4 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/mlir/include/mlir/IR/BuiltinTypes.td
b/mlir/include/mlir/IR/BuiltinTypes.td
index 365edcf68d8b9..4b3add2035263 100644
--- a/mlir/include/mlir/IR/BuiltinTypes.td
+++ b/mlir/include/mlir/IR/BuiltinTypes.td
@@ -17,6 +17,7 @@
include "mlir/IR/AttrTypeBase.td"
include "mlir/IR/BuiltinDialect.td"
include "mlir/IR/BuiltinTypeInterfaces.td"
+include "mlir/IR/CommonTypeConstraints.td"
// TODO: Currently the types defined in this file are prefixed with `Builtin_`.
// This is to differentiate the types here with the ones in OpBase.td. We
should
@@ -1146,7 +1147,7 @@ def Builtin_Vector : Builtin_Type<"Vector", "vector",
}];
let parameters = (ins
ArrayRefParameter<"int64_t">:$shape,
-"Type":$elementType,
+AnyTypeOf<[AnyInteger, Index, AnyFloat]>:$elementType,
ArrayRefParameter<"bool">:$scalableDims
);
let builders = [
@@ -1173,6 +1174,7 @@ def Builtin_Vector : Builtin_Type<"Vector", "vector",
/// type. In particular, vectors can consist of integer, index, or float
/// primitives.
static bool isValidElementType(Type t) {
+ // TODO: Auto-generate this function from $elementType.
return ::llvm::isa(t);
}
diff --git a/mlir/lib/AsmParser/TypeParser.cpp
b/mlir/lib/AsmParser/TypeParser.cpp
index 542eaeefe57f1..f070c072c4329 100644
--- a/mlir/lib/AsmParser/TypeParser.cpp
+++ b/mlir/lib/AsmParser/TypeParser.cpp
@@ -458,31 +458,24 @@ Type Parser::parseTupleType() {
/// static-dim-list ::= decimal-literal (`x` decimal-literal)*
///
VectorType Parser::parseVectorType() {
+ SMLoc loc = getToken().getLoc();
consumeToken(Token::kw_vector);
if (parseToken(Token::less, "expected '<' in vector type"))
return nullptr;
+ // Parse the dimensions.
SmallVector dimensions;
SmallVector scalableDims;
if (parseVectorDimensionList(dimensions, scalableDims))
return nullptr;
- if (any_of(dimensions, [](int64_t i) { return i <= 0; }))
-return emitError(getToken().getLoc(),
- "vector types must have positive constant sizes"),
- nullptr;
// Parse the element type.
- auto typeLoc = getToken().getLoc();
auto elementType = parseType();
if (!elementType || parseToken(Token::greater, "expected '>' in vector
type"))
return nullptr;
- if (!VectorType::isValidElementType(elementType))
-return emitError(typeLoc, "vector elements must be int/index/float type"),
- nullptr;
-
- return VectorType::get(dimensions, elementType, scalableDims);
+ return getChecked(loc, dimensions, elementType, scalableDims);
}
/// Parse a dimension list in a vector type. This populates the dimension list.
diff --git a/mlir/test/IR/invalid-builtin-types.mlir
b/mlir/test/IR/invalid-builtin-types.mlir
index 9884212e916c1..07854a25000fe 100644
--- a/mlir/test/IR/invalid-builtin-types.mlir
+++ b/mlir/test/IR/invalid-builtin-types.mlir
@@ -120,17 +120,17 @@ func.func @illegaltype(i21312312323120) // expected-error
{{invalid integer widt
// -
// Test no nested vector.
-// expected-error@+1 {{vector elements must be int/index/float type}}
+// expected-error@+1 {{failed to verify 'elementType': integer or index or
floating-point}}
func.func @vectors(vector<1 x vector<1xi32>>, vector<2x4xf32>)
// -
-// expected-error @+1 {{vector types must have positive constant sizes}}
+// expected-error @+1 {{vector types must have positive constant sizes but got
0}}
func.func @zero_vector_type() -> vector<0xi32>
// -
-// expected-error @+1 {{vector types must have positive constant sizes}}
+// expected-error @+1 {{vector types must have positive constant sizes but got
1, 0}}
func.func @zero_in_vector_type() -> vector<1x0xi32>
// -
diff --git a/mlir/test/python/ir/builtin_types.py
b/mlir/test/python/ir/builtin_types.py
index 2161f110ac31e..f9554105e 100644
--- a/mlir/test/python/ir/builtin_types.py
+++ b/mlir/test/python/ir/builtin_types.py
@@ -345,7 +345,7 @@ def testVectorType():
VectorType.get(shape, none)
except MLIRError as e:
# CHECK: Invalid type:
-# CHECK: error: unknown: vector elements must be int/index/float
type but got 'none'
+# CHECK: error: unknown: failed to verify 'elementType': integer
or index or floating-point
print(e)
else:
print("Exception not produced")
[llvm-branch-commits] [mlir] [mlir][ODS] Verify type constraints in Types and Attributes (PR #102326)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/102326 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][ODS] Verify type constraints in Types and Attributes (PR #102326)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/102326 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][ODS] Verify type constraints in Types and Attributes (PR #102326)
@@ -153,6 +153,9 @@ class TypeConstraint {
// The name of the C++ Type class if known, or Type if not.
string cppClassName = cppClassNameParam;
+ // TODO: This field is sometimes called `cppClassName` and sometimes
matthias-springer wrote:
Fixed in #102657.
https://github.com/llvm/llvm-project/pull/102326
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][IR] Auto-generate element type verification for VectorType (PR #102449)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/102449
>From c399a1c6e82afcfcc2ad531cb49d53683f294f91 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Thu, 8 Aug 2024 12:28:23 +0200
Subject: [PATCH] [mlir][IR] Auto-generate element type verification for
VectorType
---
mlir/include/mlir/IR/BuiltinTypes.td| 4 +++-
mlir/lib/AsmParser/TypeParser.cpp | 13 +++--
mlir/test/IR/invalid-builtin-types.mlir | 6 +++---
mlir/test/python/ir/builtin_types.py| 2 +-
4 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/mlir/include/mlir/IR/BuiltinTypes.td
b/mlir/include/mlir/IR/BuiltinTypes.td
index 365edcf68d8b94..4b3add2035263c 100644
--- a/mlir/include/mlir/IR/BuiltinTypes.td
+++ b/mlir/include/mlir/IR/BuiltinTypes.td
@@ -17,6 +17,7 @@
include "mlir/IR/AttrTypeBase.td"
include "mlir/IR/BuiltinDialect.td"
include "mlir/IR/BuiltinTypeInterfaces.td"
+include "mlir/IR/CommonTypeConstraints.td"
// TODO: Currently the types defined in this file are prefixed with `Builtin_`.
// This is to differentiate the types here with the ones in OpBase.td. We
should
@@ -1146,7 +1147,7 @@ def Builtin_Vector : Builtin_Type<"Vector", "vector",
}];
let parameters = (ins
ArrayRefParameter<"int64_t">:$shape,
-"Type":$elementType,
+AnyTypeOf<[AnyInteger, Index, AnyFloat]>:$elementType,
ArrayRefParameter<"bool">:$scalableDims
);
let builders = [
@@ -1173,6 +1174,7 @@ def Builtin_Vector : Builtin_Type<"Vector", "vector",
/// type. In particular, vectors can consist of integer, index, or float
/// primitives.
static bool isValidElementType(Type t) {
+ // TODO: Auto-generate this function from $elementType.
return ::llvm::isa(t);
}
diff --git a/mlir/lib/AsmParser/TypeParser.cpp
b/mlir/lib/AsmParser/TypeParser.cpp
index 542eaeefe57f12..f070c072c43296 100644
--- a/mlir/lib/AsmParser/TypeParser.cpp
+++ b/mlir/lib/AsmParser/TypeParser.cpp
@@ -458,31 +458,24 @@ Type Parser::parseTupleType() {
/// static-dim-list ::= decimal-literal (`x` decimal-literal)*
///
VectorType Parser::parseVectorType() {
+ SMLoc loc = getToken().getLoc();
consumeToken(Token::kw_vector);
if (parseToken(Token::less, "expected '<' in vector type"))
return nullptr;
+ // Parse the dimensions.
SmallVector dimensions;
SmallVector scalableDims;
if (parseVectorDimensionList(dimensions, scalableDims))
return nullptr;
- if (any_of(dimensions, [](int64_t i) { return i <= 0; }))
-return emitError(getToken().getLoc(),
- "vector types must have positive constant sizes"),
- nullptr;
// Parse the element type.
- auto typeLoc = getToken().getLoc();
auto elementType = parseType();
if (!elementType || parseToken(Token::greater, "expected '>' in vector
type"))
return nullptr;
- if (!VectorType::isValidElementType(elementType))
-return emitError(typeLoc, "vector elements must be int/index/float type"),
- nullptr;
-
- return VectorType::get(dimensions, elementType, scalableDims);
+ return getChecked(loc, dimensions, elementType, scalableDims);
}
/// Parse a dimension list in a vector type. This populates the dimension list.
diff --git a/mlir/test/IR/invalid-builtin-types.mlir
b/mlir/test/IR/invalid-builtin-types.mlir
index 9884212e916c1f..07854a25000feb 100644
--- a/mlir/test/IR/invalid-builtin-types.mlir
+++ b/mlir/test/IR/invalid-builtin-types.mlir
@@ -120,17 +120,17 @@ func.func @illegaltype(i21312312323120) // expected-error
{{invalid integer widt
// -
// Test no nested vector.
-// expected-error@+1 {{vector elements must be int/index/float type}}
+// expected-error@+1 {{failed to verify 'elementType': integer or index or
floating-point}}
func.func @vectors(vector<1 x vector<1xi32>>, vector<2x4xf32>)
// -
-// expected-error @+1 {{vector types must have positive constant sizes}}
+// expected-error @+1 {{vector types must have positive constant sizes but got
0}}
func.func @zero_vector_type() -> vector<0xi32>
// -
-// expected-error @+1 {{vector types must have positive constant sizes}}
+// expected-error @+1 {{vector types must have positive constant sizes but got
1, 0}}
func.func @zero_in_vector_type() -> vector<1x0xi32>
// -
diff --git a/mlir/test/python/ir/builtin_types.py
b/mlir/test/python/ir/builtin_types.py
index 2161f110ac31e2..f9554105ed 100644
--- a/mlir/test/python/ir/builtin_types.py
+++ b/mlir/test/python/ir/builtin_types.py
@@ -345,7 +345,7 @@ def testVectorType():
VectorType.get(shape, none)
except MLIRError as e:
# CHECK: Invalid type:
-# CHECK: error: unknown: vector elements must be int/index/float
type but got 'none'
+# CHECK: error: unknown: failed to verify 'elementType': integer
or index or floating-point
print(e)
else:
print("Exception not produced")
[llvm-branch-commits] [mlir] [mlir][Transforms][WIP] Dialect conversion: Make materializations optional (PR #104668)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/104668 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][WIP] Dialect conversion: Make materializations optional (PR #104668)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/104668
>From ca2db3fa01051a6e9b74e22efc3adbbff585f8f4 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Sat, 17 Aug 2024 11:38:40 +0200
Subject: [PATCH] [mlir][Transforms][WIP] Dialect conversion: Make
materializations optional
Build all source/target/argument materializations after the conversion has
succeeded. Provide a new configuration option for users to opt out of all
automatic materializations. In that case, the resulting IR will have
`builtin.unrealized_conversion_cast` ops.
---
.../Transforms/Utils/DialectConversion.cpp| 382 --
.../Conversion/NVGPUToNVVM/nvgpu-to-nvvm.mlir | 5 +-
.../Transforms/finalizing-bufferize.mlir | 1 +
.../test-legalize-type-conversion.mlir| 6 +-
4 files changed, 94 insertions(+), 300 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 6238a257b2ffda..1f9b78963403a8 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -702,14 +702,8 @@ class UnresolvedMaterializationRewrite : public
OperationRewrite {
return rewrite->getKind() == Kind::UnresolvedMaterialization;
}
- UnrealizedConversionCastOp getOperation() const {
-return cast(op);
- }
-
void rollback() override;
- void cleanup(RewriterBase &rewriter) override;
-
/// Return the type converter of this materialization (which may be null).
const TypeConverter *getConverter() const {
return converterAndKind.getPointer();
@@ -766,7 +760,7 @@ namespace detail {
struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
explicit ConversionPatternRewriterImpl(MLIRContext *ctx,
const ConversionConfig &config)
- : context(ctx), config(config) {}
+ : context(ctx), eraseRewriter(ctx), config(config) {}
//======//
// State Management
@@ -834,6 +828,7 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
//======//
// Materializations
//======//
+
/// Build an unresolved materialization operation given an output type and
set
/// of input operands.
Value buildUnresolvedMaterialization(MaterializationKind kind,
@@ -877,8 +872,8 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
/// no new IR is created between calls to `eraseOp`/`eraseBlock`.
struct SingleEraseRewriter : public RewriterBase, RewriterBase::Listener {
public:
-SingleEraseRewriter(MLIRContext *context)
-: RewriterBase(context, /*listener=*/this) {}
+SingleEraseRewriter(MLIRContext *ctx)
+: RewriterBase(ctx, /*listener=*/this) {}
/// Erase the given op (unless it was already erased).
void eraseOp(Operation *op) override {
@@ -912,6 +907,8 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
/// MLIR context.
MLIRContext *context;
+ SingleEraseRewriter eraseRewriter;
+
// Mapping between replaced values that differ in type. This happens when
// replacing a value with one of a different type.
ConversionValueMapping mapping;
@@ -1058,10 +1055,6 @@ void UnresolvedMaterializationRewrite::rollback() {
op->erase();
}
-void UnresolvedMaterializationRewrite::cleanup(RewriterBase &rewriter) {
- rewriter.eraseOp(op);
-}
-
void ConversionPatternRewriterImpl::applyRewrites() {
// Commit all rewrites.
IRRewriter rewriter(context, config.listener);
@@ -1069,7 +1062,6 @@ void ConversionPatternRewriterImpl::applyRewrites() {
rewrite->commit(rewriter);
// Clean up all rewrites.
- SingleEraseRewriter eraseRewriter(context);
for (auto &rewrite : rewrites)
rewrite->cleanup(eraseRewriter);
}
@@ -2354,12 +2346,6 @@ struct OperationConverter {
ConversionPatternRewriterImpl &rewriterImpl,
DenseMap> &inverseMapping);
- /// Legalize any unresolved type materializations.
- LogicalResult legalizeUnresolvedMaterializations(
- ConversionPatternRewriter &rewriter,
- ConversionPatternRewriterImpl &rewriterImpl,
- DenseMap> &inverseMapping);
-
/// Legalize an operation result that was marked as "erased".
LogicalResult
legalizeErasedResult(Operation *op, OpResult result,
@@ -2406,6 +2392,57 @@ LogicalResult
OperationConverter::convert(ConversionPatternRewriter &rewriter,
return success();
}
+static LogicalResult
+legalizeUnresolvedMaterialization(RewriterBase &rewriter,
+ UnresolvedMaterializationRewrite *rewrite) {
+ UnrealizedConversionCastOp op =
+ cast(rewrite->getOperation());
+ assert(!op.use_empty() &&
+ "expected that dead materiali
[llvm-branch-commits] [mlir] [mlir][Transforms][WIP] Dialect conversion: Make materializations optional (PR #104668)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/104668
>From 26c00f29b320e95f314372cae557cc0294796863 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Sat, 17 Aug 2024 11:38:40 +0200
Subject: [PATCH] [mlir][Transforms][WIP] Dialect conversion: Make
materializations optional
Build all source/target/argument materializations after the conversion has
succeeded. Provide a new configuration option for users to opt out of all
automatic materializations. In that case, the resulting IR will have
`builtin.unrealized_conversion_cast` ops.
---
.../Transforms/Utils/DialectConversion.cpp| 382 --
.../Conversion/NVGPUToNVVM/nvgpu-to-nvvm.mlir | 5 +-
.../Transforms/finalizing-bufferize.mlir | 1 +
.../test-legalize-type-conversion.mlir| 6 +-
4 files changed, 94 insertions(+), 300 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 6238a257b2ffda..1f9b78963403a8 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -702,14 +702,8 @@ class UnresolvedMaterializationRewrite : public
OperationRewrite {
return rewrite->getKind() == Kind::UnresolvedMaterialization;
}
- UnrealizedConversionCastOp getOperation() const {
-return cast(op);
- }
-
void rollback() override;
- void cleanup(RewriterBase &rewriter) override;
-
/// Return the type converter of this materialization (which may be null).
const TypeConverter *getConverter() const {
return converterAndKind.getPointer();
@@ -766,7 +760,7 @@ namespace detail {
struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
explicit ConversionPatternRewriterImpl(MLIRContext *ctx,
const ConversionConfig &config)
- : context(ctx), config(config) {}
+ : context(ctx), eraseRewriter(ctx), config(config) {}
//======//
// State Management
@@ -834,6 +828,7 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
//======//
// Materializations
//======//
+
/// Build an unresolved materialization operation given an output type and
set
/// of input operands.
Value buildUnresolvedMaterialization(MaterializationKind kind,
@@ -877,8 +872,8 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
/// no new IR is created between calls to `eraseOp`/`eraseBlock`.
struct SingleEraseRewriter : public RewriterBase, RewriterBase::Listener {
public:
-SingleEraseRewriter(MLIRContext *context)
-: RewriterBase(context, /*listener=*/this) {}
+SingleEraseRewriter(MLIRContext *ctx)
+: RewriterBase(ctx, /*listener=*/this) {}
/// Erase the given op (unless it was already erased).
void eraseOp(Operation *op) override {
@@ -912,6 +907,8 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
/// MLIR context.
MLIRContext *context;
+ SingleEraseRewriter eraseRewriter;
+
// Mapping between replaced values that differ in type. This happens when
// replacing a value with one of a different type.
ConversionValueMapping mapping;
@@ -1058,10 +1055,6 @@ void UnresolvedMaterializationRewrite::rollback() {
op->erase();
}
-void UnresolvedMaterializationRewrite::cleanup(RewriterBase &rewriter) {
- rewriter.eraseOp(op);
-}
-
void ConversionPatternRewriterImpl::applyRewrites() {
// Commit all rewrites.
IRRewriter rewriter(context, config.listener);
@@ -1069,7 +1062,6 @@ void ConversionPatternRewriterImpl::applyRewrites() {
rewrite->commit(rewriter);
// Clean up all rewrites.
- SingleEraseRewriter eraseRewriter(context);
for (auto &rewrite : rewrites)
rewrite->cleanup(eraseRewriter);
}
@@ -2354,12 +2346,6 @@ struct OperationConverter {
ConversionPatternRewriterImpl &rewriterImpl,
DenseMap> &inverseMapping);
- /// Legalize any unresolved type materializations.
- LogicalResult legalizeUnresolvedMaterializations(
- ConversionPatternRewriter &rewriter,
- ConversionPatternRewriterImpl &rewriterImpl,
- DenseMap> &inverseMapping);
-
/// Legalize an operation result that was marked as "erased".
LogicalResult
legalizeErasedResult(Operation *op, OpResult result,
@@ -2406,6 +2392,57 @@ LogicalResult
OperationConverter::convert(ConversionPatternRewriter &rewriter,
return success();
}
+static LogicalResult
+legalizeUnresolvedMaterialization(RewriterBase &rewriter,
+ UnresolvedMaterializationRewrite *rewrite) {
+ UnrealizedConversionCastOp op =
+ cast(rewrite->getOperation());
+ assert(!op.use_empty() &&
+ "expected that dead materiali
[llvm-branch-commits] [mlir] [mlir][Transforms][WIP] Dialect conversion: Make materializations optional (PR #104668)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/104668
>From 2658d44d5c8aab38f39c34dc99a8e874c6794b75 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Sat, 17 Aug 2024 11:38:40 +0200
Subject: [PATCH] [mlir][Transforms][WIP] Dialect conversion: Make
materializations optional
Build all source/target/argument materializations after the conversion has
succeeded. Provide a new configuration option for users to opt out of all
automatic materializations. In that case, the resulting IR will have
`builtin.unrealized_conversion_cast` ops.
---
.../mlir/Transforms/DialectConversion.h | 11 +
.../Transforms/Utils/DialectConversion.cpp| 380 --
.../Conversion/NVGPUToNVVM/nvgpu-to-nvvm.mlir | 5 +-
.../Transforms/finalizing-bufferize.mlir | 1 +
.../test-legalize-type-conversion.mlir| 6 +-
5 files changed, 105 insertions(+), 298 deletions(-)
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index 60113bdef16a23..5f680e8eca7559 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -1124,6 +1124,17 @@ struct ConversionConfig {
// already been modified) and iterators into past IR state cannot be
// represented at the moment.
RewriterBase::Listener *listener = nullptr;
+
+ /// If set to "true", the dialect conversion attempts to build source/target/
+ /// argument materializations through the type converter API in lieu of
+ /// builtin.unrealized_conversion_cast ops. The conversion process fails if
+ /// at least one materialization could not be built.
+ ///
+ /// If set to "false", the dialect conversion does not does not build any
+ /// custom materializations and instead inserts
+ /// builtin.unrealized_conversion_cast ops to ensure that the resulting IR
+ /// is valid.
+ bool buildMaterializations = true;
};
//===--===//
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 6238a257b2ffda..dd6109c9fe6cfc 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -702,14 +702,8 @@ class UnresolvedMaterializationRewrite : public
OperationRewrite {
return rewrite->getKind() == Kind::UnresolvedMaterialization;
}
- UnrealizedConversionCastOp getOperation() const {
-return cast(op);
- }
-
void rollback() override;
- void cleanup(RewriterBase &rewriter) override;
-
/// Return the type converter of this materialization (which may be null).
const TypeConverter *getConverter() const {
return converterAndKind.getPointer();
@@ -766,7 +760,7 @@ namespace detail {
struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
explicit ConversionPatternRewriterImpl(MLIRContext *ctx,
const ConversionConfig &config)
- : context(ctx), config(config) {}
+ : context(ctx), eraseRewriter(ctx), config(config) {}
//======//
// State Management
@@ -834,6 +828,7 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
//======//
// Materializations
//======//
+
/// Build an unresolved materialization operation given an output type and
set
/// of input operands.
Value buildUnresolvedMaterialization(MaterializationKind kind,
@@ -912,6 +907,8 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
/// MLIR context.
MLIRContext *context;
+ SingleEraseRewriter eraseRewriter;
+
// Mapping between replaced values that differ in type. This happens when
// replacing a value with one of a different type.
ConversionValueMapping mapping;
@@ -1058,10 +1055,6 @@ void UnresolvedMaterializationRewrite::rollback() {
op->erase();
}
-void UnresolvedMaterializationRewrite::cleanup(RewriterBase &rewriter) {
- rewriter.eraseOp(op);
-}
-
void ConversionPatternRewriterImpl::applyRewrites() {
// Commit all rewrites.
IRRewriter rewriter(context, config.listener);
@@ -1069,7 +1062,6 @@ void ConversionPatternRewriterImpl::applyRewrites() {
rewrite->commit(rewriter);
// Clean up all rewrites.
- SingleEraseRewriter eraseRewriter(context);
for (auto &rewrite : rewrites)
rewrite->cleanup(eraseRewriter);
}
@@ -2354,12 +2346,6 @@ struct OperationConverter {
ConversionPatternRewriterImpl &rewriterImpl,
DenseMap> &inverseMapping);
- /// Legalize any unresolved type materializations.
- LogicalResult legalizeUnresolvedMaterializations(
- ConversionPatternRewriter &rewriter,
- ConversionPatternRewriterImp
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Make materializations optional (PR #104668)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/104668 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Make materializations optional (PR #104668)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/104668 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Make materializations optional (PR #104668)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/104668 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Make materializations optional (PR #104668)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/104668 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Make materializations optional (PR #104668)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/104668
>From f72f6427871531a135e6621c520b05570dfa3bcb Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Sat, 17 Aug 2024 11:38:40 +0200
Subject: [PATCH] [mlir][Transforms][WIP] Dialect conversion: Make
materializations optional
Build all source/target/argument materializations after the conversion has
succeeded. Provide a new configuration option for users to opt out of all
automatic materializations. In that case, the resulting IR will have
`builtin.unrealized_conversion_cast` ops.
---
.../mlir/Transforms/DialectConversion.h | 11 +
.../Transforms/Utils/DialectConversion.cpp| 383 --
.../Conversion/NVGPUToNVVM/nvgpu-to-nvvm.mlir | 5 +-
.../Transforms/finalizing-bufferize.mlir | 1 +
.../test-legalize-type-conversion.mlir| 6 +-
5 files changed, 108 insertions(+), 298 deletions(-)
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index 60113bdef16a23..5f680e8eca7559 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -1124,6 +1124,17 @@ struct ConversionConfig {
// already been modified) and iterators into past IR state cannot be
// represented at the moment.
RewriterBase::Listener *listener = nullptr;
+
+ /// If set to "true", the dialect conversion attempts to build source/target/
+ /// argument materializations through the type converter API in lieu of
+ /// builtin.unrealized_conversion_cast ops. The conversion process fails if
+ /// at least one materialization could not be built.
+ ///
+ /// If set to "false", the dialect conversion does not does not build any
+ /// custom materializations and instead inserts
+ /// builtin.unrealized_conversion_cast ops to ensure that the resulting IR
+ /// is valid.
+ bool buildMaterializations = true;
};
//===--===//
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 6238a257b2ffda..f710116186e741 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -702,14 +702,8 @@ class UnresolvedMaterializationRewrite : public
OperationRewrite {
return rewrite->getKind() == Kind::UnresolvedMaterialization;
}
- UnrealizedConversionCastOp getOperation() const {
-return cast(op);
- }
-
void rollback() override;
- void cleanup(RewriterBase &rewriter) override;
-
/// Return the type converter of this materialization (which may be null).
const TypeConverter *getConverter() const {
return converterAndKind.getPointer();
@@ -766,7 +760,7 @@ namespace detail {
struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
explicit ConversionPatternRewriterImpl(MLIRContext *ctx,
const ConversionConfig &config)
- : context(ctx), config(config) {}
+ : context(ctx), eraseRewriter(ctx), config(config) {}
//======//
// State Management
@@ -834,6 +828,7 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
//======//
// Materializations
//======//
+
/// Build an unresolved materialization operation given an output type and
set
/// of input operands.
Value buildUnresolvedMaterialization(MaterializationKind kind,
@@ -912,6 +907,11 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
/// MLIR context.
MLIRContext *context;
+ /// A rewriter that keeps track of ops/block that were already erased and
+ /// skips duplicate op/block erasures. This rewriter is used during the
+ /// "cleanup" phase.
+ SingleEraseRewriter eraseRewriter;
+
// Mapping between replaced values that differ in type. This happens when
// replacing a value with one of a different type.
ConversionValueMapping mapping;
@@ -1058,10 +1058,6 @@ void UnresolvedMaterializationRewrite::rollback() {
op->erase();
}
-void UnresolvedMaterializationRewrite::cleanup(RewriterBase &rewriter) {
- rewriter.eraseOp(op);
-}
-
void ConversionPatternRewriterImpl::applyRewrites() {
// Commit all rewrites.
IRRewriter rewriter(context, config.listener);
@@ -1069,7 +1065,6 @@ void ConversionPatternRewriterImpl::applyRewrites() {
rewrite->commit(rewriter);
// Clean up all rewrites.
- SingleEraseRewriter eraseRewriter(context);
for (auto &rewrite : rewrites)
rewrite->cleanup(eraseRewriter);
}
@@ -2354,12 +2349,6 @@ struct OperationConverter {
ConversionPatternRewriterImpl &rewriterImpl,
DenseMap> &inverseMapping);
- /// Le
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Make materializations optional (PR #104668)
https://github.com/matthias-springer ready_for_review https://github.com/llvm/llvm-project/pull/104668 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Modularize block actions (PR #81237)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/81237
Throughout the rewrite process, the dialect conversion maintains a list of
"block actions" that can be rolled back upon failure. This commit encapsulates
the existing block actions into separate classes, making it easier to add
additional actions in the future.
This commit also renames "block actions" to "rewrite actions". In a subsequent
commit, an "operation action" that allows rolling back movements of single
operations is added. This is to support `moveOpBefore` in the dialect
conversion.
Rewrite actions have two methods: `commit()` commits an action. It can no
longer be rolled back afterwards. `rollback()` undoes an action. It can no
longer be committed afterwards.
>From 956717ee48940279edfbe84794d6f9575c14d075 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Fri, 9 Feb 2024 09:29:10 +
Subject: [PATCH] [mlir][Transforms][NFC] Modularize block actions
Throughout the rewrite process, the dialect conversion maintains a list
of "block actions" that can be rolled back upon failure. This commit
encapsulates the existing block actions into separate classes, making it
easier to add additional actions in the future.
This commit also renames "block actions" to "rewrite actions". In a
subsequent commit, an "operation action" that allows rolling back
movements of single operations is added. This is to support
`moveOpBefore` in the dialect conversion.
Rewrite actions have two methods: `commit()` commits an action. It can
no longer be rolled back afterwards. `rollback()` undoes an action. It
can no longer be committed afterwards.
---
.../Transforms/Utils/DialectConversion.cpp| 466 +++---
1 file changed, 283 insertions(+), 183 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index e41231d7cbd390..44c107c8733f3d 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,13 +154,13 @@ namespace {
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
unsigned numReplacements, unsigned numArgReplacements,
-unsigned numBlockActions, unsigned numIgnoredOperations,
+unsigned numRewriteActions, unsigned numIgnoredOperations,
unsigned numRootUpdates)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
numReplacements(numReplacements),
numArgReplacements(numArgReplacements),
-numBlockActions(numBlockActions),
+numRewriteActions(numRewriteActions),
numIgnoredOperations(numIgnoredOperations),
numRootUpdates(numRootUpdates) {}
@@ -176,8 +176,8 @@ struct RewriterState {
/// The current number of argument replacements queued.
unsigned numArgReplacements;
- /// The current number of block actions performed.
- unsigned numBlockActions;
+ /// The current number of rewrite actions performed.
+ unsigned numRewriteActions;
/// The current number of ignored operations.
unsigned numIgnoredOperations;
@@ -235,86 +235,6 @@ struct OpReplacement {
const TypeConverter *converter;
};
-//===--===//
-// BlockAction
-
-/// The kind of the block action performed during the rewrite. Actions can be
-/// undone if the conversion fails.
-enum class BlockActionKind {
- Create,
- Erase,
- Inline,
- Move,
- Split,
- TypeConversion
-};
-
-/// Original position of the given block in its parent region. During undo
-/// actions, the block needs to be placed before `insertBeforeBlock`.
-struct BlockPosition {
- Region *region;
- Block *insertBeforeBlock;
-};
-
-/// Information needed to undo inlining actions.
-/// - the source block
-/// - the first inlined operation (could be null if the source block was empty)
-/// - the last inlined operation (could be null if the source block was empty)
-struct InlineInfo {
- Block *sourceBlock;
- Operation *firstInlinedInst;
- Operation *lastInlinedInst;
-};
-
-/// The storage class for an undoable block action (one of BlockActionKind),
-/// contains the information necessary to undo this action.
-struct BlockAction {
- static BlockAction getCreate(Block *block) {
-return {BlockActionKind::Create, block, {}};
- }
- static BlockAction getErase(Block *block, BlockPosition originalPosition) {
-return {BlockActionKind::Erase, block, {originalPosition}};
- }
- static BlockAction getInline(Block *block, Block *srcBlock,
- Block::iterator before) {
-BlockAction action{BlockActionKind::Inline, block, {}};
-action.inlineInfo = {srcBlock,
- srcBlock->empty() ? nullptr : &srcBlock->front(),
- srcBlock->empty() ? nullptr : &srcBlock-
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Modularize block actions (PR #81237)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81237
>From ae9dcbbcf4a23cd9f5a28195ceb3687957fa730f Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Fri, 9 Feb 2024 09:29:10 +
Subject: [PATCH] [mlir][Transforms][NFC] Modularize block actions
Throughout the rewrite process, the dialect conversion maintains a list
of "block actions" that can be rolled back upon failure. This commit
encapsulates the existing block actions into separate classes, making it
easier to add additional actions in the future.
This commit also renames "block actions" to "rewrite actions". In a
subsequent commit, an "operation action" that allows rolling back
movements of single operations is added. This is to support
`moveOpBefore` in the dialect conversion.
Rewrite actions have two methods: `commit()` commits an action. It can
no longer be rolled back afterwards. `rollback()` undoes an action. It
can no longer be committed afterwards.
---
.../Transforms/Utils/DialectConversion.cpp| 466 +++---
1 file changed, 283 insertions(+), 183 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index e41231d7cbd390..44c107c8733f3d 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,13 +154,13 @@ namespace {
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
unsigned numReplacements, unsigned numArgReplacements,
-unsigned numBlockActions, unsigned numIgnoredOperations,
+unsigned numRewriteActions, unsigned numIgnoredOperations,
unsigned numRootUpdates)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
numReplacements(numReplacements),
numArgReplacements(numArgReplacements),
-numBlockActions(numBlockActions),
+numRewriteActions(numRewriteActions),
numIgnoredOperations(numIgnoredOperations),
numRootUpdates(numRootUpdates) {}
@@ -176,8 +176,8 @@ struct RewriterState {
/// The current number of argument replacements queued.
unsigned numArgReplacements;
- /// The current number of block actions performed.
- unsigned numBlockActions;
+ /// The current number of rewrite actions performed.
+ unsigned numRewriteActions;
/// The current number of ignored operations.
unsigned numIgnoredOperations;
@@ -235,86 +235,6 @@ struct OpReplacement {
const TypeConverter *converter;
};
-//===--===//
-// BlockAction
-
-/// The kind of the block action performed during the rewrite. Actions can be
-/// undone if the conversion fails.
-enum class BlockActionKind {
- Create,
- Erase,
- Inline,
- Move,
- Split,
- TypeConversion
-};
-
-/// Original position of the given block in its parent region. During undo
-/// actions, the block needs to be placed before `insertBeforeBlock`.
-struct BlockPosition {
- Region *region;
- Block *insertBeforeBlock;
-};
-
-/// Information needed to undo inlining actions.
-/// - the source block
-/// - the first inlined operation (could be null if the source block was empty)
-/// - the last inlined operation (could be null if the source block was empty)
-struct InlineInfo {
- Block *sourceBlock;
- Operation *firstInlinedInst;
- Operation *lastInlinedInst;
-};
-
-/// The storage class for an undoable block action (one of BlockActionKind),
-/// contains the information necessary to undo this action.
-struct BlockAction {
- static BlockAction getCreate(Block *block) {
-return {BlockActionKind::Create, block, {}};
- }
- static BlockAction getErase(Block *block, BlockPosition originalPosition) {
-return {BlockActionKind::Erase, block, {originalPosition}};
- }
- static BlockAction getInline(Block *block, Block *srcBlock,
- Block::iterator before) {
-BlockAction action{BlockActionKind::Inline, block, {}};
-action.inlineInfo = {srcBlock,
- srcBlock->empty() ? nullptr : &srcBlock->front(),
- srcBlock->empty() ? nullptr : &srcBlock->back()};
-return action;
- }
- static BlockAction getMove(Block *block, BlockPosition originalPosition) {
-return {BlockActionKind::Move, block, {originalPosition}};
- }
- static BlockAction getSplit(Block *block, Block *originalBlock) {
-BlockAction action{BlockActionKind::Split, block, {}};
-action.originalBlock = originalBlock;
-return action;
- }
- static BlockAction getTypeConversion(Block *block) {
-return BlockAction{BlockActionKind::TypeConversion, block, {}};
- }
-
- // The action kind.
- BlockActionKind kind;
-
- // A pointer to the block that was created by the action.
- Block *block;
-
- union {
-// In use if kind == BlockActionK
[llvm-branch-commits] [mlir] [mlir][Transforms] Support `moveOpBefore`/`After` in dialect conversion (PR #81240)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/81240
Add a new rewrite action for "operation movements". This action can roll back
`moveOpBefore` and `moveOpAfter`.
`RewriterBase::moveOpBefore` and `RewriterBase::moveOpAfter` is no longer
virtual. (The dialect conversion can gather all required information for
rollbacks from listener notifications.)
>From 7503c0cb484c54249ff66c5780197d46937c660d Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Fri, 9 Feb 2024 09:58:46 +
Subject: [PATCH] [mlir][Transforms] Support `moveOpBefore`/`After` in dialect
conversion
Add a new rewrite action for "operation movements". This action can roll back
`moveOpBefore` and `moveOpAfter`.
`RewriterBase::moveOpBefore` and `RewriterBase::moveOpAfter` is no longer
virtual. (The dialect conversion can gather all required information for
rollbacks from listener notifications.)
---
mlir/include/mlir/IR/PatternMatch.h | 6 +-
.../mlir/Transforms/DialectConversion.h | 5 --
.../Transforms/Utils/DialectConversion.cpp| 74 +++
mlir/test/Transforms/test-legalizer.mlir | 14
mlir/test/lib/Dialect/Test/TestPatterns.cpp | 20 -
5 files changed, 93 insertions(+), 26 deletions(-)
diff --git a/mlir/include/mlir/IR/PatternMatch.h
b/mlir/include/mlir/IR/PatternMatch.h
index 78dcfe7f6fc3d2..b8aeea0d23475b 100644
--- a/mlir/include/mlir/IR/PatternMatch.h
+++ b/mlir/include/mlir/IR/PatternMatch.h
@@ -588,8 +588,7 @@ class RewriterBase : public OpBuilder {
/// Unlink this operation from its current block and insert it right before
/// `iterator` in the specified block.
- virtual void moveOpBefore(Operation *op, Block *block,
-Block::iterator iterator);
+ void moveOpBefore(Operation *op, Block *block, Block::iterator iterator);
/// Unlink this operation from its current block and insert it right after
/// `existingOp` which may be in the same or another block in the same
@@ -598,8 +597,7 @@ class RewriterBase : public OpBuilder {
/// Unlink this operation from its current block and insert it right after
/// `iterator` in the specified block.
- virtual void moveOpAfter(Operation *op, Block *block,
- Block::iterator iterator);
+ void moveOpAfter(Operation *op, Block *block, Block::iterator iterator);
/// Unlink this block and insert it right before `existingBlock`.
void moveBlockBefore(Block *block, Block *anotherBlock);
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index f061d761ecefbb..c0c702a7d34821 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -738,11 +738,6 @@ class ConversionPatternRewriter final : public
PatternRewriter {
// Hide unsupported pattern rewriter API.
using OpBuilder::setListener;
- void moveOpBefore(Operation *op, Block *block,
-Block::iterator iterator) override;
- void moveOpAfter(Operation *op, Block *block,
- Block::iterator iterator) override;
-
std::unique_ptr impl;
};
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 44c107c8733f3d..ffdb069f6e9b81 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -757,7 +757,8 @@ class RewriteAction {
InlineBlock,
MoveBlock,
SplitBlock,
-BlockTypeConversion
+BlockTypeConversion,
+MoveOperation
};
virtual ~RewriteAction() = default;
@@ -970,6 +971,54 @@ class BlockTypeConversionAction : public BlockAction {
void rollback() override;
};
+
+/// An operation rewrite.
+class OperationAction : public RewriteAction {
+public:
+ /// Return the operation that this action operates on.
+ Operation *getOperation() const { return op; }
+
+ static bool classof(const RewriteAction *action) {
+return action->getKind() >= Kind::MoveOperation &&
+ action->getKind() <= Kind::MoveOperation;
+ }
+
+protected:
+ OperationAction(Kind kind, ConversionPatternRewriterImpl &rewriterImpl,
+ Operation *op)
+ : RewriteAction(kind, rewriterImpl), op(op) {}
+
+ // The operation that this action operates on.
+ Operation *op;
+};
+
+/// Rewrite action that represent the moving of a block.
+class MoveOperationAction : public OperationAction {
+public:
+ MoveOperationAction(ConversionPatternRewriterImpl &rewriterImpl,
+ Operation *op, Block *block, Operation *insertBeforeOp)
+ : OperationAction(Kind::MoveOperation, rewriterImpl, op), block(block),
+insertBeforeOp(insertBeforeOp) {}
+
+ static bool classof(const RewriteAction *action) {
+return action->getKind() == Kind::MoveOperation;
+ }
+
+ void rollback() override {
+// Move the operation back to its original pos
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn in-place op modifications into `RewriteAction`s (PR #81245)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/81245
This commit simplifies the internal state of the dialect conversion. A separate
field for the previous state of in-place op modifications is no longer needed.
>From cdbd92786887ba8f801cb0c4299f708f0a410465 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Fri, 9 Feb 2024 11:36:59 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn in-place op modifications into
`RewriteAction`s
This commit simplifies the internal state of the dialect conversion. A separate
field for the previous state of in-place op modifications is no longer needed.
---
.../Transforms/Utils/DialectConversion.cpp| 128 --
1 file changed, 58 insertions(+), 70 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index ffdb069f6e9b81..d0114a148cd374 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,15 +154,13 @@ namespace {
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
unsigned numReplacements, unsigned numArgReplacements,
-unsigned numRewriteActions, unsigned numIgnoredOperations,
-unsigned numRootUpdates)
+unsigned numRewriteActions, unsigned numIgnoredOperations)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
numReplacements(numReplacements),
numArgReplacements(numArgReplacements),
numRewriteActions(numRewriteActions),
-numIgnoredOperations(numIgnoredOperations),
-numRootUpdates(numRootUpdates) {}
+numIgnoredOperations(numIgnoredOperations) {}
/// The current number of created operations.
unsigned numCreatedOps;
@@ -181,44 +179,6 @@ struct RewriterState {
/// The current number of ignored operations.
unsigned numIgnoredOperations;
-
- /// The current number of operations that were updated in place.
- unsigned numRootUpdates;
-};
-
-//===--===//
-// OperationTransactionState
-
-/// The state of an operation that was updated by a pattern in-place. This
-/// contains all of the necessary information to reconstruct an operation that
-/// was updated in place.
-class OperationTransactionState {
-public:
- OperationTransactionState() = default;
- OperationTransactionState(Operation *op)
- : op(op), loc(op->getLoc()), attrs(op->getAttrDictionary()),
-operands(op->operand_begin(), op->operand_end()),
-successors(op->successor_begin(), op->successor_end()) {}
-
- /// Discard the transaction state and reset the state of the original
- /// operation.
- void resetOperation() const {
-op->setLoc(loc);
-op->setAttrs(attrs);
-op->setOperands(operands);
-for (const auto &it : llvm::enumerate(successors))
- op->setSuccessor(it.value(), it.index());
- }
-
- /// Return the original operation of this state.
- Operation *getOperation() const { return op; }
-
-private:
- Operation *op;
- LocationAttr loc;
- DictionaryAttr attrs;
- SmallVector operands;
- SmallVector successors;
};
//===--===//
@@ -758,7 +718,8 @@ class RewriteAction {
MoveBlock,
SplitBlock,
BlockTypeConversion,
-MoveOperation
+MoveOperation,
+ModifyOperation
};
virtual ~RewriteAction() = default;
@@ -980,7 +941,7 @@ class OperationAction : public RewriteAction {
static bool classof(const RewriteAction *action) {
return action->getKind() >= Kind::MoveOperation &&
- action->getKind() <= Kind::MoveOperation;
+ action->getKind() <= Kind::ModifyOperation;
}
protected:
@@ -1019,6 +980,34 @@ class MoveOperationAction : public OperationAction {
// this operation was the only operation in the region.
Operation *insertBeforeOp;
};
+
+/// Rewrite action that represents the in-place modification of an operation.
+/// The previous state of the operation is stored in this action.
+class ModifyOperationAction : public OperationAction {
+public:
+ ModifyOperationAction(ConversionPatternRewriterImpl &rewriterImpl,
+Operation *op)
+ : OperationAction(Kind::ModifyOperation, rewriterImpl, op),
+loc(op->getLoc()), attrs(op->getAttrDictionary()),
+operands(op->operand_begin(), op->operand_end()),
+successors(op->successor_begin(), op->successor_end()) {}
+
+ /// Discard the transaction state and reset the state of the original
+ /// operation.
+ void rollback() override {
+op->setLoc(loc);
+op->setAttrs(attrs);
+op->setOperands(operands);
+for (const auto &it : llvm::enumerate(successors))
+ op->setSuccessor(it.value(), it.index());
+ }
+
+private:
+
[llvm-branch-commits] [mlir] [mlir][Transforms] Support `moveOpBefore`/`After` in dialect conversion (PR #81240)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81240
>From c60c43bcd2296715ceca83a3f9666433883ec303 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Mon, 12 Feb 2024 09:05:50 +
Subject: [PATCH 1/2] [mlir][Transforms][WIP] RewriteAction
BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
.../Transforms/Utils/DialectConversion.cpp| 504 +++---
1 file changed, 306 insertions(+), 198 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index e41231d7cbd390..edca84e5a73f04 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,13 +154,12 @@ namespace {
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
unsigned numReplacements, unsigned numArgReplacements,
-unsigned numBlockActions, unsigned numIgnoredOperations,
+unsigned numRewrites, unsigned numIgnoredOperations,
unsigned numRootUpdates)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
numReplacements(numReplacements),
-numArgReplacements(numArgReplacements),
-numBlockActions(numBlockActions),
+numArgReplacements(numArgReplacements), numRewrites(numRewrites),
numIgnoredOperations(numIgnoredOperations),
numRootUpdates(numRootUpdates) {}
@@ -176,8 +175,8 @@ struct RewriterState {
/// The current number of argument replacements queued.
unsigned numArgReplacements;
- /// The current number of block actions performed.
- unsigned numBlockActions;
+ /// The current number of rewrites performed.
+ unsigned numRewrites;
/// The current number of ignored operations.
unsigned numIgnoredOperations;
@@ -235,86 +234,6 @@ struct OpReplacement {
const TypeConverter *converter;
};
-//===--===//
-// BlockAction
-
-/// The kind of the block action performed during the rewrite. Actions can be
-/// undone if the conversion fails.
-enum class BlockActionKind {
- Create,
- Erase,
- Inline,
- Move,
- Split,
- TypeConversion
-};
-
-/// Original position of the given block in its parent region. During undo
-/// actions, the block needs to be placed before `insertBeforeBlock`.
-struct BlockPosition {
- Region *region;
- Block *insertBeforeBlock;
-};
-
-/// Information needed to undo inlining actions.
-/// - the source block
-/// - the first inlined operation (could be null if the source block was empty)
-/// - the last inlined operation (could be null if the source block was empty)
-struct InlineInfo {
- Block *sourceBlock;
- Operation *firstInlinedInst;
- Operation *lastInlinedInst;
-};
-
-/// The storage class for an undoable block action (one of BlockActionKind),
-/// contains the information necessary to undo this action.
-struct BlockAction {
- static BlockAction getCreate(Block *block) {
-return {BlockActionKind::Create, block, {}};
- }
- static BlockAction getErase(Block *block, BlockPosition originalPosition) {
-return {BlockActionKind::Erase, block, {originalPosition}};
- }
- static BlockAction getInline(Block *block, Block *srcBlock,
- Block::iterator before) {
-BlockAction action{BlockActionKind::Inline, block, {}};
-action.inlineInfo = {srcBlock,
- srcBlock->empty() ? nullptr : &srcBlock->front(),
- srcBlock->empty() ? nullptr : &srcBlock->back()};
-return action;
- }
- static BlockAction getMove(Block *block, BlockPosition originalPosition) {
-return {BlockActionKind::Move, block, {originalPosition}};
- }
- static BlockAction getSplit(Block *block, Block *originalBlock) {
-BlockAction action{BlockActionKind::Split, block, {}};
-action.originalBlock = originalBlock;
-return action;
- }
- static BlockAction getTypeConversion(Block *block) {
-return BlockAction{BlockActionKind::TypeConversion, block, {}};
- }
-
- // The action kind.
- BlockActionKind kind;
-
- // A pointer to the block that was created by the action.
- Block *block;
-
- union {
-// In use if kind == BlockActionKind::Inline or BlockActionKind::Erase, and
-// contains a pointer to the region that originally contained the block as
-// well as the position of the block in that region.
-BlockPosition originalPosition;
-// In use if kind == BlockActionKind::Split and contains a pointer to the
-// block that was split into two parts.
-Block *originalBlock;
-// In use if kind == BlockActionKind::Inline, and contains the information
-// needed to undo the inlining.
-InlineInfo inlineInfo;
- };
-};
-
//===--===/
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Modularize block actions (PR #81237)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81237
>From c60c43bcd2296715ceca83a3f9666433883ec303 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Mon, 12 Feb 2024 09:05:50 +
Subject: [PATCH] [mlir][Transforms][WIP] RewriteAction
BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
.../Transforms/Utils/DialectConversion.cpp| 504 +++---
1 file changed, 306 insertions(+), 198 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index e41231d7cbd390..edca84e5a73f04 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,13 +154,12 @@ namespace {
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
unsigned numReplacements, unsigned numArgReplacements,
-unsigned numBlockActions, unsigned numIgnoredOperations,
+unsigned numRewrites, unsigned numIgnoredOperations,
unsigned numRootUpdates)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
numReplacements(numReplacements),
-numArgReplacements(numArgReplacements),
-numBlockActions(numBlockActions),
+numArgReplacements(numArgReplacements), numRewrites(numRewrites),
numIgnoredOperations(numIgnoredOperations),
numRootUpdates(numRootUpdates) {}
@@ -176,8 +175,8 @@ struct RewriterState {
/// The current number of argument replacements queued.
unsigned numArgReplacements;
- /// The current number of block actions performed.
- unsigned numBlockActions;
+ /// The current number of rewrites performed.
+ unsigned numRewrites;
/// The current number of ignored operations.
unsigned numIgnoredOperations;
@@ -235,86 +234,6 @@ struct OpReplacement {
const TypeConverter *converter;
};
-//===--===//
-// BlockAction
-
-/// The kind of the block action performed during the rewrite. Actions can be
-/// undone if the conversion fails.
-enum class BlockActionKind {
- Create,
- Erase,
- Inline,
- Move,
- Split,
- TypeConversion
-};
-
-/// Original position of the given block in its parent region. During undo
-/// actions, the block needs to be placed before `insertBeforeBlock`.
-struct BlockPosition {
- Region *region;
- Block *insertBeforeBlock;
-};
-
-/// Information needed to undo inlining actions.
-/// - the source block
-/// - the first inlined operation (could be null if the source block was empty)
-/// - the last inlined operation (could be null if the source block was empty)
-struct InlineInfo {
- Block *sourceBlock;
- Operation *firstInlinedInst;
- Operation *lastInlinedInst;
-};
-
-/// The storage class for an undoable block action (one of BlockActionKind),
-/// contains the information necessary to undo this action.
-struct BlockAction {
- static BlockAction getCreate(Block *block) {
-return {BlockActionKind::Create, block, {}};
- }
- static BlockAction getErase(Block *block, BlockPosition originalPosition) {
-return {BlockActionKind::Erase, block, {originalPosition}};
- }
- static BlockAction getInline(Block *block, Block *srcBlock,
- Block::iterator before) {
-BlockAction action{BlockActionKind::Inline, block, {}};
-action.inlineInfo = {srcBlock,
- srcBlock->empty() ? nullptr : &srcBlock->front(),
- srcBlock->empty() ? nullptr : &srcBlock->back()};
-return action;
- }
- static BlockAction getMove(Block *block, BlockPosition originalPosition) {
-return {BlockActionKind::Move, block, {originalPosition}};
- }
- static BlockAction getSplit(Block *block, Block *originalBlock) {
-BlockAction action{BlockActionKind::Split, block, {}};
-action.originalBlock = originalBlock;
-return action;
- }
- static BlockAction getTypeConversion(Block *block) {
-return BlockAction{BlockActionKind::TypeConversion, block, {}};
- }
-
- // The action kind.
- BlockActionKind kind;
-
- // A pointer to the block that was created by the action.
- Block *block;
-
- union {
-// In use if kind == BlockActionKind::Inline or BlockActionKind::Erase, and
-// contains a pointer to the region that originally contained the block as
-// well as the position of the block in that region.
-BlockPosition originalPosition;
-// In use if kind == BlockActionKind::Split and contains a pointer to the
-// block that was split into two parts.
-Block *originalBlock;
-// In use if kind == BlockActionKind::Inline, and contains the information
-// needed to undo the inlining.
-InlineInfo inlineInfo;
- };
-};
-
//===--===//
/
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn in-place op modifications into `RewriteAction`s (PR #81245)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81245
>From f7010ea9f363c011f4171a6329ad6de30880c3e7 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Mon, 12 Feb 2024 09:11:47 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn in-place op modifications into
`RewriteAction`s
This commit simplifies the internal state of the dialect conversion. A separate
field for the previous state of in-place op modifications is no longer needed.
BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
.../mlir/Transforms/DialectConversion.h | 4 +-
.../Transforms/Utils/DialectConversion.cpp| 123 --
2 files changed, 56 insertions(+), 71 deletions(-)
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index b028d2b71b3762..c0c702a7d34821 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -721,8 +721,8 @@ class ConversionPatternRewriter final : public
PatternRewriter {
/// PatternRewriter hook for updating the given operation in-place.
/// Note: These methods only track updates to the given operation itself,
- /// and not nested regions. Updates to regions will still require
- /// notification through other more specific hooks above.
+ /// and not nested regions. Updates to regions will still require
notification
+ /// through other more specific hooks above.
void startOpModification(Operation *op) override;
/// PatternRewriter hook for updating the given operation in-place.
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 85b67bb834de7c..489ccd0139c7f2 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,14 +154,12 @@ namespace {
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
unsigned numReplacements, unsigned numArgReplacements,
-unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numRootUpdates)
+unsigned numRewrites, unsigned numIgnoredOperations)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
numReplacements(numReplacements),
numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations),
-numRootUpdates(numRootUpdates) {}
+numIgnoredOperations(numIgnoredOperations) {}
/// The current number of created operations.
unsigned numCreatedOps;
@@ -180,44 +178,6 @@ struct RewriterState {
/// The current number of ignored operations.
unsigned numIgnoredOperations;
-
- /// The current number of operations that were updated in place.
- unsigned numRootUpdates;
-};
-
-//===--===//
-// OperationTransactionState
-
-/// The state of an operation that was updated by a pattern in-place. This
-/// contains all of the necessary information to reconstruct an operation that
-/// was updated in place.
-class OperationTransactionState {
-public:
- OperationTransactionState() = default;
- OperationTransactionState(Operation *op)
- : op(op), loc(op->getLoc()), attrs(op->getAttrDictionary()),
-operands(op->operand_begin(), op->operand_end()),
-successors(op->successor_begin(), op->successor_end()) {}
-
- /// Discard the transaction state and reset the state of the original
- /// operation.
- void resetOperation() const {
-op->setLoc(loc);
-op->setAttrs(attrs);
-op->setOperands(operands);
-for (const auto &it : llvm::enumerate(successors))
- op->setSuccessor(it.value(), it.index());
- }
-
- /// Return the original operation of this state.
- Operation *getOperation() const { return op; }
-
-private:
- Operation *op;
- LocationAttr loc;
- DictionaryAttr attrs;
- SmallVector operands;
- SmallVector successors;
};
//===--===//
@@ -761,7 +721,8 @@ class IRRewrite {
MoveBlock,
SplitBlock,
BlockTypeConversion,
-MoveOperation
+MoveOperation,
+ModifyOperation
};
virtual ~IRRewrite() = default;
@@ -992,7 +953,7 @@ class OperationRewrite : public IRRewrite {
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() >= Kind::MoveOperation &&
- rewrite->getKind() <= Kind::MoveOperation;
+ rewrite->getKind() <= Kind::ModifyOperation;
}
protected:
@@ -1031,6 +992,34 @@ class MoveOperationRewrite : public OperationRewrite {
// this operation was the only operation in the region.
Operation *insertBeforeOp;
};
+
+/// In-place modification of an op. This rewrite is immediately reflected in
+///
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Modularize block actions (PR #81237)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/81237 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Modularize block actions (PR #81237)
@@ -820,6 +740,238 @@ void ArgConverter::insertConversion(Block *newBlock,
conversionInfo.insert({newBlock, std::move(info)});
}
+//===--===//
+// RewriteAction
matthias-springer wrote:
I renamed `RewriteAction` to `IRRewrite`.
https://github.com/llvm/llvm-project/pull/81237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Support `moveOpBefore`/`After` in dialect conversion (PR #81240)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/81240 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Support `moveOpBefore`/`After` in dialect conversion (PR #81240)
@@ -970,6 +971,54 @@ class BlockTypeConversionAction : public BlockAction {
void rollback() override;
};
+
+/// An operation rewrite.
matthias-springer wrote:
I renamed the class to `OperationRewrite`. I added more documentation to the
superclass `IRRewrite` in the dependent PR.
https://github.com/llvm/llvm-project/pull/81240
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn in-place op modification into `IRRewrite` (PR #81245)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/81245 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Simplify `ArgConverter` state (PR #81462)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/81462
* When converting a block signature, `ArgConverter` creates a new block with
the new signature and moves all operation from the old block to the new block.
The new block is temporarily inserted into a region that is stored in
`regionMapping`. The old block is not yet deleted, so that the conversion can
be rolled back. `regionMapping` is not needed. Instead of moving the old block
to a temporary region, it can just be unlinked. Block erasures are handles in
the same way in the dialect conversion.
* `regionToConverter` is a mapping from regions to type converter. That field
is never accessed within `ArgConverter`. It should be stored in
`ConversionPatternRewriterImpl` instead.
>From 5af1476c5d439c14122ffb50d24923e522a61b32 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Mon, 12 Feb 2024 10:53:59 +
Subject: [PATCH] [mlir][Transforms][NFC] Simplify `ArgConverter` state
* When converting a block signature, `ArgConverter` creates a new block with
the new signature and moves all operation from the old block to the new block.
The new block is temporarily inserted into a region that is stored in
`regionMapping`. The old block is not yet deleted, so that the conversion can
be rolled back. `regionMapping` is not needed. Instead of moving the old block
to a temporary region, it can just be unlinked. Block erasures are handles in
the same way in the dialect conversion.
* `regionToConverter` is a mapping from regions to type converter. That field
is never accessed within `ArgConverter`. It should be stored in
`ConversionPatternRewriterImpl` instead.
---
.../Transforms/Utils/DialectConversion.cpp| 63 ++-
1 file changed, 18 insertions(+), 45 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 489ccd0139c7f..53717f632621d 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -348,18 +348,6 @@ struct ArgConverter {
return conversionInfo.count(block) || convertedBlocks.count(block);
}
- /// Set the type converter to use for the given region.
- void setConverter(Region *region, const TypeConverter *typeConverter) {
-assert(typeConverter && "expected valid type converter");
-regionToConverter[region] = typeConverter;
- }
-
- /// Return the type converter to use for the given region, or null if there
- /// isn't one.
- const TypeConverter *getConverter(Region *region) {
-return regionToConverter.lookup(region);
- }
-
//======//
// Rewrite Application
//======//
@@ -409,9 +397,6 @@ struct ArgConverter {
ConversionValueMapping &mapping,
SmallVectorImpl &argReplacements);
- /// Insert a new conversion into the cache.
- void insertConversion(Block *newBlock, ConvertedBlockInfo &&info);
-
/// A collection of blocks that have had their arguments converted. This is a
/// map from the new replacement block, back to the original block.
llvm::MapVector conversionInfo;
@@ -419,14 +404,6 @@ struct ArgConverter {
/// The set of original blocks that were converted.
DenseSet convertedBlocks;
- /// A mapping from valid regions, to those containing the original blocks of
a
- /// conversion.
- DenseMap> regionMapping;
-
- /// A mapping of regions to type converters that should be used when
- /// converting the arguments of blocks within that region.
- DenseMap regionToConverter;
-
/// The pattern rewriter to use when materializing conversions.
PatternRewriter &rewriter;
@@ -474,9 +451,10 @@ void ArgConverter::discardRewrites(Block *block) {
block->getArgument(i).dropAllUses();
block->replaceAllUsesWith(origBlock);
- // Move the operations back the original block and the delete the new block.
+ // Move the operations back the original block, move the original block back
+ // into its original location and the delete the new block.
origBlock->getOperations().splice(origBlock->end(), block->getOperations());
- origBlock->moveBefore(block);
+ block->getParent()->getBlocks().insert(Region::iterator(block), origBlock);
block->erase();
convertedBlocks.erase(origBlock);
@@ -510,6 +488,9 @@ void ArgConverter::applyRewrites(ConversionValueMapping
&mapping) {
mapping.lookupOrDefault(castValue, origArg.getType()));
}
}
+
+delete origBlock;
+blockInfo.origBlock = nullptr;
}
}
@@ -603,6 +584,9 @@ Block *ArgConverter::applySignatureConversion(
// signature.
Block *newBlock = block->splitBlock(block->begin());
block->replaceAllUsesWith(newBlock);
+ // Unlink the block, but do not erase it yet, so that the change can be
rolled
+ // back.
+ block->getParent()->getBlocks().remo
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Simplify `ArgConverter` state (PR #81462)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81462
>From ba1a8085ab576b56a7d1780d9a3f01cb6769df65 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Mon, 12 Feb 2024 11:24:52 +
Subject: [PATCH] [mlir][Transforms][NFC] Simplify `ArgConverter` state
* When converting a block signature, `ArgConverter` creates a new block with
the new signature and moves all operation from the old block to the new block.
The new block is temporarily inserted into a region that is stored in
`regionMapping`. The old block is not yet deleted, so that the conversion can
be rolled back. `regionMapping` is not needed. Instead of moving the old block
to a temporary region, it can just be unlinked. Block erasures are handles in
the same way in the dialect conversion.
* `regionToConverter` is a mapping from regions to type converter. That field
is never accessed within `ArgConverter`. It should be stored in
`ConversionPatternRewriterImpl` instead.
---
.../Transforms/Utils/DialectConversion.cpp| 79 ++-
1 file changed, 22 insertions(+), 57 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 489ccd0139c7f2..e1c838ea1e43a4 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -343,23 +343,6 @@ struct ArgConverter {
const TypeConverter *converter;
};
- /// Return if the signature of the given block has already been converted.
- bool hasBeenConverted(Block *block) const {
-return conversionInfo.count(block) || convertedBlocks.count(block);
- }
-
- /// Set the type converter to use for the given region.
- void setConverter(Region *region, const TypeConverter *typeConverter) {
-assert(typeConverter && "expected valid type converter");
-regionToConverter[region] = typeConverter;
- }
-
- /// Return the type converter to use for the given region, or null if there
- /// isn't one.
- const TypeConverter *getConverter(Region *region) {
-return regionToConverter.lookup(region);
- }
-
//======//
// Rewrite Application
//======//
@@ -409,24 +392,10 @@ struct ArgConverter {
ConversionValueMapping &mapping,
SmallVectorImpl &argReplacements);
- /// Insert a new conversion into the cache.
- void insertConversion(Block *newBlock, ConvertedBlockInfo &&info);
-
/// A collection of blocks that have had their arguments converted. This is a
/// map from the new replacement block, back to the original block.
llvm::MapVector conversionInfo;
- /// The set of original blocks that were converted.
- DenseSet convertedBlocks;
-
- /// A mapping from valid regions, to those containing the original blocks of
a
- /// conversion.
- DenseMap> regionMapping;
-
- /// A mapping of regions to type converters that should be used when
- /// converting the arguments of blocks within that region.
- DenseMap regionToConverter;
-
/// The pattern rewriter to use when materializing conversions.
PatternRewriter &rewriter;
@@ -474,12 +443,12 @@ void ArgConverter::discardRewrites(Block *block) {
block->getArgument(i).dropAllUses();
block->replaceAllUsesWith(origBlock);
- // Move the operations back the original block and the delete the new block.
+ // Move the operations back the original block, move the original block back
+ // into its original location and the delete the new block.
origBlock->getOperations().splice(origBlock->end(), block->getOperations());
- origBlock->moveBefore(block);
+ block->getParent()->getBlocks().insert(Region::iterator(block), origBlock);
block->erase();
- convertedBlocks.erase(origBlock);
conversionInfo.erase(it);
}
@@ -510,6 +479,9 @@ void ArgConverter::applyRewrites(ConversionValueMapping
&mapping) {
mapping.lookupOrDefault(castValue, origArg.getType()));
}
}
+
+delete origBlock;
+blockInfo.origBlock = nullptr;
}
}
@@ -572,9 +544,11 @@ FailureOr ArgConverter::convertSignature(
Block *block, const TypeConverter *converter,
ConversionValueMapping &mapping,
SmallVectorImpl &argReplacements) {
- // Check if the block was already converted. If the block is detached,
- // conservatively assume it is going to be deleted.
- if (hasBeenConverted(block) || !block->getParent())
+ // Check if the block was already converted.
+ // * If the block is mapped in `conversionInfo`, it is a converted block.
+ // * If the block is detached, conservatively assume that it is going to be
+ // deleted; it is likely the old block (before it was converted).
+ if (conversionInfo.count(block) || !block->getParent())
return block;
// If a converter wasn't provided, and the block wasn't already converted,
// there is nothing we can do
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Simplify `ArgConverter` state (PR #81462)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/81462 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Support `moveOpBefore`/`After` in dialect conversion (PR #81240)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81240
>From 1d17c768c1588850ceb331e3dd85930deec5728c Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 14 Feb 2024 16:08:38 +
Subject: [PATCH] [mlir][Transforms] Support `moveOpBefore`/`After` in dialect
conversion
Add a new rewrite action for "operation movements". This action can roll back
`moveOpBefore` and `moveOpAfter`.
`RewriterBase::moveOpBefore` and `RewriterBase::moveOpAfter` is no longer
virtual. (The dialect conversion can gather all required information for
rollbacks from listener notifications.)
BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
mlir/include/mlir/IR/PatternMatch.h | 6 +-
.../mlir/Transforms/DialectConversion.h | 9 +--
.../Transforms/Utils/DialectConversion.cpp| 74 +++
mlir/test/Transforms/test-legalizer.mlir | 14
mlir/test/lib/Dialect/Test/TestPatterns.cpp | 20 -
5 files changed, 95 insertions(+), 28 deletions(-)
diff --git a/mlir/include/mlir/IR/PatternMatch.h
b/mlir/include/mlir/IR/PatternMatch.h
index 78dcfe7f6fc3d2..b8aeea0d23475b 100644
--- a/mlir/include/mlir/IR/PatternMatch.h
+++ b/mlir/include/mlir/IR/PatternMatch.h
@@ -588,8 +588,7 @@ class RewriterBase : public OpBuilder {
/// Unlink this operation from its current block and insert it right before
/// `iterator` in the specified block.
- virtual void moveOpBefore(Operation *op, Block *block,
-Block::iterator iterator);
+ void moveOpBefore(Operation *op, Block *block, Block::iterator iterator);
/// Unlink this operation from its current block and insert it right after
/// `existingOp` which may be in the same or another block in the same
@@ -598,8 +597,7 @@ class RewriterBase : public OpBuilder {
/// Unlink this operation from its current block and insert it right after
/// `iterator` in the specified block.
- virtual void moveOpAfter(Operation *op, Block *block,
- Block::iterator iterator);
+ void moveOpAfter(Operation *op, Block *block, Block::iterator iterator);
/// Unlink this block and insert it right before `existingBlock`.
void moveBlockBefore(Block *block, Block *anotherBlock);
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index 851d639ae68a77..15fa39bde104b9 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -744,8 +744,8 @@ class ConversionPatternRewriter final : public
PatternRewriter {
/// PatternRewriter hook for updating the given operation in-place.
/// Note: These methods only track updates to the given operation itself,
- /// and not nested regions. Updates to regions will still require
notification
- /// through other more specific hooks above.
+ /// and not nested regions. Updates to regions will still require
+ /// notification through other more specific hooks above.
void startOpModification(Operation *op) override;
/// PatternRewriter hook for updating the given operation in-place.
@@ -761,11 +761,6 @@ class ConversionPatternRewriter final : public
PatternRewriter {
// Hide unsupported pattern rewriter API.
using OpBuilder::setListener;
- void moveOpBefore(Operation *op, Block *block,
-Block::iterator iterator) override;
- void moveOpAfter(Operation *op, Block *block,
- Block::iterator iterator) override;
-
std::unique_ptr impl;
};
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 9875f8668b65a8..84597fb7986b07 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -760,7 +760,8 @@ class IRRewrite {
InlineBlock,
MoveBlock,
SplitBlock,
-BlockTypeConversion
+BlockTypeConversion,
+MoveOperation
};
virtual ~IRRewrite() = default;
@@ -982,6 +983,54 @@ class BlockTypeConversionRewrite : public BlockRewrite {
// `ArgConverter::applyRewrites`. This should be done in the "commit" method.
void rollback() override;
};
+
+/// An operation rewrite.
+class OperationRewrite : public IRRewrite {
+public:
+ /// Return the operation that this rewrite operates on.
+ Operation *getOperation() const { return op; }
+
+ static bool classof(const IRRewrite *rewrite) {
+return rewrite->getKind() >= Kind::MoveOperation &&
+ rewrite->getKind() <= Kind::MoveOperation;
+ }
+
+protected:
+ OperationRewrite(Kind kind, ConversionPatternRewriterImpl &rewriterImpl,
+ Operation *op)
+ : IRRewrite(kind, rewriterImpl), op(op) {}
+
+ // The operation that this rewrite operates on.
+ Operation *op;
+};
+
+/// Moving of an operation. This rewrite is immediately reflected in the IR.
+class MoveOperationRewrite : public OperationRew
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn in-place op modification into `IRRewrite` (PR #81245)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81245
>From 4bb65218698f0104775f3eea05817c6d5228a9e7 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 14 Feb 2024 16:17:03 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn in-place op modifications into
`RewriteAction`s
This commit simplifies the internal state of the dialect conversion. A separate
field for the previous state of in-place op modifications is no longer needed.
BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
.../mlir/Transforms/DialectConversion.h | 4 +-
.../Transforms/Utils/DialectConversion.cpp| 139 +-
2 files changed, 68 insertions(+), 75 deletions(-)
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index 15fa39bde104b9..0d7722aa07ee38 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -744,8 +744,8 @@ class ConversionPatternRewriter final : public
PatternRewriter {
/// PatternRewriter hook for updating the given operation in-place.
/// Note: These methods only track updates to the given operation itself,
- /// and not nested regions. Updates to regions will still require
- /// notification through other more specific hooks above.
+ /// and not nested regions. Updates to regions will still require
notification
+ /// through other more specific hooks above.
void startOpModification(Operation *op) override;
/// PatternRewriter hook for updating the given operation in-place.
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 84597fb7986b07..5206a65608ba14 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,14 +154,12 @@ namespace {
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
unsigned numReplacements, unsigned numArgReplacements,
-unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numRootUpdates)
+unsigned numRewrites, unsigned numIgnoredOperations)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
numReplacements(numReplacements),
numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations),
-numRootUpdates(numRootUpdates) {}
+numIgnoredOperations(numIgnoredOperations) {}
/// The current number of created operations.
unsigned numCreatedOps;
@@ -180,44 +178,6 @@ struct RewriterState {
/// The current number of ignored operations.
unsigned numIgnoredOperations;
-
- /// The current number of operations that were updated in place.
- unsigned numRootUpdates;
-};
-
-//===--===//
-// OperationTransactionState
-
-/// The state of an operation that was updated by a pattern in-place. This
-/// contains all of the necessary information to reconstruct an operation that
-/// was updated in place.
-class OperationTransactionState {
-public:
- OperationTransactionState() = default;
- OperationTransactionState(Operation *op)
- : op(op), loc(op->getLoc()), attrs(op->getAttrDictionary()),
-operands(op->operand_begin(), op->operand_end()),
-successors(op->successor_begin(), op->successor_end()) {}
-
- /// Discard the transaction state and reset the state of the original
- /// operation.
- void resetOperation() const {
-op->setLoc(loc);
-op->setAttrs(attrs);
-op->setOperands(operands);
-for (const auto &it : llvm::enumerate(successors))
- op->setSuccessor(it.value(), it.index());
- }
-
- /// Return the original operation of this state.
- Operation *getOperation() const { return op; }
-
-private:
- Operation *op;
- LocationAttr loc;
- DictionaryAttr attrs;
- SmallVector operands;
- SmallVector successors;
};
//===--===//
@@ -761,7 +721,8 @@ class IRRewrite {
MoveBlock,
SplitBlock,
BlockTypeConversion,
-MoveOperation
+MoveOperation,
+ModifyOperation
};
virtual ~IRRewrite() = default;
@@ -992,7 +953,7 @@ class OperationRewrite : public IRRewrite {
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() >= Kind::MoveOperation &&
- rewrite->getKind() <= Kind::MoveOperation;
+ rewrite->getKind() <= Kind::ModifyOperation;
}
protected:
@@ -1031,8 +992,50 @@ class MoveOperationRewrite : public OperationRewrite {
// this operation was the only operation in the region.
Operation *insertBeforeOp;
};
+
+/// In-place modification of an op. This rewrite is immediately reflected in
+///
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Simplify `ArgConverter` state (PR #81462)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81462
>From c7afdb221528ad0aa76b2ba50a6db47eefd71bb2 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 14 Feb 2024 16:20:30 +
Subject: [PATCH] [mlir][Transforms][NFC] Simplify `ArgConverter` state
* When converting a block signature, `ArgConverter` creates a new block with
the new signature and moves all operation from the old block to the new block.
The new block is temporarily inserted into a region that is stored in
`regionMapping`. The old block is not yet deleted, so that the conversion can
be rolled back. `regionMapping` is not needed. Instead of moving the old block
to a temporary region, it can just be unlinked. Block erasures are handles in
the same way in the dialect conversion.
* `regionToConverter` is a mapping from regions to type converter. That field
is never accessed within `ArgConverter`. It should be stored in
`ConversionPatternRewriterImpl` instead.
---
.../Transforms/Utils/DialectConversion.cpp| 79 ++-
1 file changed, 22 insertions(+), 57 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 5206a65608ba14..67b076b295eae8 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -343,23 +343,6 @@ struct ArgConverter {
const TypeConverter *converter;
};
- /// Return if the signature of the given block has already been converted.
- bool hasBeenConverted(Block *block) const {
-return conversionInfo.count(block) || convertedBlocks.count(block);
- }
-
- /// Set the type converter to use for the given region.
- void setConverter(Region *region, const TypeConverter *typeConverter) {
-assert(typeConverter && "expected valid type converter");
-regionToConverter[region] = typeConverter;
- }
-
- /// Return the type converter to use for the given region, or null if there
- /// isn't one.
- const TypeConverter *getConverter(Region *region) {
-return regionToConverter.lookup(region);
- }
-
//======//
// Rewrite Application
//======//
@@ -409,24 +392,10 @@ struct ArgConverter {
ConversionValueMapping &mapping,
SmallVectorImpl &argReplacements);
- /// Insert a new conversion into the cache.
- void insertConversion(Block *newBlock, ConvertedBlockInfo &&info);
-
/// A collection of blocks that have had their arguments converted. This is a
/// map from the new replacement block, back to the original block.
llvm::MapVector conversionInfo;
- /// The set of original blocks that were converted.
- DenseSet convertedBlocks;
-
- /// A mapping from valid regions, to those containing the original blocks of
a
- /// conversion.
- DenseMap> regionMapping;
-
- /// A mapping of regions to type converters that should be used when
- /// converting the arguments of blocks within that region.
- DenseMap regionToConverter;
-
/// The pattern rewriter to use when materializing conversions.
PatternRewriter &rewriter;
@@ -474,12 +443,12 @@ void ArgConverter::discardRewrites(Block *block) {
block->getArgument(i).dropAllUses();
block->replaceAllUsesWith(origBlock);
- // Move the operations back the original block and the delete the new block.
+ // Move the operations back the original block, move the original block back
+ // into its original location and the delete the new block.
origBlock->getOperations().splice(origBlock->end(), block->getOperations());
- origBlock->moveBefore(block);
+ block->getParent()->getBlocks().insert(Region::iterator(block), origBlock);
block->erase();
- convertedBlocks.erase(origBlock);
conversionInfo.erase(it);
}
@@ -510,6 +479,9 @@ void ArgConverter::applyRewrites(ConversionValueMapping
&mapping) {
mapping.lookupOrDefault(castValue, origArg.getType()));
}
}
+
+delete origBlock;
+blockInfo.origBlock = nullptr;
}
}
@@ -572,9 +544,11 @@ FailureOr ArgConverter::convertSignature(
Block *block, const TypeConverter *converter,
ConversionValueMapping &mapping,
SmallVectorImpl &argReplacements) {
- // Check if the block was already converted. If the block is detached,
- // conservatively assume it is going to be deleted.
- if (hasBeenConverted(block) || !block->getParent())
+ // Check if the block was already converted.
+ // * If the block is mapped in `conversionInfo`, it is a converted block.
+ // * If the block is detached, conservatively assume that it is going to be
+ // deleted; it is likely the old block (before it was converted).
+ if (conversionInfo.count(block) || !block->getParent())
return block;
// If a converter wasn't provided, and the block wasn't already converted,
// there is nothing we can do
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn block type conversion into `IRRewrite` (PR #81756)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/81756
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be commited (upon success) or rolled
back (upon failure).
Until now, the signature conversion of a block was only a "partial" IR rewrite.
Rollbacks were triggered via `BlockTypeConversionRewrite::rollback`, but there
was no `BlockTypeConversionRewrite::commit` equivalent.
Overview of changes:
* Remove `ArgConverter`, an internal helper class that kept track of all block
type conversions. There is now a separate `BlockTypeConversionRewrite` for each
block type conversion.
* No more special handling for block type conversions. They are now normal "IR
rewrites", just like "block creation" or "block movement". In particular,
trigger "commits" of block type conversion via
`BlockTypeConversionRewrite::commit`.
* Remove `ArgConverter::notifyOpRemoved`. This function was used to inform the
`ArgConverter` that an operation was erased, to prevent a double-free of
operations in certain situations. It would be unpractical to add a
`notifyOpRemoved` API to `IRRewrite`. Instead, erasing ops/block should go
through a new `SingleEraseRewriter` (that is owned by the
`ConversionPatternRewriterImpl`) if there is chance of double-free. This
rewriter ignores `eraseOp`/`eraseBlock` if the op/block was already freed.
>From ae08e916df9fcee362a813b89c126716376e462f Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 14 Feb 2024 16:23:29 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn block type convertion into
`IRRewrite`
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be commited (upon success) or rolled
back (upon failure).
Until now, the signature conversion of a block was only a "partial" IR rewrite.
Rollbacks were triggered via `BlockTypeConversionRewrite::rollback`, but there
was no `BlockTypeConversionRewrite::commit` equivalent.
Overview of changes:
* Remove `ArgConverter`, an internal helper class that kept track of all block
type conversions. There is now a separate `BlockTypeConversionRewrite` for each
block type conversion.
* No more special handling for block type conversions. They are now normal "IR
rewrites", just like "block creation" or "block movement". In particular,
trigger "commits" of block type conversion via
`BlockTypeConversionRewrite::commit`.
* Remove `ArgConverter::notifyOpRemoved`. This function was used to inform the
`ArgConverter` that an operation was erased, to prevent a double-free of
operations in certain situations. It would be unpractical to add a
`notifyOpRemoved` API to `IRRewrite`. Instead, erasing ops/block should go
through a new `SingleEraseRewriter` (that is owned by the
`ConversionPatternRewriterImpl`) if there is chance of double-free. This
rewriter ignores `eraseOp`/`eraseBlock` if the op/block was already freed.
---
.../Transforms/Utils/DialectConversion.cpp| 810 --
1 file changed, 376 insertions(+), 434 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 67b076b295eae8..b2baa88879b6e9 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,12 +154,13 @@ namespace {
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
unsigned numReplacements, unsigned numArgReplacements,
-unsigned numRewrites, unsigned numIgnoredOperations)
+unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
numReplacements(numReplacements),
numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations) {}
+numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
/// The current number of created operations.
unsigned numCreatedOps;
@@ -178,6 +179,9 @@ struct RewriterState {
/// The current number of ignored operations.
unsigned numIgnoredOperations;
+
+ /// The current number of erased operations/blocks.
+ unsigned numErased;
};
//===--===//
@@ -292,374 +296,6 @@ static Value buildUnresolvedTargetMaterialization(
outputType, outputType, converter, unresolvedMaterializations);
}
-//===--===//
-// ArgConverter
-//===--===//
-namespace {
-/// This class provides a simple interface for converting the types of block
-/// arguments. This is done by cre
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op/block arg replacements into `IRRewrite`s (PR #81757)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/81757
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be committed (upon success) or
rolled back (upon failure).
Until now, op replacements and block argument replacements were kept track in
separate data structures inside the dialect conversion. This commit turns them
into `IRRewrite`s, so that they can be committed or rolled back just like any
other rewrite. This simplifies the internal state of the dialect conversion.
Overview of changes:
* Add two new rewrite classes: `ReplaceBlockArgRewrite` and
`ReplaceOperationRewrite`. Remove the `OpReplacement` helper class; it is now
part of `ReplaceOperationRewrite`.
* Simplify `RewriterState`: `numReplacements` and `numArgReplacements` are no
longer needed. (Now being kept track of by `numRewrites`.)
* Add `IRRewrite::cleanup`. Operations should not be erased in `commit` because
they may still be referenced in other internal state of the dialect conversion
(`mapping`). Detaching operations is fine.
* `trackedOps` are now updated during the "commit" phase instead of after
applying all rewrites.
>From 8405efca0845d39d01534505fde3c376ffb21fc5 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 14 Feb 2024 16:27:53 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn op/block arg replacements into
`IRRewrite`s
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be commited (upon success) or rolled
back (upon failure).
Until now, op replacements and block argument replacements were kept track in
separate data structures inside the dialect conversion. This commit turns them
into `IRRewrite`s, so that they can be committed or rolled back just like any
other rewrite. This simplifies the internal state of the dialect conversion.
Overview of changes:
* Add two new rewrite classes: `ReplaceBlockArgRewrite` and
`ReplaceOperationRewrite`. Remove the `OpReplacement` helper class; it is now
part of `ReplaceOperationRewrite`.
* Simplify `RewriterState`: `numReplacements` and `numArgReplacements` are no
longer needed. (Now being kept track of by `numRewrites`.)
* Add `IRRewrite::cleanup`. Operations should not be erased in `commit` because
they may still be referenced in other internal state of the dialect conversion
(`mapping`). Detaching operations is fine.
---
.../Transforms/Utils/DialectConversion.cpp| 297 ++
1 file changed, 159 insertions(+), 138 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index b2baa88879b6e9..a07c8a56822de5 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -153,14 +153,12 @@ namespace {
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numReplacements, unsigned numArgReplacements,
unsigned numRewrites, unsigned numIgnoredOperations,
unsigned numErased)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
-numReplacements(numReplacements),
-numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
+numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
+numErased(numErased) {}
/// The current number of created operations.
unsigned numCreatedOps;
@@ -168,12 +166,6 @@ struct RewriterState {
/// The current number of unresolved materializations.
unsigned numUnresolvedMaterializations;
- /// The current number of replacements queued.
- unsigned numReplacements;
-
- /// The current number of argument replacements queued.
- unsigned numArgReplacements;
-
/// The current number of rewrites performed.
unsigned numRewrites;
@@ -184,20 +176,6 @@ struct RewriterState {
unsigned numErased;
};
-//===--===//
-// OpReplacement
-
-/// This class represents one requested operation replacement via 'replaceOp'
or
-/// 'eraseOp`.
-struct OpReplacement {
- OpReplacement(const TypeConverter *converter = nullptr)
- : converter(converter) {}
-
- /// An optional type converter that can be used to materialize conversions
- /// between the new and old values if necessary.
- const TypeConverter *converter;
-};
-
//===--===//
// UnresolvedMaterialization
@@ -318,8 +296,10 @@ class IRRewrite {
MoveBlock,
SplitBlock,
BlockTypeConversion,
+ReplaceBlockArg,
MoveOperation,
-ModifyOpera
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op creation into `IRRewrite` (PR #81759)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/81759
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be committed (upon success) or
rolled back (upon failure).
Until now, the dialect conversion kept track of "op creation" in separate
internal data structures. This commit turns "op creation" into an `IRRewrite`
that can be committed and rolled back just like any other rewrite. This commit
simplifies the internal state of the dialect conversion.
>From 572d77c023455993be4861f5207bfd87b94124a3 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 14 Feb 2024 16:32:28 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn op creation into `IRRewrite`
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be commited (upon success) or rolled
back (upon failure).
Until now, the dialect conversion kept track of "op creation" in separate
internal data structures. This commit turns "op creation" into an `IRRewrite`
that can be committed and rolled back just like any other rewrite. This commit
simplifies the internal state of the dialect conversion.
---
.../Transforms/Utils/DialectConversion.cpp| 104 +++---
1 file changed, 66 insertions(+), 38 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index a07c8a56822de5..2bb56d5df1ebd3 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,17 +152,12 @@ namespace {
/// This class contains a snapshot of the current conversion rewriter state.
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
- RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numErased)
- : numCreatedOps(numCreatedOps),
-numUnresolvedMaterializations(numUnresolvedMaterializations),
+ RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
+unsigned numIgnoredOperations, unsigned numErased)
+ : numUnresolvedMaterializations(numUnresolvedMaterializations),
numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
numErased(numErased) {}
- /// The current number of created operations.
- unsigned numCreatedOps;
-
/// The current number of unresolved materializations.
unsigned numUnresolvedMaterializations;
@@ -299,7 +294,8 @@ class IRRewrite {
ReplaceBlockArg,
MoveOperation,
ModifyOperation,
-ReplaceOperation
+ReplaceOperation,
+CreateOperation
};
virtual ~IRRewrite() = default;
@@ -372,7 +368,11 @@ class CreateBlockRewrite : public BlockRewrite {
auto &blockOps = block->getOperations();
while (!blockOps.empty())
blockOps.remove(blockOps.begin());
-eraseBlock(block);
+if (block->getParent()) {
+ eraseBlock(block);
+} else {
+ delete block;
+}
}
};
@@ -602,7 +602,7 @@ class OperationRewrite : public IRRewrite {
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() >= Kind::MoveOperation &&
- rewrite->getKind() <= Kind::ReplaceOperation;
+ rewrite->getKind() <= Kind::CreateOperation;
}
protected:
@@ -708,6 +708,19 @@ class ReplaceOperationRewrite : public OperationRewrite {
/// 1->N conversion of some kind.
bool changedResults;
};
+
+class CreateOperationRewrite : public OperationRewrite {
+public:
+ CreateOperationRewrite(ConversionPatternRewriterImpl &rewriterImpl,
+ Operation *op)
+ : OperationRewrite(Kind::CreateOperation, rewriterImpl, op) {}
+
+ static bool classof(const IRRewrite *rewrite) {
+return rewrite->getKind() == Kind::CreateOperation;
+ }
+
+ void rollback() override;
+};
} // namespace
/// Return "true" if there is an operation rewrite that matches the specified
@@ -925,9 +938,6 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
// replacing a value with one of a different type.
ConversionValueMapping mapping;
- /// Ordered vector of all of the newly created operations during conversion.
- SmallVector createdOps;
-
/// Ordered vector of all unresolved type conversion materializations during
/// conversion.
SmallVector unresolvedMaterializations;
@@ -1110,7 +1120,18 @@ void ReplaceOperationRewrite::rollback() {
void ReplaceOperationRewrite::cleanup() { eraseOp(op); }
+void CreateOperationRewrite::rollback() {
+ for (Region ®ion : op->getRegions()) {
+while (!region.getBlocks().empty())
+ region.getBlocks().remove(region.getBlocks().begin());
+ }
+ op->dropAllUses();
+ eraseOp(op);
+}
+
void ConversionPatternRewriterImpl::detachNestedAndErase(
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Simplify `ArgConverter` state (PR #81462)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81462
>From a79501ebced4a3410c3a28c6555973bb45156e76 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 14 Feb 2024 16:20:30 +
Subject: [PATCH] [mlir][Transforms][NFC] Simplify `ArgConverter` state
* When converting a block signature, `ArgConverter` creates a new block with
the new signature and moves all operation from the old block to the new block.
The new block is temporarily inserted into a region that is stored in
`regionMapping`. The old block is not yet deleted, so that the conversion can
be rolled back. `regionMapping` is not needed. Instead of moving the old block
to a temporary region, it can just be unlinked. Block erasures are handles in
the same way in the dialect conversion.
* `regionToConverter` is a mapping from regions to type converter. That field
is never accessed within `ArgConverter`. It should be stored in
`ConversionPatternRewriterImpl` instead.
---
.../Transforms/Utils/DialectConversion.cpp| 79 ++-
1 file changed, 22 insertions(+), 57 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 5206a65608ba14..67b076b295eae8 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -343,23 +343,6 @@ struct ArgConverter {
const TypeConverter *converter;
};
- /// Return if the signature of the given block has already been converted.
- bool hasBeenConverted(Block *block) const {
-return conversionInfo.count(block) || convertedBlocks.count(block);
- }
-
- /// Set the type converter to use for the given region.
- void setConverter(Region *region, const TypeConverter *typeConverter) {
-assert(typeConverter && "expected valid type converter");
-regionToConverter[region] = typeConverter;
- }
-
- /// Return the type converter to use for the given region, or null if there
- /// isn't one.
- const TypeConverter *getConverter(Region *region) {
-return regionToConverter.lookup(region);
- }
-
//======//
// Rewrite Application
//======//
@@ -409,24 +392,10 @@ struct ArgConverter {
ConversionValueMapping &mapping,
SmallVectorImpl &argReplacements);
- /// Insert a new conversion into the cache.
- void insertConversion(Block *newBlock, ConvertedBlockInfo &&info);
-
/// A collection of blocks that have had their arguments converted. This is a
/// map from the new replacement block, back to the original block.
llvm::MapVector conversionInfo;
- /// The set of original blocks that were converted.
- DenseSet convertedBlocks;
-
- /// A mapping from valid regions, to those containing the original blocks of
a
- /// conversion.
- DenseMap> regionMapping;
-
- /// A mapping of regions to type converters that should be used when
- /// converting the arguments of blocks within that region.
- DenseMap regionToConverter;
-
/// The pattern rewriter to use when materializing conversions.
PatternRewriter &rewriter;
@@ -474,12 +443,12 @@ void ArgConverter::discardRewrites(Block *block) {
block->getArgument(i).dropAllUses();
block->replaceAllUsesWith(origBlock);
- // Move the operations back the original block and the delete the new block.
+ // Move the operations back the original block, move the original block back
+ // into its original location and the delete the new block.
origBlock->getOperations().splice(origBlock->end(), block->getOperations());
- origBlock->moveBefore(block);
+ block->getParent()->getBlocks().insert(Region::iterator(block), origBlock);
block->erase();
- convertedBlocks.erase(origBlock);
conversionInfo.erase(it);
}
@@ -510,6 +479,9 @@ void ArgConverter::applyRewrites(ConversionValueMapping
&mapping) {
mapping.lookupOrDefault(castValue, origArg.getType()));
}
}
+
+delete origBlock;
+blockInfo.origBlock = nullptr;
}
}
@@ -572,9 +544,11 @@ FailureOr ArgConverter::convertSignature(
Block *block, const TypeConverter *converter,
ConversionValueMapping &mapping,
SmallVectorImpl &argReplacements) {
- // Check if the block was already converted. If the block is detached,
- // conservatively assume it is going to be deleted.
- if (hasBeenConverted(block) || !block->getParent())
+ // Check if the block was already converted.
+ // * If the block is mapped in `conversionInfo`, it is a converted block.
+ // * If the block is detached, conservatively assume that it is going to be
+ // deleted; it is likely the old block (before it was converted).
+ if (conversionInfo.count(block) || !block->getParent())
return block;
// If a converter wasn't provided, and the block wasn't already converted,
// there is nothing we can do
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn block type conversion into `IRRewrite` (PR #81756)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81756
>From dcd13b85b50846d84850388e2bbed74c0aa54e62 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 14 Feb 2024 16:23:29 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn block type convertion into
`IRRewrite`
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be commited (upon success) or rolled
back (upon failure).
Until now, the signature conversion of a block was only a "partial" IR rewrite.
Rollbacks were triggered via `BlockTypeConversionRewrite::rollback`, but there
was no `BlockTypeConversionRewrite::commit` equivalent.
Overview of changes:
* Remove `ArgConverter`, an internal helper class that kept track of all block
type conversions. There is now a separate `BlockTypeConversionRewrite` for each
block type conversion.
* No more special handling for block type conversions. They are now normal "IR
rewrites", just like "block creation" or "block movement". In particular,
trigger "commits" of block type conversion via
`BlockTypeConversionRewrite::commit`.
* Remove `ArgConverter::notifyOpRemoved`. This function was used to inform the
`ArgConverter` that an operation was erased, to prevent a double-free of
operations in certain situations. It would be unpractical to add a
`notifyOpRemoved` API to `IRRewrite`. Instead, erasing ops/block should go
through a new `SingleEraseRewriter` (that is owned by the
`ConversionPatternRewriterImpl`) if there is chance of double-free. This
rewriter ignores `eraseOp`/`eraseBlock` if the op/block was already freed.
---
.../Transforms/Utils/DialectConversion.cpp| 810 --
1 file changed, 376 insertions(+), 434 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 67b076b295eae8..b2baa88879b6e9 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,12 +154,13 @@ namespace {
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
unsigned numReplacements, unsigned numArgReplacements,
-unsigned numRewrites, unsigned numIgnoredOperations)
+unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
numReplacements(numReplacements),
numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations) {}
+numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
/// The current number of created operations.
unsigned numCreatedOps;
@@ -178,6 +179,9 @@ struct RewriterState {
/// The current number of ignored operations.
unsigned numIgnoredOperations;
+
+ /// The current number of erased operations/blocks.
+ unsigned numErased;
};
//===--===//
@@ -292,374 +296,6 @@ static Value buildUnresolvedTargetMaterialization(
outputType, outputType, converter, unresolvedMaterializations);
}
-//===--===//
-// ArgConverter
-//===--===//
-namespace {
-/// This class provides a simple interface for converting the types of block
-/// arguments. This is done by creating a new block that contains the new legal
-/// types and extracting the block that contains the old illegal types to allow
-/// for undoing pending rewrites in the case of failure.
-struct ArgConverter {
- ArgConverter(
- PatternRewriter &rewriter,
- SmallVectorImpl &unresolvedMaterializations)
- : rewriter(rewriter),
-unresolvedMaterializations(unresolvedMaterializations) {}
-
- /// This structure contains the information pertaining to an argument that
has
- /// been converted.
- struct ConvertedArgInfo {
-ConvertedArgInfo(unsigned newArgIdx, unsigned newArgSize,
- Value castValue = nullptr)
-: newArgIdx(newArgIdx), newArgSize(newArgSize), castValue(castValue) {}
-
-/// The start index of in the new argument list that contains arguments
that
-/// replace the original.
-unsigned newArgIdx;
-
-/// The number of arguments that replaced the original argument.
-unsigned newArgSize;
-
-/// The cast value that was created to cast from the new arguments to the
-/// old. This only used if 'newArgSize' > 1.
-Value castValue;
- };
-
- /// This structure contains information pertaining to a block that has had
its
- /// signature converted.
- struct ConvertedBlockInfo {
-ConvertedBlockInfo(Block *origBlock, const TypeConverter *converter)
-
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op/block arg replacements into `IRRewrite`s (PR #81757)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81757
>From 613a6162af4ac07066dbcf87b580a085eca5a47a Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 14 Feb 2024 16:27:53 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn op/block arg replacements into
`IRRewrite`s
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be commited (upon success) or rolled
back (upon failure).
Until now, op replacements and block argument replacements were kept track in
separate data structures inside the dialect conversion. This commit turns them
into `IRRewrite`s, so that they can be committed or rolled back just like any
other rewrite. This simplifies the internal state of the dialect conversion.
Overview of changes:
* Add two new rewrite classes: `ReplaceBlockArgRewrite` and
`ReplaceOperationRewrite`. Remove the `OpReplacement` helper class; it is now
part of `ReplaceOperationRewrite`.
* Simplify `RewriterState`: `numReplacements` and `numArgReplacements` are no
longer needed. (Now being kept track of by `numRewrites`.)
* Add `IRRewrite::cleanup`. Operations should not be erased in `commit` because
they may still be referenced in other internal state of the dialect conversion
(`mapping`). Detaching operations is fine.
---
.../Transforms/Utils/DialectConversion.cpp| 297 ++
1 file changed, 159 insertions(+), 138 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index b2baa88879b6e9..a07c8a56822de5 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -153,14 +153,12 @@ namespace {
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numReplacements, unsigned numArgReplacements,
unsigned numRewrites, unsigned numIgnoredOperations,
unsigned numErased)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
-numReplacements(numReplacements),
-numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
+numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
+numErased(numErased) {}
/// The current number of created operations.
unsigned numCreatedOps;
@@ -168,12 +166,6 @@ struct RewriterState {
/// The current number of unresolved materializations.
unsigned numUnresolvedMaterializations;
- /// The current number of replacements queued.
- unsigned numReplacements;
-
- /// The current number of argument replacements queued.
- unsigned numArgReplacements;
-
/// The current number of rewrites performed.
unsigned numRewrites;
@@ -184,20 +176,6 @@ struct RewriterState {
unsigned numErased;
};
-//===--===//
-// OpReplacement
-
-/// This class represents one requested operation replacement via 'replaceOp'
or
-/// 'eraseOp`.
-struct OpReplacement {
- OpReplacement(const TypeConverter *converter = nullptr)
- : converter(converter) {}
-
- /// An optional type converter that can be used to materialize conversions
- /// between the new and old values if necessary.
- const TypeConverter *converter;
-};
-
//===--===//
// UnresolvedMaterialization
@@ -318,8 +296,10 @@ class IRRewrite {
MoveBlock,
SplitBlock,
BlockTypeConversion,
+ReplaceBlockArg,
MoveOperation,
-ModifyOperation
+ModifyOperation,
+ReplaceOperation
};
virtual ~IRRewrite() = default;
@@ -330,6 +310,12 @@ class IRRewrite {
/// Commit the rewrite.
virtual void commit() {}
+ /// Cleanup operations. Operations may be unlinked from their blocks during
+ /// the commit/rollback phase, but they must not be erased yet. This is
+ /// because internal dialect conversion state (such as `mapping`) may still
+ /// be using them. Operations must be erased during cleanup.
+ virtual void cleanup() {}
+
/// Erase the given op (unless it was already erased).
void eraseOp(Operation *op);
@@ -356,7 +342,7 @@ class BlockRewrite : public IRRewrite {
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() >= Kind::CreateBlock &&
- rewrite->getKind() <= Kind::BlockTypeConversion;
+ rewrite->getKind() <= Kind::ReplaceBlockArg;
}
protected:
@@ -424,6 +410,8 @@ class EraseBlockRewrite : public BlockRewrite {
void commit() override {
// Erase the block.
assert(block && "expected block");
+assert(block->empty() && "expected empty block");
+bloc
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op creation into `IRRewrite` (PR #81759)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81759
>From 3873a3e96d0582c0ecf24335dad01560b61e8d90 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 14 Feb 2024 16:32:28 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn op creation into `IRRewrite`
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be commited (upon success) or rolled
back (upon failure).
Until now, the dialect conversion kept track of "op creation" in separate
internal data structures. This commit turns "op creation" into an `IRRewrite`
that can be committed and rolled back just like any other rewrite. This commit
simplifies the internal state of the dialect conversion.
---
.../Transforms/Utils/DialectConversion.cpp| 104 +++---
1 file changed, 66 insertions(+), 38 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index a07c8a56822de5..2bb56d5df1ebd3 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,17 +152,12 @@ namespace {
/// This class contains a snapshot of the current conversion rewriter state.
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
- RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numErased)
- : numCreatedOps(numCreatedOps),
-numUnresolvedMaterializations(numUnresolvedMaterializations),
+ RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
+unsigned numIgnoredOperations, unsigned numErased)
+ : numUnresolvedMaterializations(numUnresolvedMaterializations),
numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
numErased(numErased) {}
- /// The current number of created operations.
- unsigned numCreatedOps;
-
/// The current number of unresolved materializations.
unsigned numUnresolvedMaterializations;
@@ -299,7 +294,8 @@ class IRRewrite {
ReplaceBlockArg,
MoveOperation,
ModifyOperation,
-ReplaceOperation
+ReplaceOperation,
+CreateOperation
};
virtual ~IRRewrite() = default;
@@ -372,7 +368,11 @@ class CreateBlockRewrite : public BlockRewrite {
auto &blockOps = block->getOperations();
while (!blockOps.empty())
blockOps.remove(blockOps.begin());
-eraseBlock(block);
+if (block->getParent()) {
+ eraseBlock(block);
+} else {
+ delete block;
+}
}
};
@@ -602,7 +602,7 @@ class OperationRewrite : public IRRewrite {
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() >= Kind::MoveOperation &&
- rewrite->getKind() <= Kind::ReplaceOperation;
+ rewrite->getKind() <= Kind::CreateOperation;
}
protected:
@@ -708,6 +708,19 @@ class ReplaceOperationRewrite : public OperationRewrite {
/// 1->N conversion of some kind.
bool changedResults;
};
+
+class CreateOperationRewrite : public OperationRewrite {
+public:
+ CreateOperationRewrite(ConversionPatternRewriterImpl &rewriterImpl,
+ Operation *op)
+ : OperationRewrite(Kind::CreateOperation, rewriterImpl, op) {}
+
+ static bool classof(const IRRewrite *rewrite) {
+return rewrite->getKind() == Kind::CreateOperation;
+ }
+
+ void rollback() override;
+};
} // namespace
/// Return "true" if there is an operation rewrite that matches the specified
@@ -925,9 +938,6 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
// replacing a value with one of a different type.
ConversionValueMapping mapping;
- /// Ordered vector of all of the newly created operations during conversion.
- SmallVector createdOps;
-
/// Ordered vector of all unresolved type conversion materializations during
/// conversion.
SmallVector unresolvedMaterializations;
@@ -1110,7 +1120,18 @@ void ReplaceOperationRewrite::rollback() {
void ReplaceOperationRewrite::cleanup() { eraseOp(op); }
+void CreateOperationRewrite::rollback() {
+ for (Region ®ion : op->getRegions()) {
+while (!region.getBlocks().empty())
+ region.getBlocks().remove(region.getBlocks().begin());
+ }
+ op->dropAllUses();
+ eraseOp(op);
+}
+
void ConversionPatternRewriterImpl::detachNestedAndErase(Operation *op) {
+ // if (erasedIR.erasedOps.contains(op)) return;
+
for (Region ®ion : op->getRegions()) {
for (Block &block : region.getBlocks()) {
while (!block.getOperations().empty())
@@ -1127,8 +1148,6 @@ void ConversionPatternRewriterImpl::discardRewrites() {
// Remove any newly created ops.
for (UnresolvedMaterialization &materialization : unresolvedMaterializations)
detachNestedAndErase(materialization.getOp());
- for (auto *op : llvm:
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC][WIP] Turn unresolved materializations into `IRRewrite`s (PR #81761)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/81761
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be committed (upon success) or
rolled back (upon failure).
This commit turns the creation of unresolved materializations
(`unrealized_conversion_cast`) into `IRRewrite` objects. After this commit, all
steps in `applyRewrites` and `discardRewrites` are calls to `IRRewrite::commit`
and `IRRewrite::rollback`.
WIP, but uploading already to show where this chain of refactorings is going.
>From 7b64a0060de5717bf4b407dc4918eed49ed16e57 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 14 Feb 2024 16:47:46 +
Subject: [PATCH] [WIP] UnresolvedMaterialization
BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
.../Transforms/Utils/DialectConversion.cpp| 374 +-
1 file changed, 179 insertions(+), 195 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 2bb56d5df1ebd3..83cc05b9e1cd98 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,15 +152,11 @@ namespace {
/// This class contains a snapshot of the current conversion rewriter state.
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
- RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
-unsigned numIgnoredOperations, unsigned numErased)
- : numUnresolvedMaterializations(numUnresolvedMaterializations),
-numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
+ RewriterState(unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
+ : numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
numErased(numErased) {}
- /// The current number of unresolved materializations.
- unsigned numUnresolvedMaterializations;
-
/// The current number of rewrites performed.
unsigned numRewrites;
@@ -171,109 +167,10 @@ struct RewriterState {
unsigned numErased;
};
-//===--===//
-// UnresolvedMaterialization
-
-/// This class represents an unresolved materialization, i.e. a materialization
-/// that was inserted during conversion that needs to be legalized at the end
of
-/// the conversion process.
-class UnresolvedMaterialization {
-public:
- /// The type of materialization.
- enum Kind {
-/// This materialization materializes a conversion for an illegal block
-/// argument type, to a legal one.
-Argument,
-
-/// This materialization materializes a conversion from an illegal type to
a
-/// legal one.
-Target
- };
-
- UnresolvedMaterialization(UnrealizedConversionCastOp op = nullptr,
-const TypeConverter *converter = nullptr,
-Kind kind = Target, Type origOutputType = nullptr)
- : op(op), converterAndKind(converter, kind),
-origOutputType(origOutputType) {}
-
- /// Return the temporary conversion operation inserted for this
- /// materialization.
- UnrealizedConversionCastOp getOp() const { return op; }
-
- /// Return the type converter of this materialization (which may be null).
- const TypeConverter *getConverter() const {
-return converterAndKind.getPointer();
- }
-
- /// Return the kind of this materialization.
- Kind getKind() const { return converterAndKind.getInt(); }
-
- /// Set the kind of this materialization.
- void setKind(Kind kind) { converterAndKind.setInt(kind); }
-
- /// Return the original illegal output type of the input values.
- Type getOrigOutputType() const { return origOutputType; }
-
-private:
- /// The unresolved materialization operation created during conversion.
- UnrealizedConversionCastOp op;
-
- /// The corresponding type converter to use when resolving this
- /// materialization, and the kind of this materialization.
- llvm::PointerIntPair converterAndKind;
-
- /// The original output type. This is only used for argument conversions.
- Type origOutputType;
-};
-} // namespace
-
-/// Build an unresolved materialization operation given an output type and set
-/// of input operands.
-static Value buildUnresolvedMaterialization(
-UnresolvedMaterialization::Kind kind, Block *insertBlock,
-Block::iterator insertPt, Location loc, ValueRange inputs, Type outputType,
-Type origOutputType, const TypeConverter *converter,
-SmallVectorImpl &unresolvedMaterializations) {
- // Avoid materializing an unnecessary cast.
- if (inputs.size() == 1 && inputs.front().getType() == outputType)
-return inputs.front();
-
- // Create an unresolved materialization. We use a new OpBuilder to avoid
- // tracking the materialization like we do for other operations
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn block type conversion into `IRRewrite` (PR #81756)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/81756 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Simplify `ArgConverter` state (PR #81462)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81462
>From a7fffc3e18588d1112411a03936e41a2931cefec Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Fri, 16 Feb 2024 15:11:41 +
Subject: [PATCH] [mlir][Transforms][NFC] Simplify `ArgConverter` state
* When converting a block signature, `ArgConverter` creates a new block with
the new signature and moves all operation from the old block to the new block.
The new block is temporarily inserted into a region that is stored in
`regionMapping`. The old block is not yet deleted, so that the conversion can
be rolled back. `regionMapping` is not needed. Instead of moving the old block
to a temporary region, it can just be unlinked. Block erasures are handles in
the same way in the dialect conversion.
* `regionToConverter` is a mapping from regions to type converter. That field
is never accessed within `ArgConverter`. It should be stored in
`ConversionPatternRewriterImpl` instead.
---
.../Transforms/Utils/DialectConversion.cpp| 79 ++-
1 file changed, 22 insertions(+), 57 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 673bd0383809cb..35028001a03dd9 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -343,23 +343,6 @@ struct ArgConverter {
const TypeConverter *converter;
};
- /// Return if the signature of the given block has already been converted.
- bool hasBeenConverted(Block *block) const {
-return conversionInfo.count(block) || convertedBlocks.count(block);
- }
-
- /// Set the type converter to use for the given region.
- void setConverter(Region *region, const TypeConverter *typeConverter) {
-assert(typeConverter && "expected valid type converter");
-regionToConverter[region] = typeConverter;
- }
-
- /// Return the type converter to use for the given region, or null if there
- /// isn't one.
- const TypeConverter *getConverter(Region *region) {
-return regionToConverter.lookup(region);
- }
-
//======//
// Rewrite Application
//======//
@@ -409,24 +392,10 @@ struct ArgConverter {
ConversionValueMapping &mapping,
SmallVectorImpl &argReplacements);
- /// Insert a new conversion into the cache.
- void insertConversion(Block *newBlock, ConvertedBlockInfo &&info);
-
/// A collection of blocks that have had their arguments converted. This is a
/// map from the new replacement block, back to the original block.
llvm::MapVector conversionInfo;
- /// The set of original blocks that were converted.
- DenseSet convertedBlocks;
-
- /// A mapping from valid regions, to those containing the original blocks of
a
- /// conversion.
- DenseMap> regionMapping;
-
- /// A mapping of regions to type converters that should be used when
- /// converting the arguments of blocks within that region.
- DenseMap regionToConverter;
-
/// The pattern rewriter to use when materializing conversions.
PatternRewriter &rewriter;
@@ -474,12 +443,12 @@ void ArgConverter::discardRewrites(Block *block) {
block->getArgument(i).dropAllUses();
block->replaceAllUsesWith(origBlock);
- // Move the operations back the original block and the delete the new block.
+ // Move the operations back the original block, move the original block back
+ // into its original location and the delete the new block.
origBlock->getOperations().splice(origBlock->end(), block->getOperations());
- origBlock->moveBefore(block);
+ block->getParent()->getBlocks().insert(Region::iterator(block), origBlock);
block->erase();
- convertedBlocks.erase(origBlock);
conversionInfo.erase(it);
}
@@ -510,6 +479,9 @@ void ArgConverter::applyRewrites(ConversionValueMapping
&mapping) {
mapping.lookupOrDefault(castValue, origArg.getType()));
}
}
+
+delete origBlock;
+blockInfo.origBlock = nullptr;
}
}
@@ -572,9 +544,11 @@ FailureOr ArgConverter::convertSignature(
Block *block, const TypeConverter *converter,
ConversionValueMapping &mapping,
SmallVectorImpl &argReplacements) {
- // Check if the block was already converted. If the block is detached,
- // conservatively assume it is going to be deleted.
- if (hasBeenConverted(block) || !block->getParent())
+ // Check if the block was already converted.
+ // * If the block is mapped in `conversionInfo`, it is a converted block.
+ // * If the block is detached, conservatively assume that it is going to be
+ // deleted; it is likely the old block (before it was converted).
+ if (conversionInfo.count(block) || !block->getParent())
return block;
// If a converter wasn't provided, and the block wasn't already converted,
// there is nothing we can do
[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Improve signature conversion API (PR #81997)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/81997
This commit improves the block signature conversion API of the dialect
conversion.
There is the following comment in `ArgConverter::applySignatureConversion`:
```
// If no arguments are being changed or added, there is nothing to do.
```
However, the implementation actually used to replace a block with a new block
even if the block argument types do not change (i.e., there is "nothing to
do"). This is fixed in this commit. The documentation of the public
`ConversionPatternRewriter` API is updated accordingly.
This commit also removes a check that used to *sometimes* skip a block
signature conversion if the block was already converted. This is not consistent
with the public `ConversionPatternRewriter` API; block should always be
converted, regardless of whether they were already converted or not.
Block signature conversion also used to be silently skipped when the specified
block was detached. Instead of silently skipping, an assertion is triggered.
Attempting to convert a detached block (which is likely an erased block) is
invalid API usage.
>From 5dc79f6af9ac00a61767062980b13eb4ae8d2571 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Fri, 16 Feb 2024 15:13:37 +
Subject: [PATCH] [mlir][Transforms] Dialect conversion: Improve signature
conversion API
This commit improves the block signature conversion API of the dialect
conversion.
There is the following comment in `ArgConverter::applySignatureConversion`:
```
// If no arguments are being changed or added, there is nothing to do.
```
However, the implementation actually used to replace a block with a new block
even if the block argument types do not change (i.e., there is "nothing to
do"). This is fixed in this commit. The documentation of the public
`ConversionPatternRewriter` API is updated accordingly.
This commit also removes a check that used to *sometimes* skip a block
signature conversion if the block was already converted. This is not consistent
with the public `ConversionPatternRewriter` API; block should always be
converted, regardless of whether they were already converted or not.
Block signature conversion also used to be silently skipped when the specified
block was detached. Instead of silently skipping, an assertion is triggered.
Attempting to convert a detached block (which is likely an erased block) is
invalid API usage.
---
mlir/include/mlir/Transforms/DialectConversion.h | 12 +---
mlir/lib/Transforms/Utils/DialectConversion.cpp | 10 +++---
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index 0d7722aa07ee38..2575be4cdea1ac 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -663,6 +663,8 @@ class ConversionPatternRewriter final : public
PatternRewriter {
/// Apply a signature conversion to the entry block of the given region. This
/// replaces the entry block with a new block containing the updated
/// signature. The new entry block to the region is returned for convenience.
+ /// If no block argument types are changing, the entry original block will be
+ /// left in place and returned.
///
/// If provided, `converter` will be used for any materializations.
Block *
@@ -671,8 +673,11 @@ class ConversionPatternRewriter final : public
PatternRewriter {
const TypeConverter *converter = nullptr);
/// Convert the types of block arguments within the given region. This
- /// replaces each block with a new block containing the updated signature.
The
- /// entry block may have a special conversion if `entryConversion` is
+ /// replaces each block with a new block containing the updated signature. If
+ /// an updated signature would match the current signature, the respective
+ /// block is left in place as is.
+ ///
+ /// The entry block may have a special conversion if `entryConversion` is
/// provided. On success, the new entry block to the region is returned for
/// convenience. Otherwise, failure is returned.
FailureOr convertRegionTypes(
@@ -681,7 +686,8 @@ class ConversionPatternRewriter final : public
PatternRewriter {
/// Convert the types of block arguments within the given region except for
/// the entry region. This replaces each non-entry block with a new block
- /// containing the updated signature.
+ /// containing the updated signature. If an updated signature would match the
+ /// current signature, the respective block is left in place as is.
///
/// If special conversion behavior is needed for the non-entry blocks (for
/// example, we need to convert only a subset of a BB arguments), such
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn block type conversion into `IRRewrite` (PR #81756)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81756
>From 5dc79f6af9ac00a61767062980b13eb4ae8d2571 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Fri, 16 Feb 2024 15:13:37 +
Subject: [PATCH 1/2] [mlir][Transforms] Dialect conversion: Improve signature
conversion API
This commit improves the block signature conversion API of the dialect
conversion.
There is the following comment in `ArgConverter::applySignatureConversion`:
```
// If no arguments are being changed or added, there is nothing to do.
```
However, the implementation actually used to replace a block with a new block
even if the block argument types do not change (i.e., there is "nothing to
do"). This is fixed in this commit. The documentation of the public
`ConversionPatternRewriter` API is updated accordingly.
This commit also removes a check that used to *sometimes* skip a block
signature conversion if the block was already converted. This is not consistent
with the public `ConversionPatternRewriter` API; block should always be
converted, regardless of whether they were already converted or not.
Block signature conversion also used to be silently skipped when the specified
block was detached. Instead of silently skipping, an assertion is triggered.
Attempting to convert a detached block (which is likely an erased block) is
invalid API usage.
---
mlir/include/mlir/Transforms/DialectConversion.h | 12 +---
mlir/lib/Transforms/Utils/DialectConversion.cpp | 10 +++---
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index 0d7722aa07ee38..2575be4cdea1ac 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -663,6 +663,8 @@ class ConversionPatternRewriter final : public
PatternRewriter {
/// Apply a signature conversion to the entry block of the given region. This
/// replaces the entry block with a new block containing the updated
/// signature. The new entry block to the region is returned for convenience.
+ /// If no block argument types are changing, the entry original block will be
+ /// left in place and returned.
///
/// If provided, `converter` will be used for any materializations.
Block *
@@ -671,8 +673,11 @@ class ConversionPatternRewriter final : public
PatternRewriter {
const TypeConverter *converter = nullptr);
/// Convert the types of block arguments within the given region. This
- /// replaces each block with a new block containing the updated signature.
The
- /// entry block may have a special conversion if `entryConversion` is
+ /// replaces each block with a new block containing the updated signature. If
+ /// an updated signature would match the current signature, the respective
+ /// block is left in place as is.
+ ///
+ /// The entry block may have a special conversion if `entryConversion` is
/// provided. On success, the new entry block to the region is returned for
/// convenience. Otherwise, failure is returned.
FailureOr convertRegionTypes(
@@ -681,7 +686,8 @@ class ConversionPatternRewriter final : public
PatternRewriter {
/// Convert the types of block arguments within the given region except for
/// the entry region. This replaces each non-entry block with a new block
- /// containing the updated signature.
+ /// containing the updated signature. If an updated signature would match the
+ /// current signature, the respective block is left in place as is.
///
/// If special conversion behavior is needed for the non-entry blocks (for
/// example, we need to convert only a subset of a BB arguments), such
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 35028001a03dd9..c16bb144efecf5 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -544,12 +544,8 @@ FailureOr ArgConverter::convertSignature(
Block *block, const TypeConverter *converter,
ConversionValueMapping &mapping,
SmallVectorImpl &argReplacements) {
- // Check if the block was already converted.
- // * If the block is mapped in `conversionInfo`, it is a converted block.
- // * If the block is detached, conservatively assume that it is going to be
- // deleted; it is likely the old block (before it was converted).
- if (conversionInfo.count(block) || !block->getParent())
-return block;
+ assert(block->getParent() && "cannot convert signature of detached block");
+
// If a converter wasn't provided, and the block wasn't already converted,
// there is nothing we can do.
if (!converter)
@@ -570,7 +566,7 @@ Block *ArgConverter::applySignatureConversion(
// If no arguments are being changed or added, there is nothing to do.
unsigned origArgCount
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn block type conversion into `IRRewrite` (PR #81756)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/81756 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op/block arg replacements into `IRRewrite`s (PR #81757)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81757
>From b8d4cbd5e237dfc9fa6b7420b85a8e5de94f0725 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Fri, 16 Feb 2024 15:19:07 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn op/block arg replacements into
`IRRewrite`s
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be commited (upon success) or rolled
back (upon failure).
Until now, op replacements and block argument replacements were kept track in
separate data structures inside the dialect conversion. This commit turns them
into `IRRewrite`s, so that they can be committed or rolled back just like any
other rewrite. This simplifies the internal state of the dialect conversion.
Overview of changes:
* Add two new rewrite classes: `ReplaceBlockArgRewrite` and
`ReplaceOperationRewrite`. Remove the `OpReplacement` helper class; it is now
part of `ReplaceOperationRewrite`.
* Simplify `RewriterState`: `numReplacements` and `numArgReplacements` are no
longer needed. (Now being kept track of by `numRewrites`.)
* Add `IRRewrite::cleanup`. Operations should not be erased in `commit` because
they may still be referenced in other internal state of the dialect conversion
(`mapping`). Detaching operations is fine.
---
.../Transforms/Utils/DialectConversion.cpp| 291 +-
1 file changed, 153 insertions(+), 138 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 30133a14dbae56..21fd02fcd5c725 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -153,14 +153,12 @@ namespace {
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numReplacements, unsigned numArgReplacements,
unsigned numRewrites, unsigned numIgnoredOperations,
unsigned numErased)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
-numReplacements(numReplacements),
-numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
+numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
+numErased(numErased) {}
/// The current number of created operations.
unsigned numCreatedOps;
@@ -168,12 +166,6 @@ struct RewriterState {
/// The current number of unresolved materializations.
unsigned numUnresolvedMaterializations;
- /// The current number of replacements queued.
- unsigned numReplacements;
-
- /// The current number of argument replacements queued.
- unsigned numArgReplacements;
-
/// The current number of rewrites performed.
unsigned numRewrites;
@@ -184,20 +176,6 @@ struct RewriterState {
unsigned numErased;
};
-//===--===//
-// OpReplacement
-
-/// This class represents one requested operation replacement via 'replaceOp'
or
-/// 'eraseOp`.
-struct OpReplacement {
- OpReplacement(const TypeConverter *converter = nullptr)
- : converter(converter) {}
-
- /// An optional type converter that can be used to materialize conversions
- /// between the new and old values if necessary.
- const TypeConverter *converter;
-};
-
//===--===//
// UnresolvedMaterialization
@@ -318,8 +296,10 @@ class IRRewrite {
MoveBlock,
SplitBlock,
BlockTypeConversion,
+ReplaceBlockArg,
MoveOperation,
-ModifyOperation
+ModifyOperation,
+ReplaceOperation
};
virtual ~IRRewrite() = default;
@@ -330,6 +310,12 @@ class IRRewrite {
/// Commit the rewrite.
virtual void commit() {}
+ /// Cleanup operations. Operations may be unlinked from their blocks during
+ /// the commit/rollback phase, but they must not be erased yet. This is
+ /// because internal dialect conversion state (such as `mapping`) may still
+ /// be using them. Operations must be erased during cleanup.
+ virtual void cleanup() {}
+
Kind getKind() const { return kind; }
static bool classof(const IRRewrite *rewrite) { return true; }
@@ -356,7 +342,7 @@ class BlockRewrite : public IRRewrite {
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() >= Kind::CreateBlock &&
- rewrite->getKind() <= Kind::BlockTypeConversion;
+ rewrite->getKind() <= Kind::ReplaceBlockArg;
}
protected:
@@ -424,6 +410,8 @@ class EraseBlockRewrite : public BlockRewrite {
void commit() override {
// Erase the block.
assert(block && "expected block");
+assert(block->empty() && "expected empty b
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op creation into `IRRewrite` (PR #81759)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81759
>From 67010343f91c6808a18731f01d139db6cb36fde6 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Fri, 16 Feb 2024 15:21:48 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn op creation into `IRRewrite`
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be commited (upon success) or rolled
back (upon failure).
Until now, the dialect conversion kept track of "op creation" in separate
internal data structures. This commit turns "op creation" into an `IRRewrite`
that can be committed and rolled back just like any other rewrite. This commit
simplifies the internal state of the dialect conversion.
---
.../Transforms/Utils/DialectConversion.cpp| 104 +++---
1 file changed, 66 insertions(+), 38 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 21fd02fcd5c725..5b7ad4e7b8e281 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,17 +152,12 @@ namespace {
/// This class contains a snapshot of the current conversion rewriter state.
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
- RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numErased)
- : numCreatedOps(numCreatedOps),
-numUnresolvedMaterializations(numUnresolvedMaterializations),
+ RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
+unsigned numIgnoredOperations, unsigned numErased)
+ : numUnresolvedMaterializations(numUnresolvedMaterializations),
numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
numErased(numErased) {}
- /// The current number of created operations.
- unsigned numCreatedOps;
-
/// The current number of unresolved materializations.
unsigned numUnresolvedMaterializations;
@@ -299,7 +294,8 @@ class IRRewrite {
ReplaceBlockArg,
MoveOperation,
ModifyOperation,
-ReplaceOperation
+ReplaceOperation,
+CreateOperation
};
virtual ~IRRewrite() = default;
@@ -372,7 +368,11 @@ class CreateBlockRewrite : public BlockRewrite {
auto &blockOps = block->getOperations();
while (!blockOps.empty())
blockOps.remove(blockOps.begin());
-eraseBlock(block);
+if (block->getParent()) {
+ eraseBlock(block);
+} else {
+ delete block;
+}
}
};
@@ -602,7 +602,7 @@ class OperationRewrite : public IRRewrite {
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() >= Kind::MoveOperation &&
- rewrite->getKind() <= Kind::ReplaceOperation;
+ rewrite->getKind() <= Kind::CreateOperation;
}
protected:
@@ -708,6 +708,19 @@ class ReplaceOperationRewrite : public OperationRewrite {
/// 1->N conversion of some kind.
bool changedResults;
};
+
+class CreateOperationRewrite : public OperationRewrite {
+public:
+ CreateOperationRewrite(ConversionPatternRewriterImpl &rewriterImpl,
+ Operation *op)
+ : OperationRewrite(Kind::CreateOperation, rewriterImpl, op) {}
+
+ static bool classof(const IRRewrite *rewrite) {
+return rewrite->getKind() == Kind::CreateOperation;
+ }
+
+ void rollback() override;
+};
} // namespace
/// Return "true" if there is an operation rewrite that matches the specified
@@ -925,9 +938,6 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
// replacing a value with one of a different type.
ConversionValueMapping mapping;
- /// Ordered vector of all of the newly created operations during conversion.
- SmallVector createdOps;
-
/// Ordered vector of all unresolved type conversion materializations during
/// conversion.
SmallVector unresolvedMaterializations;
@@ -1110,7 +1120,18 @@ void ReplaceOperationRewrite::rollback() {
void ReplaceOperationRewrite::cleanup() { eraseOp(op); }
+void CreateOperationRewrite::rollback() {
+ for (Region ®ion : op->getRegions()) {
+while (!region.getBlocks().empty())
+ region.getBlocks().remove(region.getBlocks().begin());
+ }
+ op->dropAllUses();
+ eraseOp(op);
+}
+
void ConversionPatternRewriterImpl::detachNestedAndErase(Operation *op) {
+ // if (erasedIR.erasedOps.contains(op)) return;
+
for (Region ®ion : op->getRegions()) {
for (Block &block : region.getBlocks()) {
while (!block.getOperations().empty())
@@ -1127,8 +1148,6 @@ void ConversionPatternRewriterImpl::discardRewrites() {
// Remove any newly created ops.
for (UnresolvedMaterialization &materialization : unresolvedMaterializations)
detachNestedAndErase(materialization.getOp());
- for (auto *op : llvm:
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC][WIP] Turn unresolved materializations into `IRRewrite`s (PR #81761)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81761
>From ab0cd8c2d3b66b0a11bee128dc4e11e754e11a36 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Fri, 16 Feb 2024 15:24:42 +
Subject: [PATCH] [WIP] UnresolvedMaterialization
BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
.../Transforms/Utils/DialectConversion.cpp| 374 +-
1 file changed, 179 insertions(+), 195 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 5b7ad4e7b8e281..6fc9225568d028 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,15 +152,11 @@ namespace {
/// This class contains a snapshot of the current conversion rewriter state.
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
- RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
-unsigned numIgnoredOperations, unsigned numErased)
- : numUnresolvedMaterializations(numUnresolvedMaterializations),
-numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
+ RewriterState(unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
+ : numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
numErased(numErased) {}
- /// The current number of unresolved materializations.
- unsigned numUnresolvedMaterializations;
-
/// The current number of rewrites performed.
unsigned numRewrites;
@@ -171,109 +167,10 @@ struct RewriterState {
unsigned numErased;
};
-//===--===//
-// UnresolvedMaterialization
-
-/// This class represents an unresolved materialization, i.e. a materialization
-/// that was inserted during conversion that needs to be legalized at the end
of
-/// the conversion process.
-class UnresolvedMaterialization {
-public:
- /// The type of materialization.
- enum Kind {
-/// This materialization materializes a conversion for an illegal block
-/// argument type, to a legal one.
-Argument,
-
-/// This materialization materializes a conversion from an illegal type to
a
-/// legal one.
-Target
- };
-
- UnresolvedMaterialization(UnrealizedConversionCastOp op = nullptr,
-const TypeConverter *converter = nullptr,
-Kind kind = Target, Type origOutputType = nullptr)
- : op(op), converterAndKind(converter, kind),
-origOutputType(origOutputType) {}
-
- /// Return the temporary conversion operation inserted for this
- /// materialization.
- UnrealizedConversionCastOp getOp() const { return op; }
-
- /// Return the type converter of this materialization (which may be null).
- const TypeConverter *getConverter() const {
-return converterAndKind.getPointer();
- }
-
- /// Return the kind of this materialization.
- Kind getKind() const { return converterAndKind.getInt(); }
-
- /// Set the kind of this materialization.
- void setKind(Kind kind) { converterAndKind.setInt(kind); }
-
- /// Return the original illegal output type of the input values.
- Type getOrigOutputType() const { return origOutputType; }
-
-private:
- /// The unresolved materialization operation created during conversion.
- UnrealizedConversionCastOp op;
-
- /// The corresponding type converter to use when resolving this
- /// materialization, and the kind of this materialization.
- llvm::PointerIntPair converterAndKind;
-
- /// The original output type. This is only used for argument conversions.
- Type origOutputType;
-};
-} // namespace
-
-/// Build an unresolved materialization operation given an output type and set
-/// of input operands.
-static Value buildUnresolvedMaterialization(
-UnresolvedMaterialization::Kind kind, Block *insertBlock,
-Block::iterator insertPt, Location loc, ValueRange inputs, Type outputType,
-Type origOutputType, const TypeConverter *converter,
-SmallVectorImpl &unresolvedMaterializations) {
- // Avoid materializing an unnecessary cast.
- if (inputs.size() == 1 && inputs.front().getType() == outputType)
-return inputs.front();
-
- // Create an unresolved materialization. We use a new OpBuilder to avoid
- // tracking the materialization like we do for other operations.
- OpBuilder builder(insertBlock, insertPt);
- auto convertOp =
- builder.create(loc, outputType, inputs);
- unresolvedMaterializations.emplace_back(convertOp, converter, kind,
- origOutputType);
- return convertOp.getResult(0);
-}
-static Value buildUnresolvedArgumentMaterialization(
-PatternRewriter &rewriter, Location loc, ValueRange inputs,
-Type origOutputType, Type outputType, const TypeConverter *converter,
-SmallVectorImpl &unresolvedMaterial
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC][WIP] Turn unresolved materializations into `IRRewrite`s (PR #81761)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81761
>From 51a7e12b4e0e8ceee8429460592307adfad18a96 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Mon, 19 Feb 2024 10:29:29 +
Subject: [PATCH] [WIP] UnresolvedMaterialization
BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
.../Transforms/Utils/DialectConversion.cpp| 371 +-
1 file changed, 176 insertions(+), 195 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 5b7ad4e7b8e281..4ef26a739e4ea1 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,15 +152,11 @@ namespace {
/// This class contains a snapshot of the current conversion rewriter state.
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
- RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
-unsigned numIgnoredOperations, unsigned numErased)
- : numUnresolvedMaterializations(numUnresolvedMaterializations),
-numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
+ RewriterState(unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
+ : numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
numErased(numErased) {}
- /// The current number of unresolved materializations.
- unsigned numUnresolvedMaterializations;
-
/// The current number of rewrites performed.
unsigned numRewrites;
@@ -171,109 +167,10 @@ struct RewriterState {
unsigned numErased;
};
-//===--===//
-// UnresolvedMaterialization
-
-/// This class represents an unresolved materialization, i.e. a materialization
-/// that was inserted during conversion that needs to be legalized at the end
of
-/// the conversion process.
-class UnresolvedMaterialization {
-public:
- /// The type of materialization.
- enum Kind {
-/// This materialization materializes a conversion for an illegal block
-/// argument type, to a legal one.
-Argument,
-
-/// This materialization materializes a conversion from an illegal type to
a
-/// legal one.
-Target
- };
-
- UnresolvedMaterialization(UnrealizedConversionCastOp op = nullptr,
-const TypeConverter *converter = nullptr,
-Kind kind = Target, Type origOutputType = nullptr)
- : op(op), converterAndKind(converter, kind),
-origOutputType(origOutputType) {}
-
- /// Return the temporary conversion operation inserted for this
- /// materialization.
- UnrealizedConversionCastOp getOp() const { return op; }
-
- /// Return the type converter of this materialization (which may be null).
- const TypeConverter *getConverter() const {
-return converterAndKind.getPointer();
- }
-
- /// Return the kind of this materialization.
- Kind getKind() const { return converterAndKind.getInt(); }
-
- /// Set the kind of this materialization.
- void setKind(Kind kind) { converterAndKind.setInt(kind); }
-
- /// Return the original illegal output type of the input values.
- Type getOrigOutputType() const { return origOutputType; }
-
-private:
- /// The unresolved materialization operation created during conversion.
- UnrealizedConversionCastOp op;
-
- /// The corresponding type converter to use when resolving this
- /// materialization, and the kind of this materialization.
- llvm::PointerIntPair converterAndKind;
-
- /// The original output type. This is only used for argument conversions.
- Type origOutputType;
-};
-} // namespace
-
-/// Build an unresolved materialization operation given an output type and set
-/// of input operands.
-static Value buildUnresolvedMaterialization(
-UnresolvedMaterialization::Kind kind, Block *insertBlock,
-Block::iterator insertPt, Location loc, ValueRange inputs, Type outputType,
-Type origOutputType, const TypeConverter *converter,
-SmallVectorImpl &unresolvedMaterializations) {
- // Avoid materializing an unnecessary cast.
- if (inputs.size() == 1 && inputs.front().getType() == outputType)
-return inputs.front();
-
- // Create an unresolved materialization. We use a new OpBuilder to avoid
- // tracking the materialization like we do for other operations.
- OpBuilder builder(insertBlock, insertPt);
- auto convertOp =
- builder.create(loc, outputType, inputs);
- unresolvedMaterializations.emplace_back(convertOp, converter, kind,
- origOutputType);
- return convertOp.getResult(0);
-}
-static Value buildUnresolvedArgumentMaterialization(
-PatternRewriter &rewriter, Location loc, ValueRange inputs,
-Type origOutputType, Type outputType, const TypeConverter *converter,
-SmallVectorImpl &unresolvedMaterial
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC][WIP] Turn unresolved materializations into `IRRewrite`s (PR #81761)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/81761 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC][WIP] Turn unresolved materializations into `IRRewrite`s (PR #81761)
https://github.com/matthias-springer ready_for_review https://github.com/llvm/llvm-project/pull/81761 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn unresolved materializations into `IRRewrite`s (PR #81761)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/81761 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Make `ConversionPatternRewriter` constructor private (PR #82244)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/82244
`ConversionPatternRewriter` objects should not be constructed outside of
dialect conversions. Some IR modifications performed through a
`ConversionPatternRewriter` are reflected in the IR in a delayed fashion (e.g.,
only when the dialect conversion is guaranteed to succeed). Using a
`ConversionPatternRewriter` outside of the dialect conversion is incorrect API
usage and can bring the IR in an inconsistent state.
Migration guide: Use `IRRewriter` instead of `ConversionPatternRewriter`.
>From 9c3b81074c1d1422af00b59ebb4d95465a1c3ad0 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Mon, 19 Feb 2024 12:50:30 +
Subject: [PATCH] [mlir][Transforms] Make `ConversionPatternRewriter`
constructor private
`ConversionPatternRewriter` objects should not be constructed outside of
dialect conversions. Some IR modifications performed through a
`ConversionPatternRewriter` are reflected in the IR in a delayed fashion (e.g.,
only when the dialect conversion is guaranteed to succeed). Using a
`ConversionPatternRewriter` outside of the dialect conversion is incorrect API
usage and can bring the IR in an inconsistent state.
Migration guide: Use `IRRewriter` instead of `ConversionPatternRewriter`.
---
.../mlir/Transforms/DialectConversion.h| 10 +-
.../lib/Transforms/Utils/DialectConversion.cpp | 18 +++---
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index 2575be4cdea1ac..5c91a9498b35d4 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -27,6 +27,7 @@ class Block;
class ConversionPatternRewriter;
class MLIRContext;
class Operation;
+struct OperationConverter;
class Type;
class Value;
@@ -657,7 +658,6 @@ struct ConversionPatternRewriterImpl;
/// hooks.
class ConversionPatternRewriter final : public PatternRewriter {
public:
- explicit ConversionPatternRewriter(MLIRContext *ctx);
~ConversionPatternRewriter() override;
/// Apply a signature conversion to the entry block of the given region. This
@@ -764,6 +764,14 @@ class ConversionPatternRewriter final : public
PatternRewriter {
detail::ConversionPatternRewriterImpl &getImpl();
private:
+ // Allow OperationConverter to construct new rewriters.
+ friend struct OperationConverter;
+
+ /// Conversion pattern rewriters must not be used outside of dialect
+ /// conversions. They apply some IR rewrites in a delayed fashion and could
+ /// bring the IR into an inconsistent state when used standalone.
+ explicit ConversionPatternRewriter(MLIRContext *ctx);
+
// Hide unsupported pattern rewriter API.
using OpBuilder::setListener;
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 4ef26a739e4ea1..6cf178e149be7f 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -594,9 +594,11 @@ class ReplaceOperationRewrite : public OperationRewrite {
void cleanup() override;
-private:
- friend struct OperationConverter;
+ const TypeConverter *getConverter() const { return converter; }
+
+ bool hasChangedResults() const { return changedResults; }
+private:
/// An optional type converter that can be used to materialize conversions
/// between the new and old values if necessary.
const TypeConverter *converter;
@@ -2354,7 +2356,9 @@ enum OpConversionMode {
/// applied to the operations on success.
Analysis,
};
+} // namespace
+namespace mlir {
// This class converts operations to a given conversion target via a set of
// rewrite patterns. The conversion behaves differently depending on the
// conversion mode.
@@ -2414,7 +2418,7 @@ struct OperationConverter {
/// *not* to be legalizable to the target.
DenseSet *trackedOps;
};
-} // namespace
+} // namespace mlir
LogicalResult OperationConverter::convert(ConversionPatternRewriter &rewriter,
Operation *op) {
@@ -2506,7 +2510,7 @@ OperationConverter::finalize(ConversionPatternRewriter
&rewriter) {
for (unsigned i = 0; i < rewriterImpl.rewrites.size(); ++i) {
auto *opReplacement =
dyn_cast(rewriterImpl.rewrites[i].get());
-if (!opReplacement || !opReplacement->changedResults)
+if (!opReplacement || !opReplacement->hasChangedResults())
continue;
Operation *op = opReplacement->getOperation();
for (OpResult result : op->getResults()) {
@@ -2530,9 +2534,9 @@ OperationConverter::finalize(ConversionPatternRewriter
&rewriter) {
// Legalize this result.
rewriter.setInsertionPoint(op);
- if (failed(legalizeChangedResultType(op, result, newValue,
- opReplacement->converter, rewri
[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Make `ConversionPatternRewriter` constructor private (PR #82244)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/82244
>From f3fe2f1f9e2b561693bcaa2c8df84ac8cc8a60af Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Mon, 19 Feb 2024 12:50:30 +
Subject: [PATCH] [mlir][Transforms] Make `ConversionPatternRewriter`
constructor private
`ConversionPatternRewriter` objects should not be constructed outside of
dialect conversions. Some IR modifications performed through a
`ConversionPatternRewriter` are reflected in the IR in a delayed fashion (e.g.,
only when the dialect conversion is guaranteed to succeed). Using a
`ConversionPatternRewriter` outside of the dialect conversion is incorrect API
usage and can bring the IR in an inconsistent state.
Migration guide: Use `IRRewriter` instead of `ConversionPatternRewriter`.
---
flang/lib/Frontend/FrontendActions.cpp | 2 +-
.../mlir/Transforms/DialectConversion.h| 10 +-
.../lib/Transforms/Utils/DialectConversion.cpp | 18 +++---
3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/flang/lib/Frontend/FrontendActions.cpp
b/flang/lib/Frontend/FrontendActions.cpp
index 44e80e946ed832..849b3c8e4dc027 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -177,7 +177,7 @@ static void addAMDGPUSpecificMLIRItems(mlir::ModuleOp
&mlirModule,
return;
}
- mlir::ConversionPatternRewriter builder(mlirModule.getContext());
+ mlir::IRRewriter builder(mlirModule.getContext());
unsigned oclcABIVERsion = codeGenOpts.CodeObjectVersion;
auto int32Type = builder.getI32Type();
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index 2575be4cdea1ac..5c91a9498b35d4 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -27,6 +27,7 @@ class Block;
class ConversionPatternRewriter;
class MLIRContext;
class Operation;
+struct OperationConverter;
class Type;
class Value;
@@ -657,7 +658,6 @@ struct ConversionPatternRewriterImpl;
/// hooks.
class ConversionPatternRewriter final : public PatternRewriter {
public:
- explicit ConversionPatternRewriter(MLIRContext *ctx);
~ConversionPatternRewriter() override;
/// Apply a signature conversion to the entry block of the given region. This
@@ -764,6 +764,14 @@ class ConversionPatternRewriter final : public
PatternRewriter {
detail::ConversionPatternRewriterImpl &getImpl();
private:
+ // Allow OperationConverter to construct new rewriters.
+ friend struct OperationConverter;
+
+ /// Conversion pattern rewriters must not be used outside of dialect
+ /// conversions. They apply some IR rewrites in a delayed fashion and could
+ /// bring the IR into an inconsistent state when used standalone.
+ explicit ConversionPatternRewriter(MLIRContext *ctx);
+
// Hide unsupported pattern rewriter API.
using OpBuilder::setListener;
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 4ef26a739e4ea1..6cf178e149be7f 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -594,9 +594,11 @@ class ReplaceOperationRewrite : public OperationRewrite {
void cleanup() override;
-private:
- friend struct OperationConverter;
+ const TypeConverter *getConverter() const { return converter; }
+
+ bool hasChangedResults() const { return changedResults; }
+private:
/// An optional type converter that can be used to materialize conversions
/// between the new and old values if necessary.
const TypeConverter *converter;
@@ -2354,7 +2356,9 @@ enum OpConversionMode {
/// applied to the operations on success.
Analysis,
};
+} // namespace
+namespace mlir {
// This class converts operations to a given conversion target via a set of
// rewrite patterns. The conversion behaves differently depending on the
// conversion mode.
@@ -2414,7 +2418,7 @@ struct OperationConverter {
/// *not* to be legalizable to the target.
DenseSet *trackedOps;
};
-} // namespace
+} // namespace mlir
LogicalResult OperationConverter::convert(ConversionPatternRewriter &rewriter,
Operation *op) {
@@ -2506,7 +2510,7 @@ OperationConverter::finalize(ConversionPatternRewriter
&rewriter) {
for (unsigned i = 0; i < rewriterImpl.rewrites.size(); ++i) {
auto *opReplacement =
dyn_cast(rewriterImpl.rewrites[i].get());
-if (!opReplacement || !opReplacement->changedResults)
+if (!opReplacement || !opReplacement->hasChangedResults())
continue;
Operation *op = opReplacement->getOperation();
for (OpResult result : op->getResults()) {
@@ -2530,9 +2534,9 @@ OperationConverter::finalize(ConversionPatternRewriter
&rewriter) {
// Legalize this result.
rewriter.setInsertionPoint(op);
- if (failed(l
[llvm-branch-commits] [mlir] [mlir][Transforms] Encapsulate dialect conversion options in `ConversionConfig` (PR #82250)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/82250
This commit adds a new `ConversionConfig` struct that allows users to customize
the dialect conversion. This configuration is similar to `GreedyRewriteConfig`
for the greedy pattern rewrite driver.
A few existing options are moved to this objects, simplifying the dialect
conversion API.
>From 819e5f95ed0857e88972501cb10b9931b1e91a1c Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Mon, 19 Feb 2024 14:02:14 +
Subject: [PATCH] [mlir][Transforms] Encapsulate dialect conversion options in
`ConversionConfig`
This commit adds a new `ConversionConfig` struct that allows users to customize
the dialect conversion. This configuration is similar to `GreedyRewriteConfig`
for the greedy pattern rewrite driver.
A few existing options are moved to this objects, simplifying the dialect
conversion API.
---
.../mlir/Transforms/DialectConversion.h | 75 ++
.../Transforms/Utils/DialectConversion.cpp| 129 +-
mlir/test/lib/Dialect/Test/TestPatterns.cpp | 14 +-
3 files changed, 118 insertions(+), 100 deletions(-)
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index 5c91a9498b35d4..8da5dcb0be3fd0 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -24,6 +24,7 @@ namespace mlir {
// Forward declarations.
class Attribute;
class Block;
+struct ConversionConfig;
class ConversionPatternRewriter;
class MLIRContext;
class Operation;
@@ -770,7 +771,8 @@ class ConversionPatternRewriter final : public
PatternRewriter {
/// Conversion pattern rewriters must not be used outside of dialect
/// conversions. They apply some IR rewrites in a delayed fashion and could
/// bring the IR into an inconsistent state when used standalone.
- explicit ConversionPatternRewriter(MLIRContext *ctx);
+ explicit ConversionPatternRewriter(MLIRContext *ctx,
+ const ConversionConfig &config);
// Hide unsupported pattern rewriter API.
using OpBuilder::setListener;
@@ -1070,6 +1072,30 @@ class PDLConversionConfig final {
#endif // MLIR_ENABLE_PDL_IN_PATTERNMATCH
+//===--===//
+// ConversionConfig
+//===--===//
+
+/// Dialect conversion configuration.
+struct ConversionConfig {
+ /// An optional callback used to notify about match failure diagnostics
during
+ /// the conversion. Diagnostics are only reported to this callback may only
be
+ /// available in debug mode.
+ function_ref notifyCallback = nullptr;
+
+ /// Partial conversion only. All operations that are found not to be
+ /// legalizable are placed in this set. (Note that if there is an op
+ /// explicitly marked as illegal, the conversion terminates and the set will
+ /// not necessarily be complete.)
+ DenseSet *unlegalizedOps = nullptr;
+
+ /// Analysis conversion only. All operations that are found to be legalizable
+ /// are placed in this set. Note that no actual rewrites are applied to the
+ /// IR during an analysis conversion and only pre-existing operations are
+ /// added to the set.
+ DenseSet *legalizableOps = nullptr;
+};
+
//===--===//
// Op Conversion Entry Points
//===--===//
@@ -1083,19 +1109,16 @@ class PDLConversionConfig final {
/// Apply a partial conversion on the given operations and all nested
/// operations. This method converts as many operations to the target as
/// possible, ignoring operations that failed to legalize. This method only
-/// returns failure if there ops explicitly marked as illegal. If an
-/// `unconvertedOps` set is provided, all operations that are found not to be
-/// legalizable to the given `target` are placed within that set. (Note that if
-/// there is an op explicitly marked as illegal, the conversion terminates and
-/// the `unconvertedOps` set will not necessarily be complete.)
+/// returns failure if there ops explicitly marked as illegal.
LogicalResult
-applyPartialConversion(ArrayRef ops, const ConversionTarget
&target,
+applyPartialConversion(ArrayRef ops,
+ const ConversionTarget &target,
const FrozenRewritePatternSet &patterns,
- DenseSet *unconvertedOps = nullptr);
+ ConversionConfig config = ConversionConfig());
LogicalResult
applyPartialConversion(Operation *op, const ConversionTarget &target,
const FrozenRewritePatternSet &patterns,
- DenseSet *unconvertedOps = nullptr);
+ ConversionConfig config = ConversionConfig());
/// Apply a compl
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Decouple `ConversionPatternRewriterImpl` from `ConversionPatternRewriter` (PR #82333)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/82333
`ConversionPatternRewriterImpl` no longer maintains a reference to the
respective `ConversionPatternRewriter`. An `MLIRContext` is sufficient. This
commit simplifies the internal state of `ConversionPatternRewriterImpl`.
>From 362813d590d6b61ebc0d03db2ed56764c5332d2c Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Tue, 20 Feb 2024 09:48:57 +
Subject: [PATCH] [mlir][Transforms][NFC] Decouple
`ConversionPatternRewriterImpl` from `ConversionPatternRewriter`
`ConversionPatternRewriterImpl` no longer maintains a reference to the
respective `ConversionPatternRewriter`. An `MLIRContext` is sufficient. This
commit simplifies the internal state of `ConversionPatternRewriterImpl`.
---
.../Transforms/Utils/DialectConversion.cpp| 44 +--
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 30fc2298b3deb3..ec97a4247658b8 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -725,10 +725,9 @@ static RewriteTy *findSingleRewrite(R &&rewrites, Block
*block) {
namespace mlir {
namespace detail {
struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
- explicit ConversionPatternRewriterImpl(PatternRewriter &rewriter,
+ explicit ConversionPatternRewriterImpl(MLIRContext *ctx,
const ConversionConfig &config)
- : rewriter(rewriter), eraseRewriter(rewriter.getContext()),
-config(config) {}
+ : eraseRewriter(ctx), config(config) {}
//======//
// State Management
@@ -823,8 +822,8 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
Type origOutputType,
const TypeConverter *converter);
- Value buildUnresolvedArgumentMaterialization(PatternRewriter &rewriter,
- Location loc, ValueRange inputs,
+ Value buildUnresolvedArgumentMaterialization(Block *block, Location loc,
+ ValueRange inputs,
Type origOutputType,
Type outputType,
const TypeConverter *converter);
@@ -903,8 +902,6 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
// State
//======//
- PatternRewriter &rewriter;
-
/// This rewriter must be used for erasing ops/blocks.
SingleEraseRewriter eraseRewriter;
@@ -1008,8 +1005,12 @@ void BlockTypeConversionRewrite::rollback() {
LogicalResult BlockTypeConversionRewrite::materializeLiveConversions(
function_ref findLiveUser) {
+ auto builder = OpBuilder::atBlockBegin(block, /*listener=*/&rewriterImpl);
+
// Process the remapping for each of the original arguments.
for (unsigned i = 0, e = origBlock->getNumArguments(); i != e; ++i) {
+OpBuilder::InsertionGuard g(builder);
+
// If the type of this argument changed and the argument is still live, we
// need to materialize a conversion.
BlockArgument origArg = origBlock->getArgument(i);
@@ -1021,14 +1022,12 @@ LogicalResult
BlockTypeConversionRewrite::materializeLiveConversions(
Value replacementValue = rewriterImpl.mapping.lookupOrDefault(origArg);
bool isDroppedArg = replacementValue == origArg;
-if (isDroppedArg)
- rewriterImpl.rewriter.setInsertionPointToStart(getBlock());
-else
- rewriterImpl.rewriter.setInsertionPointAfterValue(replacementValue);
+if (!isDroppedArg)
+ builder.setInsertionPointAfterValue(replacementValue);
Value newArg;
if (converter) {
newArg = converter->materializeSourceConversion(
- rewriterImpl.rewriter, origArg.getLoc(), origArg.getType(),
+ builder, origArg.getLoc(), origArg.getType(),
isDroppedArg ? ValueRange() : ValueRange(replacementValue));
assert((!newArg || newArg.getType() == origArg.getType()) &&
"materialization hook did not provide a value of the expected "
@@ -1293,6 +1292,8 @@ LogicalResult
ConversionPatternRewriterImpl::convertNonEntryRegionTypes(
Block *ConversionPatternRewriterImpl::applySignatureConversion(
Block *block, const TypeConverter *converter,
TypeConverter::SignatureConversion &signatureConversion) {
+ MLIRContext *ctx = block->getParentOp()->getContext();
+
// If no arguments are being changed or added, there is nothing to do.
unsigned origArgCount = block->getNumArguments();
auto convertedTypes = signatureConversion.getConvertedTypes();
@@ -1309,7 +1310,7 @@ B
[llvm-branch-commits] [mlir] [mlir][Transforms] Support rolling back properties in dialect conversion (PR #82474)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/82474
The dialect conversion rolls back in-place op modifications upon failure.
Rolling back modifications of attributes is already supported, but there was no
support for properties until now.
>From 7be59f67152a684a572c45109e8288304cc4833a Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 21 Feb 2024 08:41:44 +
Subject: [PATCH] [mlir][Transforms] Support rolling back properties in dialect
conversion
The dialect conversion rolls back inplace op modifications upon failure.
Rolling back modifications of op properties was not supported before this
commit.
---
.../Transforms/Utils/DialectConversion.cpp| 28 ++-
mlir/test/Transforms/test-legalizer.mlir | 12
mlir/test/lib/Dialect/Test/TestPatterns.cpp | 18 +++-
3 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 673bd0383809cb..495e9d8df19d0a 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -1002,12 +1002,31 @@ class ModifyOperationRewrite : public OperationRewrite {
: OperationRewrite(Kind::ModifyOperation, rewriterImpl, op),
loc(op->getLoc()), attrs(op->getAttrDictionary()),
operands(op->operand_begin(), op->operand_end()),
-successors(op->successor_begin(), op->successor_end()) {}
+successors(op->successor_begin(), op->successor_end()) {
+if (OpaqueProperties prop = op->getPropertiesStorage()) {
+ // Make a copy of the properties.
+ int size = op->getPropertiesStorageSize();
+ propertiesStorage = operator new(size);
+ memcpy(propertiesStorage, prop.as(), size);
+}
+ }
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() == Kind::ModifyOperation;
}
+ ~ModifyOperationRewrite() override {
+assert(!propertiesStorage &&
+ "rewrite was neither committed nor rolled back");
+ }
+
+ void commit() override {
+if (propertiesStorage) {
+ operator delete(propertiesStorage);
+ propertiesStorage = nullptr;
+}
+ }
+
/// Discard the transaction state and reset the state of the original
/// operation.
void rollback() override {
@@ -1016,6 +1035,12 @@ class ModifyOperationRewrite : public OperationRewrite {
op->setOperands(operands);
for (const auto &it : llvm::enumerate(successors))
op->setSuccessor(it.value(), it.index());
+if (propertiesStorage) {
+ OpaqueProperties prop(propertiesStorage);
+ op->copyProperties(prop);
+ operator delete(propertiesStorage);
+ propertiesStorage = nullptr;
+}
}
private:
@@ -1023,6 +1048,7 @@ class ModifyOperationRewrite : public OperationRewrite {
DictionaryAttr attrs;
SmallVector operands;
SmallVector successors;
+ void *propertiesStorage = nullptr;
};
} // namespace
diff --git a/mlir/test/Transforms/test-legalizer.mlir
b/mlir/test/Transforms/test-legalizer.mlir
index 84fcc18ab7d370..62d776cd7573ee 100644
--- a/mlir/test/Transforms/test-legalizer.mlir
+++ b/mlir/test/Transforms/test-legalizer.mlir
@@ -334,3 +334,15 @@ func.func @test_move_op_before_rollback() {
}) : () -> ()
"test.return"() : () -> ()
}
+
+// -
+
+// CHECK-LABEL: func @test_properties_rollback()
+func.func @test_properties_rollback() {
+ // CHECK: test.with_properties <{a = 32 : i64,
+ // expected-remark @below{{op 'test.with_properties' is not legalizable}}
+ test.with_properties
+ <{a = 32 : i64, array = array, b = "foo"}>
+ {modify_inplace}
+ "test.return"() : () -> ()
+}
diff --git a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
index 1c02232b8adbb1..57e846294f8b9f 100644
--- a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
+++ b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
@@ -806,6 +806,21 @@ struct TestUndoBlockErase : public ConversionPattern {
}
};
+/// A pattern that modifies a property in-place, but keeps the op illegal.
+struct TestUndoPropertiesModification : public ConversionPattern {
+ TestUndoPropertiesModification(MLIRContext *ctx)
+ : ConversionPattern("test.with_properties", /*benefit=*/1, ctx) {}
+ LogicalResult
+ matchAndRewrite(Operation *op, ArrayRef operands,
+ ConversionPatternRewriter &rewriter) const final {
+if (!op->hasAttr("modify_inplace"))
+ return failure();
+rewriter.modifyOpInPlace(
+op, [&]() { cast(op).getProperties().setA(42);
});
+return success();
+ }
+};
+
//===--===//
// Type-Conversion Rewrite Testing
@@ -1085,7 +1100,8 @@ struct TestLegalizePatternDriver
TestChangeProducerTypeF32ToInvalid, TestUpdateConsumerType,
TestNonRootReplacement, TestBoundedRecursiveRewrit
[llvm-branch-commits] [mlir] [mlir][Transforms] Support rolling back properties in dialect conversion (PR #82474)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/82474
>From 33f2ae9da319110ca8d2581ec6d66d2db83201cb Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 21 Feb 2024 08:41:44 +
Subject: [PATCH] [mlir][Transforms] Support rolling back properties in dialect
conversion
The dialect conversion rolls back inplace op modifications upon failure.
Rolling back modifications of op properties was not supported before this
commit.
---
.../Transforms/Utils/DialectConversion.cpp| 28 ++-
mlir/test/Transforms/test-legalizer.mlir | 12
mlir/test/lib/Dialect/Test/TestPatterns.cpp | 18 +++-
3 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 673bd0383809cb..5be3e6b90b5d08 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -1002,12 +1002,31 @@ class ModifyOperationRewrite : public OperationRewrite {
: OperationRewrite(Kind::ModifyOperation, rewriterImpl, op),
loc(op->getLoc()), attrs(op->getAttrDictionary()),
operands(op->operand_begin(), op->operand_end()),
-successors(op->successor_begin(), op->successor_end()) {}
+successors(op->successor_begin(), op->successor_end()) {
+if (OpaqueProperties prop = op->getPropertiesStorage()) {
+ // Make a copy of the properties.
+ propertiesStorage = operator new(op->getPropertiesStorageSize());
+ OpaqueProperties propCopy(propertiesStorage);
+ op->getName().copyOpProperties(propCopy, prop);
+}
+ }
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() == Kind::ModifyOperation;
}
+ ~ModifyOperationRewrite() override {
+assert(!propertiesStorage &&
+ "rewrite was neither committed nor rolled back");
+ }
+
+ void commit() override {
+if (propertiesStorage) {
+ operator delete(propertiesStorage);
+ propertiesStorage = nullptr;
+}
+ }
+
/// Discard the transaction state and reset the state of the original
/// operation.
void rollback() override {
@@ -1016,6 +1035,12 @@ class ModifyOperationRewrite : public OperationRewrite {
op->setOperands(operands);
for (const auto &it : llvm::enumerate(successors))
op->setSuccessor(it.value(), it.index());
+if (propertiesStorage) {
+ OpaqueProperties prop(propertiesStorage);
+ op->copyProperties(prop);
+ operator delete(propertiesStorage);
+ propertiesStorage = nullptr;
+}
}
private:
@@ -1023,6 +1048,7 @@ class ModifyOperationRewrite : public OperationRewrite {
DictionaryAttr attrs;
SmallVector operands;
SmallVector successors;
+ void *propertiesStorage = nullptr;
};
} // namespace
diff --git a/mlir/test/Transforms/test-legalizer.mlir
b/mlir/test/Transforms/test-legalizer.mlir
index 84fcc18ab7d370..62d776cd7573ee 100644
--- a/mlir/test/Transforms/test-legalizer.mlir
+++ b/mlir/test/Transforms/test-legalizer.mlir
@@ -334,3 +334,15 @@ func.func @test_move_op_before_rollback() {
}) : () -> ()
"test.return"() : () -> ()
}
+
+// -
+
+// CHECK-LABEL: func @test_properties_rollback()
+func.func @test_properties_rollback() {
+ // CHECK: test.with_properties <{a = 32 : i64,
+ // expected-remark @below{{op 'test.with_properties' is not legalizable}}
+ test.with_properties
+ <{a = 32 : i64, array = array, b = "foo"}>
+ {modify_inplace}
+ "test.return"() : () -> ()
+}
diff --git a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
index 1c02232b8adbb1..57e846294f8b9f 100644
--- a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
+++ b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
@@ -806,6 +806,21 @@ struct TestUndoBlockErase : public ConversionPattern {
}
};
+/// A pattern that modifies a property in-place, but keeps the op illegal.
+struct TestUndoPropertiesModification : public ConversionPattern {
+ TestUndoPropertiesModification(MLIRContext *ctx)
+ : ConversionPattern("test.with_properties", /*benefit=*/1, ctx) {}
+ LogicalResult
+ matchAndRewrite(Operation *op, ArrayRef operands,
+ ConversionPatternRewriter &rewriter) const final {
+if (!op->hasAttr("modify_inplace"))
+ return failure();
+rewriter.modifyOpInPlace(
+op, [&]() { cast(op).getProperties().setA(42);
});
+return success();
+ }
+};
+
//===--===//
// Type-Conversion Rewrite Testing
@@ -1085,7 +1100,8 @@ struct TestLegalizePatternDriver
TestChangeProducerTypeF32ToInvalid, TestUpdateConsumerType,
TestNonRootReplacement, TestBoundedRecursiveRewrite,
TestNestedOpCreationUndoRewrite, TestReplaceEraseOp,
- TestCreateUnregisteredOp, TestUndoMoveOpBefore>(&getContext());
+
[llvm-branch-commits] [mlir] [mlir][Transforms] Support rolling back properties in dialect conversion (PR #82474)
@@ -1002,12 +1002,31 @@ class ModifyOperationRewrite : public OperationRewrite {
: OperationRewrite(Kind::ModifyOperation, rewriterImpl, op),
loc(op->getLoc()), attrs(op->getAttrDictionary()),
operands(op->operand_begin(), op->operand_end()),
-successors(op->successor_begin(), op->successor_end()) {}
+successors(op->successor_begin(), op->successor_end()) {
+if (OpaqueProperties prop = op->getPropertiesStorage()) {
+ // Make a copy of the properties.
+ int size = op->getPropertiesStorageSize();
+ propertiesStorage = operator new(size);
+ memcpy(propertiesStorage, prop.as(), size);
matthias-springer wrote:
Changed to `copyOpProperties`.
https://github.com/llvm/llvm-project/pull/82474
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Support rolling back properties in dialect conversion (PR #82474)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/82474
>From f7a0cc4dd214b9f2b234ea5a61eaecdb19b6b3c4 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 21 Feb 2024 09:37:48 +
Subject: [PATCH] [mlir][Transforms] Support rolling back properties in dialect
conversion
The dialect conversion rolls back inplace op modifications upon failure.
Rolling back modifications of op properties was not supported before this
commit.
---
.../Transforms/Utils/DialectConversion.cpp| 31 ++-
mlir/test/Transforms/test-legalizer.mlir | 12 +++
mlir/test/lib/Dialect/Test/TestPatterns.cpp | 18 ++-
3 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 673bd0383809cb..37ecc694455e54 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -1002,12 +1002,33 @@ class ModifyOperationRewrite : public OperationRewrite {
: OperationRewrite(Kind::ModifyOperation, rewriterImpl, op),
loc(op->getLoc()), attrs(op->getAttrDictionary()),
operands(op->operand_begin(), op->operand_end()),
-successors(op->successor_begin(), op->successor_end()) {}
+successors(op->successor_begin(), op->successor_end()) {
+if (OpaqueProperties prop = op->getPropertiesStorage()) {
+ // Make a copy of the properties.
+ propertiesStorage = operator new(op->getPropertiesStorageSize());
+ OpaqueProperties propCopy(propertiesStorage);
+ op->getName().initOpProperties(propCopy, /*init=*/prop);
+}
+ }
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() == Kind::ModifyOperation;
}
+ ~ModifyOperationRewrite() override {
+assert(!propertiesStorage &&
+ "rewrite was neither committed nor rolled back");
+ }
+
+ void commit() override {
+if (propertiesStorage) {
+ OpaqueProperties prop(propertiesStorage);
+ op->getName().destroyOpProperties(prop);
+ operator delete(propertiesStorage);
+ propertiesStorage = nullptr;
+}
+ }
+
/// Discard the transaction state and reset the state of the original
/// operation.
void rollback() override {
@@ -1016,6 +1037,13 @@ class ModifyOperationRewrite : public OperationRewrite {
op->setOperands(operands);
for (const auto &it : llvm::enumerate(successors))
op->setSuccessor(it.value(), it.index());
+if (propertiesStorage) {
+ OpaqueProperties prop(propertiesStorage);
+ op->copyProperties(prop);
+ op->getName().destroyOpProperties(prop);
+ operator delete(propertiesStorage);
+ propertiesStorage = nullptr;
+}
}
private:
@@ -1023,6 +1051,7 @@ class ModifyOperationRewrite : public OperationRewrite {
DictionaryAttr attrs;
SmallVector operands;
SmallVector successors;
+ void *propertiesStorage = nullptr;
};
} // namespace
diff --git a/mlir/test/Transforms/test-legalizer.mlir
b/mlir/test/Transforms/test-legalizer.mlir
index 84fcc18ab7d370..62d776cd7573ee 100644
--- a/mlir/test/Transforms/test-legalizer.mlir
+++ b/mlir/test/Transforms/test-legalizer.mlir
@@ -334,3 +334,15 @@ func.func @test_move_op_before_rollback() {
}) : () -> ()
"test.return"() : () -> ()
}
+
+// -
+
+// CHECK-LABEL: func @test_properties_rollback()
+func.func @test_properties_rollback() {
+ // CHECK: test.with_properties <{a = 32 : i64,
+ // expected-remark @below{{op 'test.with_properties' is not legalizable}}
+ test.with_properties
+ <{a = 32 : i64, array = array, b = "foo"}>
+ {modify_inplace}
+ "test.return"() : () -> ()
+}
diff --git a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
index 1c02232b8adbb1..57e846294f8b9f 100644
--- a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
+++ b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
@@ -806,6 +806,21 @@ struct TestUndoBlockErase : public ConversionPattern {
}
};
+/// A pattern that modifies a property in-place, but keeps the op illegal.
+struct TestUndoPropertiesModification : public ConversionPattern {
+ TestUndoPropertiesModification(MLIRContext *ctx)
+ : ConversionPattern("test.with_properties", /*benefit=*/1, ctx) {}
+ LogicalResult
+ matchAndRewrite(Operation *op, ArrayRef operands,
+ ConversionPatternRewriter &rewriter) const final {
+if (!op->hasAttr("modify_inplace"))
+ return failure();
+rewriter.modifyOpInPlace(
+op, [&]() { cast(op).getProperties().setA(42);
});
+return success();
+ }
+};
+
//===--===//
// Type-Conversion Rewrite Testing
@@ -1085,7 +1100,8 @@ struct TestLegalizePatternDriver
TestChangeProducerTypeF32ToInvalid, TestUpdateConsumerType,
TestNonRootReplacement, TestBoundedRecursiveRewrite,
[llvm-branch-commits] [mlir] [mlir][Transforms] Support rolling back properties in dialect conversion (PR #82474)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/82474
>From 1795b6dcfdc0b6447c5209000332432de49109cc Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 21 Feb 2024 09:37:48 +
Subject: [PATCH] [mlir][Transforms] Support rolling back properties in dialect
conversion
The dialect conversion rolls back inplace op modifications upon failure.
Rolling back modifications of op properties was not supported before this
commit.
---
.../Transforms/Utils/DialectConversion.cpp| 31 ++-
mlir/test/Transforms/test-legalizer.mlir | 12 +++
mlir/test/lib/Dialect/Test/TestPatterns.cpp | 18 ++-
3 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 673bd0383809cb..75094a41012e8d 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -1002,12 +1002,33 @@ class ModifyOperationRewrite : public OperationRewrite {
: OperationRewrite(Kind::ModifyOperation, rewriterImpl, op),
loc(op->getLoc()), attrs(op->getAttrDictionary()),
operands(op->operand_begin(), op->operand_end()),
-successors(op->successor_begin(), op->successor_end()) {}
+successors(op->successor_begin(), op->successor_end()) {
+if (OpaqueProperties prop = op->getPropertiesStorage()) {
+ // Make a copy of the properties.
+ propertiesStorage = operator new(op->getPropertiesStorageSize());
+ OpaqueProperties propCopy(propertiesStorage);
+ op->getName().initOpProperties(propCopy, /*init=*/prop);
+}
+ }
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() == Kind::ModifyOperation;
}
+ ~ModifyOperationRewrite() override {
+assert(!propertiesStorage &&
+ "rewrite was neither committed nor rolled back");
+ }
+
+ void commit() override {
+if (propertiesStorage) {
+ OpaqueProperties propCopy(propertiesStorage);
+ op->getName().destroyOpProperties(propCopy);
+ operator delete(propertiesStorage);
+ propertiesStorage = nullptr;
+}
+ }
+
/// Discard the transaction state and reset the state of the original
/// operation.
void rollback() override {
@@ -1016,6 +1037,13 @@ class ModifyOperationRewrite : public OperationRewrite {
op->setOperands(operands);
for (const auto &it : llvm::enumerate(successors))
op->setSuccessor(it.value(), it.index());
+if (propertiesStorage) {
+ OpaqueProperties propCopy(propertiesStorage);
+ op->copyProperties(propCopy);
+ op->getName().destroyOpProperties(propCopy);
+ operator delete(propertiesStorage);
+ propertiesStorage = nullptr;
+}
}
private:
@@ -1023,6 +1051,7 @@ class ModifyOperationRewrite : public OperationRewrite {
DictionaryAttr attrs;
SmallVector operands;
SmallVector successors;
+ void *propertiesStorage = nullptr;
};
} // namespace
diff --git a/mlir/test/Transforms/test-legalizer.mlir
b/mlir/test/Transforms/test-legalizer.mlir
index 84fcc18ab7d370..62d776cd7573ee 100644
--- a/mlir/test/Transforms/test-legalizer.mlir
+++ b/mlir/test/Transforms/test-legalizer.mlir
@@ -334,3 +334,15 @@ func.func @test_move_op_before_rollback() {
}) : () -> ()
"test.return"() : () -> ()
}
+
+// -
+
+// CHECK-LABEL: func @test_properties_rollback()
+func.func @test_properties_rollback() {
+ // CHECK: test.with_properties <{a = 32 : i64,
+ // expected-remark @below{{op 'test.with_properties' is not legalizable}}
+ test.with_properties
+ <{a = 32 : i64, array = array, b = "foo"}>
+ {modify_inplace}
+ "test.return"() : () -> ()
+}
diff --git a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
index 1c02232b8adbb1..57e846294f8b9f 100644
--- a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
+++ b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
@@ -806,6 +806,21 @@ struct TestUndoBlockErase : public ConversionPattern {
}
};
+/// A pattern that modifies a property in-place, but keeps the op illegal.
+struct TestUndoPropertiesModification : public ConversionPattern {
+ TestUndoPropertiesModification(MLIRContext *ctx)
+ : ConversionPattern("test.with_properties", /*benefit=*/1, ctx) {}
+ LogicalResult
+ matchAndRewrite(Operation *op, ArrayRef operands,
+ ConversionPatternRewriter &rewriter) const final {
+if (!op->hasAttr("modify_inplace"))
+ return failure();
+rewriter.modifyOpInPlace(
+op, [&]() { cast(op).getProperties().setA(42);
});
+return success();
+ }
+};
+
//===--===//
// Type-Conversion Rewrite Testing
@@ -1085,7 +1100,8 @@ struct TestLegalizePatternDriver
TestChangeProducerTypeF32ToInvalid, TestUpdateConsumerType,
TestNonRootReplacement, TestBoundedR
[llvm-branch-commits] [mlir] [mlir][Transforms] Support rolling back properties in dialect conversion (PR #82474)
@@ -1002,12 +1002,31 @@ class ModifyOperationRewrite : public OperationRewrite {
: OperationRewrite(Kind::ModifyOperation, rewriterImpl, op),
loc(op->getLoc()), attrs(op->getAttrDictionary()),
operands(op->operand_begin(), op->operand_end()),
-successors(op->successor_begin(), op->successor_end()) {}
+successors(op->successor_begin(), op->successor_end()) {
+if (OpaqueProperties prop = op->getPropertiesStorage()) {
+ // Make a copy of the properties.
+ propertiesStorage = operator new(op->getPropertiesStorageSize());
+ OpaqueProperties propCopy(propertiesStorage);
+ op->getName().copyOpProperties(propCopy, prop);
matthias-springer wrote:
Replaced `copyOpProperties` with `initProperties`, this is calling the copy
constructor.
https://github.com/llvm/llvm-project/pull/82474
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn block type conversion into `IRRewrite` (PR #81756)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81756
>From 68cb2590ea6295504b64d065326a606c2892a2a1 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 21 Feb 2024 16:48:48 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn block type convertion into
`IRRewrite`
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be commited (upon success) or rolled
back (upon failure).
Until now, the signature conversion of a block was only a "partial" IR rewrite.
Rollbacks were triggered via `BlockTypeConversionRewrite::rollback`, but there
was no `BlockTypeConversionRewrite::commit` equivalent.
Overview of changes:
* Remove `ArgConverter`, an internal helper class that kept track of all block
type conversions. There is now a separate `BlockTypeConversionRewrite` for each
block type conversion.
* No more special handling for block type conversions. They are now normal "IR
rewrites", just like "block creation" or "block movement". In particular,
trigger "commits" of block type conversion via
`BlockTypeConversionRewrite::commit`.
* Remove `ArgConverter::notifyOpRemoved`. This function was used to inform the
`ArgConverter` that an operation was erased, to prevent a double-free of
operations in certain situations. It would be unpractical to add a
`notifyOpRemoved` API to `IRRewrite`. Instead, erasing ops/block should go
through a new `SingleEraseRewriter` (that is owned by the
`ConversionPatternRewriterImpl`) if there is chance of double-free. This
rewriter ignores `eraseOp`/`eraseBlock` if the op/block was already freed.
BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
.../Transforms/Utils/DialectConversion.cpp| 794 --
1 file changed, 364 insertions(+), 430 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index afdd31a748c8c4..db41b9f19e7e8d 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,12 +154,13 @@ namespace {
struct RewriterState {
RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
unsigned numReplacements, unsigned numArgReplacements,
-unsigned numRewrites, unsigned numIgnoredOperations)
+unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
: numCreatedOps(numCreatedOps),
numUnresolvedMaterializations(numUnresolvedMaterializations),
numReplacements(numReplacements),
numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations) {}
+numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
/// The current number of created operations.
unsigned numCreatedOps;
@@ -178,6 +179,9 @@ struct RewriterState {
/// The current number of ignored operations.
unsigned numIgnoredOperations;
+
+ /// The current number of erased operations/blocks.
+ unsigned numErased;
};
//===--===//
@@ -292,370 +296,6 @@ static Value buildUnresolvedTargetMaterialization(
outputType, outputType, converter, unresolvedMaterializations);
}
-//===--===//
-// ArgConverter
-//===--===//
-namespace {
-/// This class provides a simple interface for converting the types of block
-/// arguments. This is done by creating a new block that contains the new legal
-/// types and extracting the block that contains the old illegal types to allow
-/// for undoing pending rewrites in the case of failure.
-struct ArgConverter {
- ArgConverter(
- PatternRewriter &rewriter,
- SmallVectorImpl &unresolvedMaterializations)
- : rewriter(rewriter),
-unresolvedMaterializations(unresolvedMaterializations) {}
-
- /// This structure contains the information pertaining to an argument that
has
- /// been converted.
- struct ConvertedArgInfo {
-ConvertedArgInfo(unsigned newArgIdx, unsigned newArgSize,
- Value castValue = nullptr)
-: newArgIdx(newArgIdx), newArgSize(newArgSize), castValue(castValue) {}
-
-/// The start index of in the new argument list that contains arguments
that
-/// replace the original.
-unsigned newArgIdx;
-
-/// The number of arguments that replaced the original argument.
-unsigned newArgSize;
-
-/// The cast value that was created to cast from the new arguments to the
-/// old. This only used if 'newArgSize' > 1.
-Value castValue;
- };
-
- /// This structure contains information pertaining to a block that has had
its
- /// signature converted.
- struct ConvertedBlockInfo {
-
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op creation into `IRRewrite` (PR #81759)
@@ -2549,10 +2572,15 @@ LogicalResult
OperationConverter::legalizeConvertedArgumentTypes(
});
return liveUserIt == val.user_end() ? nullptr : *liveUserIt;
};
- for (auto &r : rewriterImpl.rewrites)
-if (auto *rewrite = dyn_cast(r.get()))
- if (failed(rewrite->materializeLiveConversions(findLiveUser)))
+ // Note: `rewrites` may be reallocated as the loop is running.
+ for (int64_t i = 0; i < rewriterImpl.rewrites.size(); ++i) {
matthias-springer wrote:
That won't work here because things may get added to the vector while the loop
is running and that could invalidate the iterator (when the vector reallocates
storage).
https://github.com/llvm/llvm-project/pull/81759
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op creation into `IRRewrite` (PR #81759)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81759
>From d15c439f166426a613ac0021a9101b774d544357 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Fri, 16 Feb 2024 15:21:48 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn op creation into `IRRewrite`
This commit is a refactoring of the dialect conversion. The dialect conversion
maintains a list of "IR rewrites" that can be commited (upon success) or rolled
back (upon failure).
Until now, the dialect conversion kept track of "op creation" in separate
internal data structures. This commit turns "op creation" into an `IRRewrite`
that can be committed and rolled back just like any other rewrite. This commit
simplifies the internal state of the dialect conversion.
---
.../Transforms/Utils/DialectConversion.cpp| 101 +++---
1 file changed, 63 insertions(+), 38 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index dec68048dc1d30..ae81dfa4d8303b 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,17 +152,12 @@ namespace {
/// This class contains a snapshot of the current conversion rewriter state.
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
- RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numErased)
- : numCreatedOps(numCreatedOps),
-numUnresolvedMaterializations(numUnresolvedMaterializations),
+ RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
+unsigned numIgnoredOperations, unsigned numErased)
+ : numUnresolvedMaterializations(numUnresolvedMaterializations),
numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
numErased(numErased) {}
- /// The current number of created operations.
- unsigned numCreatedOps;
-
/// The current number of unresolved materializations.
unsigned numUnresolvedMaterializations;
@@ -303,7 +298,8 @@ class IRRewrite {
// Operation rewrites
MoveOperation,
ModifyOperation,
-ReplaceOperation
+ReplaceOperation,
+CreateOperation
};
virtual ~IRRewrite() = default;
@@ -376,7 +372,10 @@ class CreateBlockRewrite : public BlockRewrite {
auto &blockOps = block->getOperations();
while (!blockOps.empty())
blockOps.remove(blockOps.begin());
-eraseBlock(block);
+if (block->getParent())
+ eraseBlock(block);
+else
+ delete block;
}
};
@@ -606,7 +605,7 @@ class OperationRewrite : public IRRewrite {
static bool classof(const IRRewrite *rewrite) {
return rewrite->getKind() >= Kind::MoveOperation &&
- rewrite->getKind() <= Kind::ReplaceOperation;
+ rewrite->getKind() <= Kind::CreateOperation;
}
protected:
@@ -740,6 +739,19 @@ class ReplaceOperationRewrite : public OperationRewrite {
/// A boolean flag that indicates whether result types have changed or not.
bool changedResults;
};
+
+class CreateOperationRewrite : public OperationRewrite {
+public:
+ CreateOperationRewrite(ConversionPatternRewriterImpl &rewriterImpl,
+ Operation *op)
+ : OperationRewrite(Kind::CreateOperation, rewriterImpl, op) {}
+
+ static bool classof(const IRRewrite *rewrite) {
+return rewrite->getKind() == Kind::CreateOperation;
+ }
+
+ void rollback() override;
+};
} // namespace
/// Return "true" if there is an operation rewrite that matches the specified
@@ -957,9 +969,6 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
// replacing a value with one of a different type.
ConversionValueMapping mapping;
- /// Ordered vector of all of the newly created operations during conversion.
- SmallVector createdOps;
-
/// Ordered vector of all unresolved type conversion materializations during
/// conversion.
SmallVector unresolvedMaterializations;
@@ -1144,6 +1153,15 @@ void ReplaceOperationRewrite::rollback() {
void ReplaceOperationRewrite::cleanup() { eraseOp(op); }
+void CreateOperationRewrite::rollback() {
+ for (Region ®ion : op->getRegions()) {
+while (!region.getBlocks().empty())
+ region.getBlocks().remove(region.getBlocks().begin());
+ }
+ op->dropAllUses();
+ eraseOp(op);
+}
+
void ConversionPatternRewriterImpl::detachNestedAndErase(Operation *op) {
for (Region ®ion : op->getRegions()) {
for (Block &block : region.getBlocks()) {
@@ -1161,8 +1179,6 @@ void ConversionPatternRewriterImpl::discardRewrites() {
// Remove any newly created ops.
for (UnresolvedMaterialization &materialization : unresolvedMaterializations)
detachNestedAndErase(materialization.getOp());
- for (auto *op : llvm::reverse(createdOps))
-detachNestedAndErase(op);
}
void C
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn unresolved materializations into `IRRewrite`s (PR #81761)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/81761
>From 6d02f6d545fff81a41b7e40fa48f19c55483c77b Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Mon, 19 Feb 2024 10:29:29 +
Subject: [PATCH] [WIP] UnresolvedMaterialization
BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
.../Transforms/Utils/DialectConversion.cpp| 369 +-
1 file changed, 176 insertions(+), 193 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index ae81dfa4d8303b..5ed31e35dc0705 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,15 +152,11 @@ namespace {
/// This class contains a snapshot of the current conversion rewriter state.
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
- RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
-unsigned numIgnoredOperations, unsigned numErased)
- : numUnresolvedMaterializations(numUnresolvedMaterializations),
-numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
+ RewriterState(unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
+ : numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
numErased(numErased) {}
- /// The current number of unresolved materializations.
- unsigned numUnresolvedMaterializations;
-
/// The current number of rewrites performed.
unsigned numRewrites;
@@ -171,109 +167,10 @@ struct RewriterState {
unsigned numErased;
};
-//===--===//
-// UnresolvedMaterialization
-
-/// This class represents an unresolved materialization, i.e. a materialization
-/// that was inserted during conversion that needs to be legalized at the end
of
-/// the conversion process.
-class UnresolvedMaterialization {
-public:
- /// The type of materialization.
- enum Kind {
-/// This materialization materializes a conversion for an illegal block
-/// argument type, to a legal one.
-Argument,
-
-/// This materialization materializes a conversion from an illegal type to
a
-/// legal one.
-Target
- };
-
- UnresolvedMaterialization(UnrealizedConversionCastOp op = nullptr,
-const TypeConverter *converter = nullptr,
-Kind kind = Target, Type origOutputType = nullptr)
- : op(op), converterAndKind(converter, kind),
-origOutputType(origOutputType) {}
-
- /// Return the temporary conversion operation inserted for this
- /// materialization.
- UnrealizedConversionCastOp getOp() const { return op; }
-
- /// Return the type converter of this materialization (which may be null).
- const TypeConverter *getConverter() const {
-return converterAndKind.getPointer();
- }
-
- /// Return the kind of this materialization.
- Kind getKind() const { return converterAndKind.getInt(); }
-
- /// Set the kind of this materialization.
- void setKind(Kind kind) { converterAndKind.setInt(kind); }
-
- /// Return the original illegal output type of the input values.
- Type getOrigOutputType() const { return origOutputType; }
-
-private:
- /// The unresolved materialization operation created during conversion.
- UnrealizedConversionCastOp op;
-
- /// The corresponding type converter to use when resolving this
- /// materialization, and the kind of this materialization.
- llvm::PointerIntPair converterAndKind;
-
- /// The original output type. This is only used for argument conversions.
- Type origOutputType;
-};
-} // namespace
-
-/// Build an unresolved materialization operation given an output type and set
-/// of input operands.
-static Value buildUnresolvedMaterialization(
-UnresolvedMaterialization::Kind kind, Block *insertBlock,
-Block::iterator insertPt, Location loc, ValueRange inputs, Type outputType,
-Type origOutputType, const TypeConverter *converter,
-SmallVectorImpl &unresolvedMaterializations) {
- // Avoid materializing an unnecessary cast.
- if (inputs.size() == 1 && inputs.front().getType() == outputType)
-return inputs.front();
-
- // Create an unresolved materialization. We use a new OpBuilder to avoid
- // tracking the materialization like we do for other operations.
- OpBuilder builder(insertBlock, insertPt);
- auto convertOp =
- builder.create(loc, outputType, inputs);
- unresolvedMaterializations.emplace_back(convertOp, converter, kind,
- origOutputType);
- return convertOp.getResult(0);
-}
-static Value buildUnresolvedArgumentMaterialization(
-PatternRewriter &rewriter, Location loc, ValueRange inputs,
-Type origOutputType, Type outputType, const TypeConverter *converter,
-SmallVectorImpl &unresolvedMaterial
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Remove `SplitBlockRewrite` (PR #82777)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/82777 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Simplify `BlockTypeConversionRewrite` (PR #83286)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/83286
When a block signature is converted during dialect conversion a
`BlockTypeConversionRewrite` object is stored in the stack of rewrites. Such an
object represents multiple steps:
- Splitting the old block, i.e., creating a new block and moving all operations
over.
- Rewriting block arguments.
- Erasing the old block.
We have dedicated `IRRewrite` objects that represent "creating a block",
"moving an op" and "erasing a block". This commit reuses those rewrite objects,
so that there is less work to do in `BlockTypeConversionRewrite::rollback` and
`BlockTypeConversionRewrite::commit`.
>From ab78e8c90ca3ecf60e1192c198d9f5025563dec2 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Wed, 28 Feb 2024 16:33:00 +
Subject: [PATCH] [mlir][Transforms][NFC] Simplify `BlockTypeConversionRewrite`
---
.../Transforms/Utils/DialectConversion.cpp| 80 +--
1 file changed, 40 insertions(+), 40 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index b81495a95c80ed..cac990d498d7d3 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -746,24 +746,27 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
/// block is returned containing the new arguments. Returns `block` if it did
/// not require conversion.
FailureOr convertBlockSignature(
- Block *block, const TypeConverter *converter,
+ ConversionPatternRewriter &rewriter, Block *block,
+ const TypeConverter *converter,
TypeConverter::SignatureConversion *conversion = nullptr);
/// Convert the types of non-entry block arguments within the given region.
LogicalResult convertNonEntryRegionTypes(
- Region *region, const TypeConverter &converter,
+ ConversionPatternRewriter &rewriter, Region *region,
+ const TypeConverter &converter,
ArrayRef blockConversions = {});
/// Apply a signature conversion on the given region, using `converter` for
/// materializations if not null.
Block *
- applySignatureConversion(Region *region,
+ applySignatureConversion(ConversionPatternRewriter &rewriter, Region *region,
TypeConverter::SignatureConversion &conversion,
const TypeConverter *converter);
/// Convert the types of block arguments within the given region.
FailureOr
- convertRegionTypes(Region *region, const TypeConverter &converter,
+ convertRegionTypes(ConversionPatternRewriter &rewriter, Region *region,
+ const TypeConverter &converter,
TypeConverter::SignatureConversion *entryConversion);
/// Apply the given signature conversion on the given block. The new block
@@ -773,7 +776,8 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
/// translate between the origin argument types and those specified in the
/// signature conversion.
Block *applySignatureConversion(
- Block *block, const TypeConverter *converter,
+ ConversionPatternRewriter &rewriter, Block *block,
+ const TypeConverter *converter,
TypeConverter::SignatureConversion &signatureConversion);
//======//
@@ -940,24 +944,10 @@ void BlockTypeConversionRewrite::commit() {
rewriterImpl.mapping.lookupOrDefault(castValue, origArg.getType()));
}
}
-
- assert(origBlock->empty() && "expected empty block");
- origBlock->dropAllDefinedValueUses();
- delete origBlock;
- origBlock = nullptr;
}
void BlockTypeConversionRewrite::rollback() {
- // Drop all uses of the new block arguments and replace uses of the new
block.
- for (int i = block->getNumArguments() - 1; i >= 0; --i)
-block->getArgument(i).dropAllUses();
block->replaceAllUsesWith(origBlock);
-
- // Move the operations back the original block, move the original block back
- // into its original location and the delete the new block.
- origBlock->getOperations().splice(origBlock->end(), block->getOperations());
- block->getParent()->getBlocks().insert(Region::iterator(block), origBlock);
- eraseBlock(block);
}
LogicalResult BlockTypeConversionRewrite::materializeLiveConversions(
@@ -1173,10 +1163,11 @@ bool
ConversionPatternRewriterImpl::wasOpReplaced(Operation *op) const {
// Type Conversion
FailureOr ConversionPatternRewriterImpl::convertBlockSignature(
-Block *block, const TypeConverter *converter,
+ConversionPatternRewriter &rewriter, Block *block,
+const TypeConverter *converter,
TypeConverter::SignatureConversion *conversion) {
if (conversion)
-return applySignatureConversion(block, converter, *conversion);
+return applySignatureConversion(rewriter, block, converter, *conversion);
// If a converter
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Simplify `BlockTypeConversionRewrite` (PR #83286)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/83286 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Simplify `BlockTypeConversionRewrite` (PR #83286)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/83286 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Simplify handling of erased IR (PR #83423)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/83423
The dialect conversion uses a `SingleEraseRewriter` to ensure that an op/block
is not erased twice. This can happen during the "commit" phase when an
unresolved materialization is inserted into a block and the enclosing op is
erased by the user. In that case, the unresolved materialization should not be
erased a second time later in the "commit" phase.
This problem cannot happen during "rollback", so ops/block can be erased
directly without using the rewriter. With this change, the
`SingleEraseRewriter` is used only during "commit"/"cleanup". At that point,
the dialect conversion is guaranteed to succeed and no rollback can happen.
Therefore, it is not necessary to store the number of erased IR objects
(because we will never "reset" the rewriter to previous a previous state).
>From 6b6c4b1a7dd4943bfe2d97245e8369b9ba63aa20 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Thu, 29 Feb 2024 12:48:28 +
Subject: [PATCH] [mlir][Transforms][NFC] Do not use SingleEraseRewriter during
rollback
---
.../Transforms/Utils/DialectConversion.cpp| 22 +++
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index cac990d498d7d3..9f6468402686bd 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -153,9 +153,9 @@ namespace {
/// This is useful when saving and undoing a set of rewrites.
struct RewriterState {
RewriterState(unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numErased, unsigned numReplacedOps)
+unsigned numReplacedOps)
: numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
-numErased(numErased), numReplacedOps(numReplacedOps) {}
+numReplacedOps(numReplacedOps) {}
/// The current number of rewrites performed.
unsigned numRewrites;
@@ -163,9 +163,6 @@ struct RewriterState {
/// The current number of ignored operations.
unsigned numIgnoredOperations;
- /// The current number of erased operations/blocks.
- unsigned numErased;
-
/// The current number of replaced ops that are scheduled for erasure.
unsigned numReplacedOps;
};
@@ -273,8 +270,9 @@ class CreateBlockRewrite : public BlockRewrite {
auto &blockOps = block->getOperations();
while (!blockOps.empty())
blockOps.remove(blockOps.begin());
+block->dropAllUses();
if (block->getParent())
- eraseBlock(block);
+ block->erase();
else
delete block;
}
@@ -858,7 +856,7 @@ struct ConversionPatternRewriterImpl : public
RewriterBase::Listener {
void notifyBlockErased(Block *block) override { erased.insert(block); }
/// Pointers to all erased operations and blocks.
-SetVector erased;
+DenseSet erased;
};
//======//
@@ -1044,7 +1042,7 @@ void CreateOperationRewrite::rollback() {
region.getBlocks().remove(region.getBlocks().begin());
}
op->dropAllUses();
- eraseOp(op);
+ op->erase();
}
void UnresolvedMaterializationRewrite::rollback() {
@@ -1052,7 +1050,7 @@ void UnresolvedMaterializationRewrite::rollback() {
for (Value input : op->getOperands())
rewriterImpl.mapping.erase(input);
}
- eraseOp(op);
+ op->erase();
}
void UnresolvedMaterializationRewrite::cleanup() { eraseOp(op); }
@@ -1069,8 +1067,7 @@ void ConversionPatternRewriterImpl::applyRewrites() {
// State Management
RewriterState ConversionPatternRewriterImpl::getCurrentState() {
- return RewriterState(rewrites.size(), ignoredOps.size(),
- eraseRewriter.erased.size(), replacedOps.size());
+ return RewriterState(rewrites.size(), ignoredOps.size(), replacedOps.size());
}
void ConversionPatternRewriterImpl::resetState(RewriterState state) {
@@ -1081,9 +1078,6 @@ void
ConversionPatternRewriterImpl::resetState(RewriterState state) {
while (ignoredOps.size() != state.numIgnoredOperations)
ignoredOps.pop_back();
- while (eraseRewriter.erased.size() != state.numErased)
-eraseRewriter.erased.pop_back();
-
while (replacedOps.size() != state.numReplacedOps)
replacedOps.pop_back();
}
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Add listener support to dialect conversion (PR #83425)
https://github.com/matthias-springer created
https://github.com/llvm/llvm-project/pull/83425
This commit adds listener support to the dialect conversion. Similarly to the
greedy pattern rewrite driver, an optional listener can be specified in the
configuration object.
Listeners are notified only if the dialect conversion succeeds. In case of a
failure, where some IR changes are first performed and then rolled back, no
notifications are sent.
Due to the fact that some kinds of rewrite are reflected in the IR immediately
and some in a delayed fashion, there are certain limitations when attaching a
listener; these are documented in `ConversionConfig`. To summarize, users are
always notified about all rewrites that happened, but the notifications are
sent all at once at the very end, and not interleaved with the actual IR
changes.
This change is in preparation improvements to
`transform.apply_conversion_patterns`, which currently invalidates all handles.
In the future, it can use a listener to update handles accordingly, similar to
`transform.apply_patterns`.
>From 216e0b2d62e418cddcb6e4ecf6b07be361141131 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Thu, 29 Feb 2024 12:58:26 +
Subject: [PATCH] [mlir][Transforms] Add listener support to dialect conversion
---
.../mlir/Transforms/DialectConversion.h | 26 +++
.../Transforms/Utils/DialectConversion.cpp| 174 +-
mlir/test/Transforms/test-legalizer.mlir | 71 ++-
mlir/test/lib/Dialect/Test/TestPatterns.cpp | 28 ++-
4 files changed, 248 insertions(+), 51 deletions(-)
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index 84396529eb7c2e..98944b8a1ea648 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -1091,6 +1091,32 @@ struct ConversionConfig {
/// IR during an analysis conversion and only pre-existing operations are
/// added to the set.
DenseSet *legalizableOps = nullptr;
+
+ /// An optional listener that is notified about all IR modifications in case
+ /// dialect conversion succeeds. If the dialect conversion fails and no IR
+ /// modifications are visible (i.e., they were all rolled back), no
+ /// notifications are sent.
+ ///
+ /// Note: Notifications are sent in a delayed fashion, when the dialect
+ /// conversion is guaranteed to succeed. At that point, some IR modifications
+ /// may already have been materialized. Consequently, operations/blocks that
+ /// are passed to listener callbacks should not be accessed. (Ops/blocks are
+ /// guaranteed to be valid pointers and accessing op names is allowed. But
+ /// there are no guarantees about the state of ops/blocks at the time that a
+ /// callback is triggered.)
+ ///
+ /// Example: Consider a dialect conversion a new op ("test.foo") is created
+ /// and inserted, and later moved to another block. (Moving ops also triggers
+ /// "notifyOperationInserted".)
+ ///
+ /// (1) notifyOperationInserted: "test.foo" (into block "b1")
+ /// (2) notifyOperationInserted: "test.foo" (moved to another block "b2")
+ ///
+ /// When querying "op->getBlock()" during the first
"notifyOperationInserted",
+ /// "b2" would be returned because "moving an op" is a kind of rewrite that
is
+ /// immediately performed by the dialect conversion (and rolled back upon
+ /// failure).
+ RewriterBase::Listener *listener = nullptr;
};
//===--===//
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 9f6468402686bd..0048347cae9314 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -203,14 +203,22 @@ class IRRewrite {
/// Roll back the rewrite. Operations may be erased during rollback.
virtual void rollback() = 0;
- /// Commit the rewrite. Operations may be unlinked from their blocks during
- /// the commit phase, but they must not be erased yet. This is because
- /// internal dialect conversion state (such as `mapping`) may still be using
- /// them. Operations must be erased during cleanup.
- virtual void commit() {}
+ /// Commit the rewrite. At this point, it is certain that the dialect
+ /// conversion will succeed. All IR modifications, except for operation
+ /// erasure, must be performed through the given rewriter.
+ ///
+ /// Instead of erasing operations, they should merely be unlinked from their
+ /// blocks during the commit phase and finally be erased during the cleanup
+ /// phase. This is because internal dialect conversion state (such as
+ /// `mapping`) may still be using them.
+ ///
+ /// Any IR modification that was already performed before the commit phase
+ /// (e.g., insertion of an op) must be communicated to the listener that may
+ /// be attache
[llvm-branch-commits] [mlir] [mlir][Transforms] Add listener support to dialect conversion (PR #83425)
https://github.com/matthias-springer updated
https://github.com/llvm/llvm-project/pull/83425
>From 22e8d5ea80b74d687c3e792c512f50b9de81f489 Mon Sep 17 00:00:00 2001
From: Matthias Springer
Date: Thu, 29 Feb 2024 12:58:26 +
Subject: [PATCH] [mlir][Transforms] Add listener support to dialect conversion
---
.../mlir/Transforms/DialectConversion.h | 26 +++
.../Transforms/Utils/DialectConversion.cpp| 174 +-
mlir/test/Transforms/test-legalizer.mlir | 71 ++-
mlir/test/lib/Dialect/Test/TestPatterns.cpp | 28 ++-
4 files changed, 248 insertions(+), 51 deletions(-)
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h
b/mlir/include/mlir/Transforms/DialectConversion.h
index 84396529eb7c2e..98944b8a1ea648 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -1091,6 +1091,32 @@ struct ConversionConfig {
/// IR during an analysis conversion and only pre-existing operations are
/// added to the set.
DenseSet *legalizableOps = nullptr;
+
+ /// An optional listener that is notified about all IR modifications in case
+ /// dialect conversion succeeds. If the dialect conversion fails and no IR
+ /// modifications are visible (i.e., they were all rolled back), no
+ /// notifications are sent.
+ ///
+ /// Note: Notifications are sent in a delayed fashion, when the dialect
+ /// conversion is guaranteed to succeed. At that point, some IR modifications
+ /// may already have been materialized. Consequently, operations/blocks that
+ /// are passed to listener callbacks should not be accessed. (Ops/blocks are
+ /// guaranteed to be valid pointers and accessing op names is allowed. But
+ /// there are no guarantees about the state of ops/blocks at the time that a
+ /// callback is triggered.)
+ ///
+ /// Example: Consider a dialect conversion a new op ("test.foo") is created
+ /// and inserted, and later moved to another block. (Moving ops also triggers
+ /// "notifyOperationInserted".)
+ ///
+ /// (1) notifyOperationInserted: "test.foo" (into block "b1")
+ /// (2) notifyOperationInserted: "test.foo" (moved to another block "b2")
+ ///
+ /// When querying "op->getBlock()" during the first
"notifyOperationInserted",
+ /// "b2" would be returned because "moving an op" is a kind of rewrite that
is
+ /// immediately performed by the dialect conversion (and rolled back upon
+ /// failure).
+ RewriterBase::Listener *listener = nullptr;
};
//===--===//
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 9f6468402686bd..0048347cae9314 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -203,14 +203,22 @@ class IRRewrite {
/// Roll back the rewrite. Operations may be erased during rollback.
virtual void rollback() = 0;
- /// Commit the rewrite. Operations may be unlinked from their blocks during
- /// the commit phase, but they must not be erased yet. This is because
- /// internal dialect conversion state (such as `mapping`) may still be using
- /// them. Operations must be erased during cleanup.
- virtual void commit() {}
+ /// Commit the rewrite. At this point, it is certain that the dialect
+ /// conversion will succeed. All IR modifications, except for operation
+ /// erasure, must be performed through the given rewriter.
+ ///
+ /// Instead of erasing operations, they should merely be unlinked from their
+ /// blocks during the commit phase and finally be erased during the cleanup
+ /// phase. This is because internal dialect conversion state (such as
+ /// `mapping`) may still be using them.
+ ///
+ /// Any IR modification that was already performed before the commit phase
+ /// (e.g., insertion of an op) must be communicated to the listener that may
+ /// be attached to the given rewriter.
+ virtual void commit(RewriterBase &rewriter) {}
/// Cleanup operations. Cleanup is called after commit.
- virtual void cleanup() {}
+ virtual void cleanup(RewriterBase &rewriter) {}
Kind getKind() const { return kind; }
@@ -220,12 +228,6 @@ class IRRewrite {
IRRewrite(Kind kind, ConversionPatternRewriterImpl &rewriterImpl)
: kind(kind), rewriterImpl(rewriterImpl) {}
- /// Erase the given op (unless it was already erased).
- void eraseOp(Operation *op);
-
- /// Erase the given block (unless it was already erased).
- void eraseBlock(Block *block);
-
const ConversionConfig &getConfig() const;
const Kind kind;
@@ -264,6 +266,12 @@ class CreateBlockRewrite : public BlockRewrite {
return rewrite->getKind() == Kind::CreateBlock;
}
+ void commit(RewriterBase &rewriter) override {
+// The block was already created and inserted. Just inform the listener.
+if (auto *listener = rewriter.getListener())
+ lis
[llvm-branch-commits] [mlir] [mlir][Transforms] Encapsulate dialect conversion options in `ConversionConfig` (PR #83754)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/83754 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][Transforms] Add listener support to dialect conversion (PR #83425)
https://github.com/matthias-springer edited https://github.com/llvm/llvm-project/pull/83425 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
