At its core, this commit changes `OpaqueProperties` (aka a void*) to
`PropertyRef`, which is a {TypeID, void*}, where the TypeID is the ID of
the storage type of the given property (which can, as is often the case
for operations, be a struct of other properties).
Long-term, this change will allow for
1) Some sort of getFooPropertyRef() on property structs, allowing
individual members to be extracted generically
2) By having a property kind that is an OwningProprtyRef, generic
parsing (in combination with a bunch of other changes) 3) Probably a
safer C/Python API because we'll be able to indicate what's supposed to
be under a given void*
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This is based on empirical data from compiling 9 medium to large
language and diffusion models with IREE. e2e, this improves compilation
times by 0.33% in terms of `instructions:u` (same metric is used by the
[CTMark for
Clang](https://www.npopov.com/2024/01/01/This-year-in-LLVM-2023.html#compile-time-improvements)).
I explored using other constants and these are the ones that performed
best while keeping the sizes relatively small.
When running dialect conversion with --no-implicit-module, the root op
is
parsed without a wrapping module and then detached from its temporary
parsing
block (block == nullptr). If a conversion pattern replaces this detached
root
op, ReplaceOperationRewrite::commit() would crash with a null pointer
dereference when calling op->getBlock()->getOperations().remove(op).
Fix this with two complementary changes:
1. In ReplaceOperationRewrite::commit(), add a guard that calls
reportFatalInternalError when op->getBlock() is null. This turns the
opaque null-pointer crash into a clear diagnostic pointing at the API
misuse.
2. Make --convert-func-to-spirv explicitly reject detached top-level ops
at
pass startup with a clear diagnostic rather than letting the conversion
framework abort with a fatal error.
Add a regression test in mlir/test/Conversion/ConvertToSPIRV/ that
verifies
the diagnostic is emitted instead of crashing.
Fixes#60491
Assisted-by: Claude Code
Some function ops (e.g., gpu.func with workgroup memory arguments) have
more entry block arguments than their FunctionType has inputs. The
workgroup memory arguments are not part of the public function signature
but are present as additional block arguments.
`convertFuncOpTypes` previously created a `SignatureConversion` sized
only for `type.getNumInputs()`, then called `applySignatureConversion`
on the entry block. When the block had more arguments (e.g., workgroup
args), the loop in `applySignatureConversion` would call
`getInputMapping(i)` with out-of-bounds indices, causing an assertion
failure in `SmallVector::operator[]`.
Fix this by:
1. Sizing the `SignatureConversion` for all entry block arguments.
2. Adding identity mappings for extra block args beyond the function
type inputs.
3. Using only the converted function-type-input types when updating the
FunctionType (so extra block arg types are not included in the
signature).
Fixes#184744
Assisted-by: Claude Code
This PR improves MLIR dialect conversion failure diagnostics when
legalization fails.
Previously, the diagnostic mostly included the operation name (and in
partial conversion, whether it was explicitly marked illegal). This
change keeps that prefix and appends the printed failing operation. This
provides immediate operand/result/type context directly in the same
error line.
### Example
Before:
```
failed to legalize operation 'test.type_consumer' that was explicitly marked illegal
```
After:
```
failed to legalize operation 'test.type_consumer' that was explicitly marked illegal: "test.type_consumer"(%arg0) : (f32) -> ()
```
### Tests
- Updated `mlir/test/Transforms/test-legalizer.mlir` expectations for
the richer emitted diagnostic.
Migrate from deprecated report_fatal_error to reportFatalInternalError
in DialectConversion.cpp. All 6 instances are internal consistency
checks in MLIR's dialect conversion system, so reportFatalInternalError
is the appropriate replacement.
Part of #138914
Prior to this change, rollback of the `MoveBlockRewrite` could result in
segfault if the block wasn't contained in a region anymore.
That situation could arise if the previous rollback of another rewrite
orphaned the block by removing it from its region, as demonstrated by
the new test pattern.
Signed-off-by: Lukas Sommer <lukas.sommer@amd.com>
This commit align the implementation of
`ConversionPatternRewriter::legalize` with its documentation:
```
/// Attempt to legalize the given region. This can be used within
...
LogicalResult legalize(Region *r);
```
This function now legalizes the entire region, including nested ops. The
implementation follows the same logic as the "main" traversal:
pre-order, forward-dominance.
This commit adds support for `replaceUsesWithIf` (and variants such as
`replaceAllUsesExcept`) to the `ConversionPatternRewriter`. This API is
supported only in no-rollback mode. An assertion is triggered in
rollback mode. (This missing assertion has been confusing for users
because it seemed that the API supported, while it was actually not
working properly.)
This commit brings us a bit closer towards removing
[this](76ec25f729/mlir/lib/Transforms/Utils/DialectConversion.cpp (L1214))
workaround.
Additional changes are needed to support this API in rollback mode. In
particular, no entries should be added to the `ConversionValueMapping`
for conditional replacements. It's unclear at this point if this API can
be supported in rollback mode, so this is deferred to later.
This commit turns `replaceUsesWithIf` into a virtual function, so that
the `ConversionPatternRewriter` can override it. All other API functions
for conditional value replacements call that function.
Note for LLVM integration: If you are seeing failed assertions due to
this change, you are using unsupported API in your dialect conversion.
You have 3 options: (1) Migrate to the no-rollback driver. (2) Rewrite
your patterns without the unsupported API. (3) Last resort: bypass the
rewriter and call `replaceUsesWithIf` etc. directly on the `Value`
object.
When both `MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS` and MLIR
multithreading are enabled, `topLevelFingerPrint` is empty but its value
is accessed. This adds a `has_value()` check before dereferencing the
optional.
Fixes a bug causing every conversion to fail fatally with "expected
pattern to replace the root operation or modify it in place" when
`MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS` is enabled and pattern
rollback is disabled.
When `allowPatternRollback` is disabled, the rewriter does not keep
track of the rewrites it performs and can therefore not use that list to
check whether the root op was replaced or updated in place.
By default, the dialect conversion driver processes operations in
pre-order: the initial worklist is populated pre-order. (New/modified
operations are immediately legalized recursively.)
This commit adds a new API for selective post-order legalization.
Patterns can request an operation / region legalization via
`ConversionPatternRewriter::legalize`. They can call these helper
functions on nested regions before rewriting the operation itself.
Note: In rollback mode, a failed recursive legalization typically leads
to a conversion failure. Since recursive legalization is performed by
separate pattern applications, there is no way for the original pattern
to recover from such a failure.
When converting a function, convert only the entry block signature. The
remaining block signatures should be converted by the respective
branching ops. The `FuncToLLVM` / `ControlFlowToLLVM` patterns already
use that design.
```c++
struct BranchOpLowering : public ConvertOpToLLVMPattern<cf::BranchOp> {
LogicalResult
matchAndRewrite(cf::BranchOp op, OneToNOpAdaptor adaptor,
ConversionPatternRewriter &rewriter) const override {
// Convert successor block.
SmallVector<Value> flattenedAdaptor = flattenValues(adaptor.getOperands());
FailureOr<Block *> convertedBlock =
getConvertedBlock(rewriter, getTypeConverter(), op, op.getSuccessor(),
TypeRange(ValueRange(flattenedAdaptor)));
// ...
}
};
```
This is consistent with the fact that operations from unreachable blocks
are not put on the initial worklist.
With this change, parent ops are no longer recursively legalized when
inserting a block, simplifying the conversion driver a bit.
Note for LLVM integration: If you are seeing failures, make sure to:
- Drop `converter.isLegal(&op.getBody())` when checking the legality of
a function op. Only the entry block signature / function type should be
taken into account.
- If you need to convert all reachable blocks and are using `cf`
branching ops, add `populateCFStructuralTypeConversionsAndLegality`.
- If you need to convert all reachable blocks and are using custom
branching ops, implement and populate custom structural type conversion
patterns, similar to `populateCFStructuralTypeConversionsAndLegality`.
When running with `-debug`, print a note when the replacement types
(during a `ConversionPatternRewriter::replaceOp`) do not match the
legalized types of the current type converter. That's not an API
violation, but it could indicate a bug in user code.
Example output:
```
[dialect-conversion:1] ** Replace : 'test.multiple_1_to_n_replacement'(0x56b745f99470)
[dialect-conversion:1] Note: Replacing op result of type f16 with value(s) of type (f16, f16), but the legalized type(s) is/are (f16)
```
Move the logic for building "out-of-thin-air" source materializations
during op replacements from `replaceOp` to
`findOrBuildReplacementValue`. That function already builds source
materializations and can handle the case where an op result is dropped.
This commit is in preparation of turning `replaceOp` into a non-virtual
function. (It is sufficient for `replaceAllUsesWith` and `eraseOp` to be
virtual.)
The `reconcile-unrealized-casts` pass used to crash when the input
contains circular chains of `unrealized_conversion_cast` ops.
Furthermore, the `reconcileUnrealizedCasts` helper functions used to
erase ops that were not passed via the `castOps` operand. Such ops are
now preserved. That's why some integration tests had to be changed.
Also avoid copying the set of all unresolved materializations in
`convertOperations`.
This commit is in preparation of turning `RewriterBase::replaceOp` into
a non-virtual function.
This is a re-upload of #158067, which was reverted due to CI failures.
Note for LLVM integration: If you are seeing tests that are failing with
`error: LLVM Translation failed for operation:
builtin.unrealized_conversion_cast`, you may have to add the
`-reconcile-unrealized-casts` pass to your pass pipeline. (Or switch to
the `-convert-to-llvm` pass instead of combining the various
`-convert-*-to-llvm` passes.)
---------
Co-authored-by: Mehdi Amini <joker.eph@gmail.com>
The `reconcile-unrealized-casts` pass used to crash when the input
contains circular chains of `unrealized_conversion_cast` ops.
Furthermore, the `reconcileUnrealizedCasts` helper functions used to
erase ops that were not passed via the `castOps` operand. Such ops are
now preserved. That's why some integration tests had to be changed.
Also avoid copying the set of all unresolved materializations in
`convertOperations`.
This commit is in preparation of turning `RewriterBase::replaceOp` into
a non-virtual function.
---------
Co-authored-by: Mehdi Amini <joker.eph@gmail.com>
The current implementation is overly conservative and disable all
possible caching as soon as a context-aware conversion is present.
However the context-aware conversion only affects subsequent converters,
we can cache the previous ones.
This isn't NFC because if fixed a bug where we use to unconditionally
cache when using the `convertType(Type t, ...` API, while now all APIs
are aware of context-aware conversions.
This commit generalizes `replaceUsesOfBlockArgument` to
`replaceAllUsesWith`. In rollback mode, the same restrictions keep
applying: a value cannot be replaced multiple times and a call to
`replaceAllUsesWith` will replace all current and future uses of the
`from` value.
`replaceAllUsesWith` is now fully supported and its behavior is
consistent with the remaining dialect conversion API. Before this
commit, `replaceAllUsesWith` was immediately reflected in the IR when
running in rollback mode. After this commit, `replaceAllUsesWith`
changes are materialized in a delayed fashion, at the end of the dialect
conversion. This is consistent with the `replaceUsesOfBlockArgument` and
`replaceOp` APIs.
`replaceAllUsesExcept` etc. are still not supported and will be
deactivated on the `ConversionPatternRewriter` (when running in rollback
mode) in a follow-up commit.
Note for LLVM integration: Replace `replaceUsesOfBlockArgument` with
`replaceAllUsesWith`. If you are seeing failures, you may have patterns
that use `replaceAllUsesWith` incorrectly (e.g., being called multiple
times on the same value) or bypass the rewriter API entirely. E.g., such
failures were mitigated in Flang by switching to the walk-patterns
driver (#156171).
You can temporarily reactivate the old behavior by calling
`RewriterBase::replaceAllUsesWith`. However, note that that behavior is
faulty in a dialect conversion. E.g., the base
`RewriterBase::replaceAllUsesWith` implementation does not see uses of
the `from` value that have not materialized yet and will, therefore, not
replace them.
Many internal functions take a `ConversionPatternRewriter &` or
`ConversionPatternRewriterImpl &` as a parameter. There's only a single
instance of these classes, so it's better to store the reference in a
field. This commit is in preparation of another PR that will require
access to `ConversionPatternRewriter` in additional helper functions.
Note: Public API does not change.
This commit adds support for context-aware type conversions: type
conversion rules that can return different types depending on the IR.
There is no change for existing (context-unaware) type conversion rules:
```c++
// Example: Conversion any integer type to f32.
converter.addConversion([](IntegerType t) {
return Float32Type::get(t.getContext());
}
```
There is now an additional overload to register context-aware type
conversion rules:
```c++
// Example: Type conversion rule for integers, depending on the context:
// Get the defining op of `v`, read its "increment" attribute and return an
// integer with a bitwidth that is increased by "increment".
converter.addConversion([](Value v) -> std::optional<Type> {
auto intType = dyn_cast<IntegerType>(v.getType());
if (!intType)
return std::nullopt;
Operation *op = v.getDefiningOp();
if (!op)
return std::nullopt;
auto incrementAttr = op->getAttrOfType<IntegerAttr>("increment");
if (!incrementAttr)
return std::nullopt;
return IntegerType::get(v.getContext(),
intType.getWidth() + incrementAttr.getInt());
});
```
For performance reasons, the type converter caches the result of type
conversions. This is no longer possible when there context-aware type
conversions because each conversion could compute a different type
depending on the context. There is no performance degradation when there
are only context-unaware type conversions.
Note: This commit just adds context-aware type conversions to the
dialect conversion framework. There are many existing patterns that
still call `converter.convertType(someValue.getType())`. These should be
gradually updated in subsequent commits to call
`converter.convertType(someValue)`.
Co-authored-by: Markus Böck <markus.boeck02@gmail.com>
Improve the documentation of `replaceUsesOfBlockArgument` to clarify its
semantics is rollback mode. Add an assertion to make sure that the same
block argument is not replaced multiple times. That's an API violation
and messes with the internal state of the conversion driver.
This commit is in preparation of adding full support for
`RewriterBase::replaceAllUsesWith`.
Add a debugging flag to the dialect conversion to dump the
materialization kind. This flag is useful to find out whether a missing
materialization rule is for source or target materializations.
Also add missing test coverage for the `buildMaterializations` flag.
Prior to this PR, the default behaviour of a conversion pattern which
receives operands of a 1:N is to abort the compilation. This has
historically been useful when the 1:N type conversion got merged into
the dialect conversion as it allowed us to easily find patterns that
should be capable of handling 1:N type conversions but didn't.
However, this behaviour has the disadvantage of being non-composable:
While the pattern in question cannot handle the 1:N type conversion,
another pattern part of the set might, but doesn't get the chance as
compilation is aborted.
This PR fixes this behaviour by failing to match and instead of
aborting, giving other patterns the chance to legalize an op. The
implementation uses a reusable function called `dispatchTo1To1` to allow
derived conversion patterns to also implement the behaviour.
This commit improves the `allowPatternRollback` flag handling in the
dialect conversion driver. Previously, this flag was used to merely
detect cases that are incompatible with the new One-Shot Dialect
Conversion driver. This commit implements the driver itself: when the
flag is set to "false", all IR changes are materialized immediately,
bypassing the `IRRewrite` and `ConversionValueMapping` infrastructure.
A few selected test cases now run with both the old and the new driver.
RFC:
https://discourse.llvm.org/t/rfc-a-new-one-shot-dialect-conversion-driver/79083
Previously this only happened post checking if the op is legal, but was
done unconditionally post (and before other legalization patterns). Add
option to not attempt folding and one to do so as last resort.
Did consider but did not add a always attempt to fold option (which
would have folded whether or not legal), but removed TODO about it.
Print a more detailed error message when new/modified IR could not be
legalized with `allowPatternRollback = false`. This is useful to
understand why a pattern is incompatible with the new One-Shot Dialect
Conversion driver.
---------
Co-authored-by: Jeremy Kun <jkun@google.com>
When a conversion pattern is initialized without a type converter, the
driver implementation currently looks up the most recently mapped value.
This is undesirable because the most recently mapped value could be a
materialization. I.e., the type of the value being looked up could
depend on which other patterns have run before. Such an implementation
makes the type conversion infrastructure fragile and unpredictable.
The current implementation also contradicts the documentation in the
markdown file. According to that documentation, the values provided by
the adaptor should match the types of the operands of the match
operation when running without a type converter. This mechanism is not
desirable, either, for two reasons:
1. Some patterns have started to rely on receiving the most recently
mapped value. Changing the behavior to the documented behavior will
cause regressions. (And there would be no easy way to fix those without
forcing the use of a type converter or extending the `getRemappedValue`
API.)
2. It is more useful to receive the most recently mapped value. A value
of the original operand type can be retrieved by using the operand of
the matched operation. The adaptor is not needed at all in that case.
To implement the new behavior, materializations are now annotated with a
marker attribute. The marker is needed because not all
`unrealized_conversion_cast` ops are materializations that act as "pure
type conversions". E.g., when erasing an operation, its results are
mapped to newly-created "out-of-thin-air values", which are
materializations (with no input) that should be treated like regular
replacement values during a lookup. This marker-based lookup strategy is
also compatible with the One-Shot Dialect Conversion implementation
strategy, which does not utilize the mapping infrastructure anymore and
queries all necessary information by examining the IR.
Add a helper function to `ConversionPatternRewriter` that returns the
dialect conversion configuration. This flag is useful when migrating
conversion patterns to the new One-Shot Conversion Driver: patterns can
check if they are running in rollback mode or not. They can then work
around API changes and makes sure that the pattern keeps working with
both the old and new driver.
Also remove the `config` field from `OperationLegalizer`. That field was
never needed.
This prefix the output with the DEBUG_TYPE.
Dialect conversion is using a ScopedPrinter, we insert the
raw_ldbg_ostream to consistently prefix each new line.
Operation folders can do two things:
1. Modify IR (in-place op modification). Failing to legalize an in-place
folded operation does not trigger an immediate rollback. This happens
only if the driver decides to try a different lowering path, requiring
it to roll back a bunch of modifications, including the application of
the folder.
2. Create new IR (constant op materialization of a folded attribute).
Failing to legalize a newly created constant op triggers an immediate
rollback.
In-place op modifications should be guarded by
`startOpModification`/`finalizeOpModification` because they are no
different from other in-place op modifications. (They just happen
outside of a pattern, but that does not mean that we should not track
those changes; we are tracking everything else.) This commit adds those
two function calls.
This commit also moves the `rewriter.replaceOp(op, replacementValues);`
function call before the loop nest that legalizes the newly created
constant ops (and therefore `replacementValues`). Conceptually, the
folded op must be replaced before attempting to legalize the constants
because the constant ops may themselves be replaced as part of their own
legalization process. The previous implementation happened to work in
the current conversion driver, but is incompatible with the One-Shot
Dialect Conversion driver, which expects to see the most recent IR at
all time.
From an end-user perspective, this commit should be NFC. A common
folder-rollback pattern that is exercised by multiple tests cases: A
`memref.dim` is folded to `arith.constant`, but `arith.constant` is not
marked as legal as per the conversion target, triggering a rollback.
Note: Folding is generally unsafe in a dialect conversion (see #92683),
but that's a different issue. (In a One-Shot Dialect Conversion, it will
no longer be unsafe.)
Erasing the operation to which the current insertion point is set,
leaves the insertion point in an invalid state. This commit resets the
insertion point to the following operation.
Also adjust the insertion point when inlining a block.
This commit makes some minor NFC-style improvements to the
`notifyBlockInserted` and `notifyOperationInserted` implementations:
* Rename some variables.
* Add more comments and document the fact the current mechanism has a
bug when running in "rollback allowed" mode.
* Move some code from the `notify...` functions into the constructor of
the respective `IRRewrite` objects. This is in preparation of the
One-Shot Dialect Conversion refactoring. The moved pieces of code are
not needed in "no rollback" mode and properly encapsulated inside of
`IRRewrite`, which is also not needed in "no rollback" mode.
* Slightly improve `-debug` output.
Add a new "expensive check" when running with `allowPatternRollback =
false`: returning "failure" after modifying IR is no longer allowed.
This check detects a few more API violations in addition to the check
`undoRewrites`. The latter check will be removed soon. (Because the
One-Shot Dialect Conversion will no longer maintain the stack of IR
rewrites.)
Also fix a build error when expensive checks are enabled.
Add `lookupOrDefault` / `lookupOrNull` wrappers to
`ConversionPatternRewriterImpl` and call those wrappers throughout the
code base.
This commit is in preparation of the One-Shot Dialect Conversion
refactoring. In future, the implementation will bypass the `mapping`
when rollback is disabled. The switch will be made in those wrapper
functions.
Store metadata about unresolved materializations in a separate data
structure. This is in preparation of the One-Shot Dialect Conversion
refactoring, which no longer maintains a stack of `IRRewrite` objects.
Therefore, metadata about unresolved materializations can no longer be
retrieved from `UnresolvedMaterializationRewrite` objects.
This commit also removes a pointer indirection and may slightly improve
the performance of the existing driver.
Report all `allowPatternRollback` API violations as fatal errors. If
violated, the IR is potentially in an invalid/inconsistent state from
which the driver cannot recover.
