| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
With LDBG(), the code isn't guarded in release mode, even if the optimizer
will remove it because there is a `if (false)` statement. We need the
function declaration to be there at minima.
|
|
|
|
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 patch fixes:
mlir/lib/Transforms/Utils/DialectConversion.cpp:1686:14: error:
unused variable 'newParentOp' [-Werror,-Wunused-variable]
|
|
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.
|
|
modification (#150748)
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.
|
|
Operation streaming (#150636)
|
|
Change local variants to use new central one.
|
|
(#150555)
The Canonicalizer pass has a dependency to UB dialect which shouldn't have.
It also no longer needs to directly depend on the UB dialect since the Vector dialect
(which uses UB dialect for poison index operations introduced by 35df525) already
declares this dependency(878d3594).
|
|
This patch fixes a bug in the RemoveDeadValues pass where unused
function arguments were not removed from the function signature in an
edge case where the function returns void.
A corresponding test was added to the MLIR LIT test suite to cover this
case.
|
|
These are identified by misc-include-cleaner. I've filtered out those
that break builds. Also, I'm staying away from llvm-config.h,
config.h, and Compiler.h, which likely cause platform- or
compiler-specific build failures.
|
|
building this file would fail when
MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS = 1
Signed-off-by: dan <danimal197@gmail.com>
|
|
(#149809)
|
|
See https://github.com/llvm/llvm-project/pull/147168 for more info.
---------
Co-authored-by: Tobias Gysi <tobias.gysi@nextsilicon.com>
|
|
separately (#148415)
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.
|
|
errors (#148416)
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.
|
|
foldings (#148394)
When an operation is folded to an attribute, the attribute must be
materialized as a constant operation. That operation must then be
legalized. If such a legalization fails, the entire folding is rolled
back. This is not supported in a One-Shot Dialect Conversion. (Support
for rolling back foldings could be added at a later point of time.)
This commit improves the `allowPatternRollback` flag handling, such that
a fatal error is reported when a folder is attempted to be rolled back.
|
|
When legalizing an operation, the conversion driver skips "ignored" ops.
Ops are ignored if they are inside of a recursively legal operation or
if they were erased.
This commit moves the "is ignored" check a bit earlier: it is now
checked before checking if the op is recursively legal. This is in
preparation of the One-Shot Dialect Conversion refactoring: erased ops
should not be accessed, not even for checking recursive legality.
This commit is NFC: When an op is erased, it is added to the set of
ignored ops and we don't want to process it, regardless of legality.
Nested ops are also added to the set of ignored ops when erasing an
enclosing op.
|
|
|
|
Add a new `ApplyConversionAction` so that users can profile the time
that is spent in the conversion driver.
|
|
These are identified by misc-include-cleaner. I've filtered out those
that break builds. Also, I'm staying away from llvm-config.h,
config.h, and Compiler.h, which likely cause platform- or
compiler-specific build failures.
|
|
(#145319)
This commit adds extra state to `ConversionPatternRewriterImpl` to store
all modified / newly-created operations and moved / newly-created blocks
in separate lists on a per-pattern basis.
This is in preparation of the One-Shot Dialect Conversion refactoring:
the new driver will no longer maintain a list of all IR rewrites, so
information about newly-created operations (which is needed to trigger
recursive legalization) must be retained in a different data structure.
This commit is also expected to improve the performance of the existing
driver. The previous implementation iterated over all new IR
modifications and then filtered them by type. It also required an
additional pointer indirection (through `std::unique_ptr<IRRewrite>`) to
retrieve the operation/block pointer.
|
|
The previous code was effectively that a symbol is dead if was not
nested in sequence of SymbolTables. But one can have operations that one
cannot delete/DCE that refers to symbols which one could delete which
resulted in symbol-dce deleting symbols that are still referenced and
the resulting IR being invalid. This changes it so that all operations
inside non SymbolTable op are considered to find nested SymbolTable ops.
---------
Co-authored-by: Mehdi Amini <joker.eph@gmail.com>
|
|
When a block is getting inlined, the destination block does not have to
be legalized. That's because the signature of the destination block does
not change by inlining.
This commit makes the implementation consistent with this comment:
```
// If the pattern moved or created any blocks, make sure the types of block
// arguments get legalized.
```
|
|
This commit adds 1:N support to
`ConversionPatternRewriter::replaceUsesOfBlockArgument`. This was one of
the few remaining dialect conversion APIs that does not support 1:N
conversions yet.
This commit also reuses `replaceUsesOfBlockArgument` in the
implementation of `applySignatureConversion`. This is in preparation of
the One-Shot Dialect Conversion refactoring. The goal is to bring the
`applySignatureConversion` implementation into a state where it works
both with and without rollbacks. To that end, `applySignatureConversion`
should not directly access the `mapping`.
|
|
(#145155)
Since #145030, `ConversionPatternRewriter::eraseBlock` no longer calls
`ConversionPatternRewriter::eraseOp`. This now happens in the rewriter
impl (during the cleanup phase). Therefore, a safety check in
`replaceOp` can now be simplified.
|
|
Simple IR patterns will trigger assertion error:
```
func.func @test_zero_operands(%I: memref<10xindex>, %I2: memref<10xf32>) {
%v0 = arith.constant 0 : index
%result = memref.alloca_scope -> index {
%c = arith.addi %v0, %v0 : index
memref.store %c, %I[%v0] : memref<10xindex>
memref.alloca_scope.return %c: index
}
func.return
}
```
with error: `mlir/include/mlir/IR/Operation.h:988:
mlir::detail::OperandStorage& mlir::Operation::getOperandStorage():
Assertion `hasOperandStorage && "expected operation to have operand
storage"' failed.`
This PR will fix this issue.
---------
Co-authored-by: Andrzej Warzyński <andrzej.warzynski@gmail.com>
Co-authored-by: Mehdi Amini <joker.eph@gmail.com>
|
|
(#144695)
Debugging issues with this pass is quite difficult at the moment, this
should help.
|
|
(#145030)
Add missing listener notifications when erasing nested
blocks/operations.
This commit also moves some of the functionality from
`ConversionPatternRewriter` to `ConversionPatternRewriterImpl`. This is
in preparation of the One-Shot Dialect Conversion refactoring: The
implementations in `ConversionPatternRewriter` should be as simple as
possible, so that a switch between "rollback allowed" and "rollback not
allowed" can be inserted at that level. (In the latter case,
`ConversionPatternRewriterImpl` can be bypassed to some degree, and
`PatternRewriter::eraseBlock` etc. can be used.)
Depends on #145018.
|
|
Rename a few internal functions: drop the `notify` prefix, which
incorrectly suggests that the function is a listener callback function.
|
|
Before this PR, users had to pass the "old" block argument when
replacing the uses of a block argument in a newly converted block. Users
can now pass the actual block argument that should be replaced.
Note for LLVM integration: Make sure to pass the current block argument
instead of the old one.
|
|
`unresolvedMaterializations` up to date (#144254)
`unresolvedMaterializations` is a mapping from
`UnrealizedConversionCastOp` to `UnresolvedMaterializationRewrite`. This
mapping is needed to find the correct type converter for an unresolved
materialization.
With this commit, `unresolvedMaterializations` is updated immediately
when an op is being erased. This also cleans up the code base a bit:
`SingleEraseRewriter` is now used only during the "cleanup" phase and no
longer needed as a field of `ConversionRewriterImpl`.
This commit is in preparation of the One-Shot Dialect Conversion
refactoring: `allowPatternRollback = false` will in the future trigger
immediate materialization of all IR changes.
|
|
This patch simplifies code by removing the values from
insert/try_emplace. Note that default values inserted by try_emplace
are immediately overrideen in all these cases.
|
|
- try_emplace(Key) is shorter than insert({Key, nullptr}).
- try_emplace performs value initialization without value parameters.
- We overwrite values on successful insertion anyway.
|
|
Before:
```
** Erase : 'scf.yield'(0x6000037b1630)
** Insert Block into detached Region (nullptr parent op)' ** Insert Block into detached Region (nullptr parent op)' ** Insert : 'scf.if'(0x6000025b8140)
** Erase : 'scf.if'(0x600003ab0780)
```
After:
```
** Erase : 'scf.yield'(0x600003128b90)
** Insert Block into detached Region (nullptr parent op)'
** Insert Block into detached Region (nullptr parent op)'
** Insert : 'scf.if'(0x6000023206e0)
```
|
|
|
|
This patch fixes warnings of the form:
mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp:320:19: error:
unused variable 'result' [-Werror,-Wunused-variable]
|
|
(#140961)
The current implementation of getBackwardSlice will crash if an
operation in the dependency chain is defined by an operation with
multiple regions or blocks. Crashing is bad (and forbids many analyses
from using getBackwardSlice, as well as causing existing users of
getBackwardSlice to fail for IR with this property).
This PR instead causes the analysis to return a failure, rather than
crash in the cases it cannot compute the full slice
---------
Co-authored-by: Oleksandr "Alex" Zinenko <git@ozinenko.com>
|
|
|
|
(#140347)
If a type conversion rule fails to apply, it should not append any types
to the result. This commit just adds an assertion to detect such cases
of incorrect API usage.
|
|
|
|
When a `CompositePass` is created programmatically, it incorrectly
prepends an extra `any` to the inner pipeline string. For example:
```c++
passManager.nestAny().addPass(createCompositeFixedPointPass(
"Pass1AndPass2",
[](OpPassManager &nestedPassManger) {
nestedPassManger.addPass(createPass1());
nestedPassManger.addPass(createPass2());
},
```
This would result in the following pipeline string:
```
any(composite-fixed-point-pass{max-iterations=3 name=Pass1AndPass2
pipeline=any(pass1,pass2)})
```
This commit fixes this issue, resulting in the pipeline string:
```
any(composite-fixed-point-pass{max-iterations=3 name=Pass1AndPass2
pipeline=pass1,pass2})
```
|
|
|
|
There are two kind of materialization callbacks: one for target
materializations and one for source materializations. The callback type
for target materializations is `TargetMaterializationCallbackFn`. This
commit renames the one for source materializations from
`MaterializationCallbackFn` to `SourceMaterializationCallbackFn`, for
consistency.
There used to be a single callback type for both kind of
materializations, but the materialization function signatures have
changed over time.
Also clean up a few places in the documentation that still referred to
argument materializations.
|
|
|
|
`FunctionOpInterface` assumed the fact that the function type (attribute
of the operation) can be cloned with arbirary lists of function
arguments and results to support argument and result list mutation. This
is not always correct, in particular, LLVM dialect functions require
exactly one result making it impossible to erase the result.
Allow function type cloning to fail and propagate this failure through
various APIs that use it. The common assumption is that existing IR has
not been modified.
Fixes #131142.
Reland a8c7ecdcbc3e89b493b495c6831cc93671c3b844 / #136300.
|
|
|
