This commit adds an extra assertion to `applySignatureConversion` to
prevent incorrect API usage: The same block cannot be converted multiple
times. That would mess with the underlying conversion value mapping.
(Mappings would be overwritten.) This is similar to op replacements: The
same op cannot be replaced multiple times.
To simplify the check, `BlockTypeConversionRewrite::block` now stores
the original block. The new block is stored in an extra field. (It used
to be the other way around.)
This commit is in preparation of adding 1:N support to the conversion
value mapping. Before making any further changes to the mapping
infrastructure, I'd like to make sure that the code base around it (that
uses the mapping) is robust.
This is a re-upload of #116934, which was reverted.
The dialect conversion driver has three phases:
- **Create** `IRRewrite` objects as the IR is traversed.
- **Finalize** `IRRewrite` objects. During this phase, source
materializations for mismatching value types are created. (E.g., when
`Value` is replaced with a `Value` of different type, but there is a
user of the original value that was not modified because it is already
legal.)
- **Commit** `IRRewrite` objects. During this phase, all remaining IR
modifications are materialized. In particular, SSA values are actually
being replaced during this phase.
This commit removes the "finalize" phase. This simplifies the code base
a bit and avoids one traversal over the `IRRewrite` stack. Source
materializations are now built during the "commit" phase, right before
an SSA value is being replaced.
This commit also removes the "inverse mapping" of the conversion value
mapping, which was used to predict if an SSA value will be dead at the
end of the conversion. This check is replaced with an approximate check
that does not require an inverse mapping. (A false positive for `v` can
occur if another value `v2` is mapped to `v` and `v2` turns out to be
dead at the end of the conversion. This case is not expected to happen
very often.) This reduces the complexity of the driver a bit and removes
one potential source of bugs. (There have been bugs in the usage of the
inverse mapping in the past.)
`BlockTypeConversionRewrite` no longer stores a pointer to the type
converter. This pointer is now stored in `ReplaceBlockArgRewrite`.
This commit is in preparation of merging the 1:1 and 1:N dialect
conversion driver. It simplifies the upcoming changes around the
conversion value mapping. (API surface of the conversion value mapping
is reduced.)
The dialect conversion driver has three phases:
- **Create** `IRRewrite` objects as the IR is traversed.
- **Finalize** `IRRewrite` objects. During this phase, source
materializations for mismatching value types are created. (E.g., when
`Value` is replaced with a `Value` of different type, but there is a
user of the original value that was not modified because it is already
legal.)
- **Commit** `IRRewrite` objects. During this phase, all remaining IR
modifications are materialized. In particular, SSA values are actually
being replaced during this phase.
This commit removes the "finalize" phase. This simplifies the code base
a bit and avoids one traversal over the `IRRewrite` stack. Source
materializations are now built during the "commit" phase, right before
an SSA value is being replaced.
This commit also removes the "inverse mapping" of the conversion value
mapping, which was used to predict if an SSA value will be dead at the
end of the conversion. This check is replaced with an approximate check
that does not require an inverse mapping. (A false positive for `v` can
occur if another value `v2` is mapped to `v` and `v2` turns out to be
dead at the end of the conversion. This case is not expected to happen
very often.) This reduces the complexity of the driver a bit and removes
one potential source of bugs. (There have been bugs in the usage of the
inverse mapping in the past.)
`BlockTypeConversionRewrite` no longer stores a pointer to the type
converter. This pointer is now stored in `ReplaceBlockArgRewrite`.
This commit is in preparation of merging the 1:1 and 1:N dialect
conversion driver. It simplifies the upcoming changes around the
conversion value mapping. (API surface of the conversion value mapping
is reduced.)
This commit adds a new function
`ConversionPatternRewriter::replaceOpWithMultiple`. This function is
similar to `replaceOp`, but it accepts multiple `ValueRange`
replacements, one per op result.
Note: This function is not an overload of `replaceOp` because of
ambiguous overload resolution that would make the API difficult to use.
This commit aligns "block signature conversions" with "op replacements":
both support 1:N replacements now. Due to incomplete 1:N support in the
dialect conversion driver, an argument materialization is inserted when
an SSA value is replaced with multiple values; same as block signature
conversions already work around the problem. These argument
materializations are going to be removed in a subsequent commit that
adds full 1:N support. The purpose of this PR is to add missing features
gradually in small increments.
This commit also updates two MLIR transformations that have their custom
workarounds around missing 1:N support. These can already start using
`replaceOpWithMultiple`.
Co-authored-by: Markus Böck <markus.boeck02@gmail.com>
This commit changes the format of the materialization error message.
Previously: `failed to legalize unresolved materialization from ('f64')
to 'f32' that remained live after conversion`
Now: `failed to legalize unresolved materialization from ('f64') to
('f32') that remained live after conversion`
This commit is in preparation of merging the 1:1 and 1:N dialect
conversions. At that point, target materializations may create more than
one SSA value. I am sending this change as a separate PR to keep the
main PR smaller.
This commit adds extra checks/assertions to the
`ConversionPatternRewriterImpl::notifyOpReplaced` to improve its
robustness.
1. Replacing an `unrealized_conversion_cast` op that was created by the
driver is now forbidden and caught early during `replaceOp`. It may work
in some cases, but it is generally dangerous because the conversion
driver keeps track of these ops and performs some extra legalization
steps during the "finalize" phase. (Erasing is them is fine.)
2. `null` replacement values are no longer registered in the
`ConversionValueMapping`. This was an oversight in #106760. There is no
benefit in having `null` values in the `ConversionValueMapping`. (It may
even cause problems.)
This change is in preparation of merging the 1:1 and 1:N dialect
conversion drivers.
The 1:N type converter derived from the 1:1 type converter and extends
it with 1:N target materializations. This commit merges the two type
converters and stores 1:N target materializations in the 1:1 type
converter. This is in preparation of merging the 1:1 and 1:N dialect
conversion infrastructures.
1:1 target materializations (producing a single `Value`) will remain
valid. An additional API is added to the type converter to register 1:N
target materializations (producing a `SmallVector<Value>`). Internally,
all target materializations are stored as 1:N materializations.
The 1:N type converter is removed.
Note for LLVM integration: If you are using the `OneToNTypeConverter`,
simply switch all occurrences to `TypeConverter`.
---------
Co-authored-by: Markus Böck <markus.boeck02@gmail.com>
This commit simplifies the result type of materialization functions.
Previously: `std::optional<Value>`
Now: `Value`
The previous implementation allowed 3 possible return values:
- Non-null value: The materialization function produced a valid
materialization.
- `std::nullopt`: The materialization function failed, but another
materialization can be attempted.
- `Value()`: The materialization failed and so should the dialect
conversion. (Previously: Dialect conversion can roll back.)
This commit removes the last variant. It is not particularly useful
because the dialect conversion will fail anyway if all other
materialization functions produced `std::nullopt`.
Furthermore, in contrast to type conversions, at least one
materialization callback is expected to succeed. In case of a failing
type conversion, the current dialect conversion can roll back and try a
different pattern. This also used to be the case for materializations,
but that functionality was removed with #107109: failed materializations
can no longer trigger a rollback. (They can just make the entire dialect
conversion fail without rollback.) With this in mind, it is even less
useful to have an additional error state for materialization functions.
This commit is in preparation of merging the 1:1 and 1:N type
converters. Target materializations will have to return multiple values
instead of a single one. With this commit, we can keep the API simple:
`SmallVector<Value>` instead of `std::optional<SmallVector<Value>>`.
Note for LLVM integration: All 1:1 materializations should return
`Value` instead of `std::optional<Value>`. Instead of `std::nullopt`
return `Value()`.
This commit adds an optional `originalType` parameter to target
materialization functions. Without this parameter, target
materializations are underspecified.
Note: `originalType` is only needed for target materializations.
Source/argument materializations do not have it.
Consider the following example: Let's assume that a conversion pattern
"P1" replaced an SSA value "v1" (type "t1") with "v2" (type "t2"). Then
a different conversion pattern "P2" matches an op that has "v1" as an
operand. Let's furthermore assume that "P2" determines that the
legalized type of "t1" is "t3", which may be different from "t2". In
this example, the target materialization callback will be invoked with:
outputType = "t3", inputs = "v2", originalType = "t1". Note that the
original type "t1" cannot be recovered from just "t3" and "v2"; that's
why the `originalType` parameter is added.
This change is in preparation of merging the 1:1 and 1:N dialect
conversion drivers. As part of that change, argument materializations
will be removed (as they are no longer needed; they were just a
workaround because of missing 1:N support in the dialect conversion).
The new `originalType` parameter is needed when lowering MemRef to LLVM.
During that lowering, MemRef function block arguments are replaced with
the elements that make up a MemRef descriptor. The type converter is set
up in such a way that the legalized type of a MemRef type is an
`!llvm.struct` that represents the MemRef descriptor. When the bare
pointer calling convention is enabled, the function block arguments
consist of just an LLVM pointer. In such a case, a target
materialization will be invoked to construct a MemRef descriptor (output
type = `!llvm.struct<...>`) from just the bare pointer (inputs =
`!llvm.ptr`). The original MemRef type is required to construct the
MemRef descriptor, as static sizes/strides/offset cannot be inferred
from just the bare pointer.
This commit changes the implementation of analysis conversions, so that
no rollback is needed at the end of the analysis. Instead, the dialect conversion is run on a clone of the IR.
The purpose of this commit is to reduce the number of rollbacks in the
dialect conversion framework. (Long term goal: Remove rollback
functionality entirely.)
This commit simplifies the signature of `OperationConverter::finalize`.
This function always returns "success", so the return value can be
removed.
Note: Previously, this function used to return "failure" if a
materialization failed to legalize. This is now optional and happening
at a later point of time (see `config.buildMaterializations`).
Simplify the nesting structure of "if" checks in `remapValues` and
update the code comments.
This is what the comments stated in case there is no type converter:
```
// TODO: What we should do here is just set `desiredType` to `origType`
// and then handle the necessary type conversions after the conversion
// process has finished. Unfortunately a lot of patterns currently rely on
// receiving the new operands even if the types change, so we keep the
// original behavior here for now until all of the patterns relying on
// this get updated.
```
However, without a type converter it is not possible to perform any
materializations. Furthermore, the absence of a type converter indicates
that the pattern does not care about type legality. Therefore, the
current implementation is correct and this TODO can be removed.
Note: Patterns that actually require a remapped type to match the
original operand type can be equipped with a type converter that maps
each type to itself.
This TODO is outdated:
```
// TODO: There currently isn't any mechanism to do 1->N type conversion
// via the PatternRewriter replacement API, so for now we just ignore it.
```
1->N type conversions are already possible as part of block signature
conversions. It is incorrect to just ignore such cases. However, there
is currently no better way to handle 1->N conversions in this function
because of infrastructure limitations. This is now clarified in the
comments.
Remove a redundant `lookupOrDefault` that has no effect.
When no type is passed to `lookupOrDefault`, that function returns the
furthest mapped value (by following the mapping iteratively). If there
is no mapped value with the desired type, then the function also returns
the furthest mapped value.
The value that was passed to the redundant `lookupOrDefault` was
produced by this code:
```
Value newOperand = mapping.lookupOrDefault(operand, desiredType);
```
There are 2 possible cases:
- Case 1: There is no mapping to `desiredType`. Then `newOperand` is the
furthest mapped value.
- Case 2: There is a mapping to `desiredType`. Then the type of
`newOperand` is `desiredType` and the "if" branch that encloses the
redundant `lookupOrDefault` is not executed at all.
Also improve the documentation of
`ConversionValueMapping::lookupOrDefault` and simplify the
implementation a bit.
PR #106760 aligned the handling of dropped block arguments and dropped
op results. The two helper functions that insert source materializations
for uses of replaced block arguments / op results that survived the
conversion are now almost identical (`legalizeConvertedArgumentTypes`
and `legalizeConvertedOpResultTypes`). This PR merges the two functions
and moves the implementation directly into `finalize`.
This PR simplifies the code base and improves the efficiency a bit:
previously, `finalize` iterated over
`ConversionPatternRewriterImpl::rewrites` twice. Now, only one iteration
is needed.
---------
Co-authored-by: Jakub Kuderski <jakub@nod-labs.com>
The dialect conversion maintains a set of unresolved materializations
(`UnrealizedConversionCastOp`). Turn that set into a `DenseMap` that
maps from ops to `UnresolvedMaterializationRewrite *`. This improves
efficiency a bit, because an iteration over
`ConversionPatternRewriterImpl::rewrites` can be avoided.
Also delete some dead code.
Handle dropped block arguments and dropped op results in the same way:
build a source materialization (that may fold away if unused). This
simplifies the code base a bit and makes it possible to merge
`legalizeConvertedArgumentTypes` and `legalizeConvertedOpResultTypes` in
a future commit. These two functions are almost doing the same thing
now.
As a side effect, this commit also changes the dialect conversion such
that temporary circular cast ops are no longer generated. (There was a
workaround in #107109 that can now be removed again.) Example:
```
%0 = "builtin.unrealized_conversion_cast"(%1) : (!a) -> !b
%1 = "builtin.unrealized_conversion_cast"(%0) : (!b) -> !a
// No further uses of %0, %1.
```
This happened when:
1. An op was erased. (No replacement values provided.)
2. A conversion pattern for another op builds a replacement value for
the erased op's results (first cast op) during `remapValues`, but that
SSA value is not used during the pattern application.
3. During the finalization phase, `legalizeConvertedOpResultTypes`
thinks that the erased op is alive because of the cast op that was built
in Step 2. It builds a cast from that replacement value to the original
type.
4. During the commit phase, all uses of the original op are replaced
with the casted value produced in Step 3. We have generated circular IR.
This problem can be avoided by making sure that source materializations
are generated for all dropped results. This ensures that we always have
some replacement SSA value in the mapping. Previously, we sometimes had
a value mapped and sometimes not. (No more special casing is needed
anymore to distinguish between "value dropped" or "value replaced with
SSA value".)
This commit makes source/target/argument materializations (via the
`TypeConverter` API) optional.
By default (`ConversionConfig::buildMaterializations = true`), the
dialect conversion infrastructure tries to legalize all unresolved
materializations right after the main transformation process has
succeeded. If at least one unresolved materialization fails to resolve,
the dialect conversion fails. (With an error message such as `failed to
legalize unresolved materialization ...`.) Automatic materializations
through the `TypeConverter` API can now be deactivated. In that case,
every unresolved materialization will show up as a
`builtin.unrealized_conversion_cast` op in the output IR.
There used to be a complex and error-prone analysis in the dialect
conversion that predicted the future uses of unresolved
materializations. Based on that logic, some casts (that were deemed to
unnecessary) were folded. This analysis was needed because folding
happened at a point of time when some IR changes (e.g., op replacements)
had not materialized yet.
This commit removes that analysis. Any folding of cast ops now happens
after all other IR changes have been materialized and the uses can
directly be queried from the IR. This simplifies the analysis
significantly. And certain helper data structures such as
`inverseMapping` are no longer needed for the analysis. The folding
itself is done by `reconcileUnrealizedCasts` (which also exists as a
standalone pass).
After casts have been folded, the remaining casts are materialized
through the `TypeConverter`, as usual. This last step can be deactivated
in the `ConversionConfig`.
`ConversionConfig::buildMaterializations = false` can be used to debug
error messages such as `failed to legalize unresolved materialization
...`. (It is also useful in case automatic materializations are not
needed.) The materializations that failed to resolve can then be seen as
`builtin.unrealized_conversion_cast` ops in the resulting IR. (This is
better than running with `-debug`, because `-debug` shows IR where some
IR changes have not been materialized yet.)
Note: This is a reupload of #104668, but with correct handling of cyclic
unrealized_conversion_casts that may be generated by the dialect
conversion.
There was a redundant `appendRewrite<ReplaceBlockArgRewrite>(block,
origArg);` in `ConversionPatternRewriterImpl::applySignatureConversion`
that had no effect.
This commit makes source/target/argument materializations (via the
`TypeConverter` API) optional.
By default (`ConversionConfig::buildMaterializations = true`), the
dialect conversion infrastructure tries to legalize all unresolved
materializations right after the main transformation process has
succeeded. If at least one unresolved materialization fails to resolve,
the dialect conversion fails. (With an error message such as `failed to
legalize unresolved materialization ...`.) Automatic materializations
through the `TypeConverter` API can now be deactivated. In that case,
every unresolved materialization will show up as a
`builtin.unrealized_conversion_cast` op in the output IR.
There used to be a complex and error-prone analysis in the dialect
conversion that predicted the future uses of unresolved
materializations. Based on that logic, some casts (that were deemed to
unnecessary) were folded. This analysis was needed because folding
happened at a point of time when some IR changes (e.g., op replacements)
had not materialized yet.
This commit removes that analysis. Any folding of cast ops now happens
after all other IR changes have been materialized and the uses can
directly be queried from the IR. This simplifies the analysis
significantly. And certain helper data structures such as
`inverseMapping` are no longer needed for the analysis. The folding
itself is done by `reconcileUnrealizedCasts` (which also exists as a
standalone pass).
After casts have been folded, the remaining casts are materialized
through the `TypeConverter`, as usual. This last step can be deactivated
in the `ConversionConfig`.
`ConversionConfig::buildMaterializations = false` can be used to debug
error messages such as `failed to legalize unresolved materialization
...`. (It is also useful in case automatic materializations are not
needed.) The materializations that failed to resolve can then be seen as
`builtin.unrealized_conversion_cast` ops in the resulting IR. (This is
better than running with `-debug`, because `-debug` shows IR where some
IR changes have not been materialized yet.)
Move the implementation of `ReconcileUnrealizedCasts` to
`DialectConversion.cpp`, so that it can be called from there in a future
commit.
This commit is in preparation of decoupling argument/source/target
materializations from the dialect conversion framework. The existing
logic around unresolved materializations that predicts IR changes to
decide if a cast op can be folded/erased will become obsolete, as
`ReconcileUnrealizedCasts` will perform these kind of foldings on fully
materialized IR.
---------
Co-authored-by: Markus Böck <markus.boeck02@gmail.com>
There was a typo in the code path that removes unnecessary
materializations.
Before: Update `opResult` (result of an op different from `user`) in
mapping and remove `user`.
```
replaceMaterialization(rewriterImpl, opResult, inputOperands,
inverseMapping);
necessaryMaterializations.remove(materializationOps.lookup(user));
```
After: Update `user->getResults()` in mapping and remove `user`.
```
replaceMaterialization(rewriterImpl, user->getResults(), inputOperands,
inverseMapping);
necessaryMaterializations.remove(materializationOps.lookup(user));
```
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.
---------
Co-authored-by: Jakub Kuderski <jakub@nod-labs.com>
The "inverse mapping" is an inverse IRMapping that points from replaced
values to their original values. This inverse mapping is needed when
legalizing unresolved materializations, to figure out if a value has any
uses. (It is not sufficient to examine the IR, because some IR changes
have not been materialized yet.)
There was a check in `OperationConverter::finalize` that computed the
inverse mapping only when needed. This check is not needed.
`legalizeUnresolvedMaterializations` always computes the inverse
mapping, so we can just do that in `OperationConverter::finalize` before
calling `legalizeUnresolvedMaterializations`.
Depends on #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.
This reverts commit 2aa96fcf751ee948702e8447de62d6bea8235e3a.
This was merged without a test. Also it seems it was only fixing an
issue for users which used a particular workaround that is not actually
needed anymore (skipping UnrealizedConversionCast operands).
This code got lost in #97213 and there was no test for it. Add it back
with an MLIR test.
When a pattern is run without a type converter, we can assume that the
new block argument types of a signature conversion are legal. That's
because they were specified by the user. This won't work for 1->N
conversions due to limitations in the dialect conversion infrastructure,
so the original `FIXME` has to stay in place.
This commit fixes a crash in the dialect conversion when applying a
signature conversion to a block inside of a detached region.
This fixes an issue reported in
4114d5be87 (r1691809730).
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:
- 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.
This commit fixes a bug in the dialect conversion. During a 1:N
signature conversion, the dialect conversion did not insert a cast back
to the original block argument type, producing invalid IR.
See `test-block-legalization.mlir`: Without this commit, the operand
type of the op changes because an `unrealized_conversion_cast` is
missing:
```
"test.consumer_of_complex"(%v) : (!llvm.struct<(f64, f64)>) -> ()
```
To implement this fix, it was necessary to change the meaning of
argument materializations. An argument materialization now maps from the
new block argument types to the original block argument type. (It now
behaves almost like a source materialization.) This also addresses a
`FIXME` in the code base:
```
// FIXME: The current argument materialization hook expects the original
// output type, even though it doesn't use that as the actual output type
// of the generated IR. The output type is just used as an indicator of
// the type of materialization to do. This behavior is really awkward in
// that it diverges from the behavior of the other hooks, and can be
// easily misunderstood. We should clean up the argument hooks to better
// represent the desired invariants we actually care about.
```
It is no longer necessary to distinguish between the "output type" and
the "original output type".
Most type converter are already written according to the new API. (Most
implementations use the same conversion functions as for source
materializations.) One exception is the MemRef-to-LLVM type converter,
which materialized an `!llvm.struct` based on the elements of a memref
descriptor. It still does that, but casts the `!llvm.struct` back to the
original memref type. The dialect conversion inserts a target
materialization (to `!llvm.struct`) which cancels out with the other
cast.
This commit also fixes a bug in `computeNecessaryMaterializations`. The
implementation did not account for the possibility that a value was
replaced multiple times. E.g., replace `a` by `b`, then `b` by `c`.
This commit also adds a transform dialect op to populate SCF-to-CF
patterns. This transform op was needed to write a test case. The bug
described here appears only during a complex interplay of 1:N signature
conversions and op replacements. (I was not able to trigger it with ops
and patterns from the `test` dialect without duplicating the `scf.if`
pattern.)
Note for LLVM integration: Make sure that all
`addArgument/Source/TargetMaterialization` functions produce an SSA of
the specified type.
Depends on #98743.
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 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 commit removes a `FIXME` in the code base that was in place because
of patterns that used the dialect conversion API incorrectly. Those
patterns have been fixed and the workaround is no longer needed.
Remove a TODO in the dialect conversion code base when materializing
unresolved conversions:
```
// FIXME: Determine a suitable insertion location when there are multiple
// inputs.
```
The implementation used to select an insertion point as follows:
- If the cast has exactly one operand: right after the definition of the
SSA value.
- Otherwise: right before the cast op.
However, it is not necessary to change the insertion point. Unresolved
materializations (`UnrealizedConversionCastOp`) are built during
`buildUnresolvedArgumentMaterialization` or
`buildUnresolvedTargetMaterialization`. In the former case, the op is
inserted at the beginning of the block. In the latter case, only one
operand is supported in the dialect conversion, and the op is inserted
right after the definition of the SSA value. I.e., the
`UnrealizedConversionCastOp` is already inserted at the right place and
it is not necessary to change the insertion point for the resolved
materialization op.
Note: The IR change changes slightly because the
`unrealized_conversion_cast` ops at the beginning of a block are no
longer doubly-inverted (by setting the insertion to the beginning of the
block when inserting the `unrealized_conversion_cast` and again when
inserting the resolved conversion op). All affected test cases were
fixed by using `CHECK-DAG` instead of `CHECK`.
Also improve the quality of multiple test cases that did not check for
the correct operands.
Note: This commit is in preparation of decoupling the
argument/source/target materialization logic of the type converter from
the dialect conversion (to reduce its complexity and make that
functionality usable from a new dialect conversion driver).
This commit simplifies and improves documentation for the part of the
`ConversionPatternRewriter` API that deals with signature conversions.
There are now two public functions for signature conversion:
* `applySignatureConversion` converts a single block signature. This
function used to take a `Region *` (but converted only the entry block).
It now takes a `Block *`.
* `convertRegionTypes` converts all block signatures of a region.
`convertNonEntryRegionTypes` is removed because it is not widely used
and can easily be expressed with a call to `applySignatureConversion`
inside a loop. (See `Detensorize.cpp` for an example.)
Note: For consistency, `convertRegionTypes` could be renamed to
`applySignatureConversion` (overload) in the future. (Or
`applySignatureConversion` renamed to `convertBlockTypes`.)
Also clarify when a type converter and/or signature conversion object is
needed and for what purpose.
Internal code refactoring (NFC) of `ConversionPatternRewriterImpl` (the
part that deals with signature conversions). This part of the codebase
was quite convoluted and unintuitive.
From a functional perspective, this change is NFC. However, the public
API changes, thus not marking as NFC.
Note for LLVM integration: When you see
`applySignatureConversion(region, ...)`, replace with
`applySignatureConversion(region->front(), ...)`. In the unlikely case
that you see `convertNonEntryRegionTypes`, apply the same changes as
this commit did to `Detensorize.cpp`.
---------
Co-authored-by: Markus Böck <markus.boeck02@gmail.com>
This commit changes `OpBuilder::tryFold` to behave more similarly to
`Operation::fold`. Concretely, this ensures that even an in-place fold
returns `success`.
This is necessary to fix a bug in the dialect conversion that occurred
when an in-place folding made an operation legal. The dialect conversion
infrastructure did not check if the result of an in-place folding
legalized the operation and just went ahead and tried to apply pattern
anyways.
The added test contains a simplified version of a breakage we observed
downstream.
This reverts commit 49a4ec20a8be5888cbf225bab340dbaf204902c7.
Reason: Broke the ASan build bot with a memory leak. See the comments at
https://github.com/llvm/llvm-project/pull/87297
for more information.
