Introduce a lightweight, targeted dead-op eliminator that complements
the
existing liveness-based runRegionDCE. The algorithm is O(ops):
step 1 walks each op in reverse program order, erases it outright if
trivially dead, or recurses into nested regions when recursive cleanup
is
enabled; step 2 drives a per-region worklist that only ever holds ops
already verified dead, propagating new deadness by dropping each
operand's
use before re-checking isOpTriviallyDead on the defining op.
Unlike runRegionDCE, this does not touch dead block arguments, dead
successor operands, or dead use-def cycles. Use runRegionDCE when those
are required. An includeNestedRegions parameter lets callers restrict
simplification to the top-level region.
Switch transform.apply_dce to use the new helper, dropping a custom
worklist that had subtle invariants: SetVector dedup, linear search on
every erase, and pre-order walk with skip.
In general it is bad practice to implement complex custom logic in
Transform ops apply methods: this should be an adaptor exposing MLIR
transformations to the transform dialect; non-trivial logic deserves a
public API for reusability and proper layering.
Introduce a TrivialDeadCodeEliminationPass exposed as -trivial-dce. The
pass
removes trivially dead operations and, when removeBlocks is enabled,
unreachable blocks. It does not run liveness analysis and does not
remove
dead use-def cycles. Add recursive and removeBlocks options, both
defaulting
to true, to control whether nested regions are visited and whether
unreachable blocks are erased.
Assisted-by: Claude Code
Add a new `filter-dialects` list option to the canonicalize pass. When
provided, only canonicalization patterns from the listed dialects are
collected, and the named dialects are force-loaded via
getDependentDialects.
Loading flow: the Canonicalizer's getDependentDialects override calls
`registry.addDialectToPreload(name)` for each filter-dialect name, which
records the name in a new `dialectsToPreload` list on DialectRegistry.
The PassManager's pipeline-init then calls
`dependentDialects.preloadSelectDialects(ctx, emitError)`, which loads
each preload entry via `context->getOrLoadDialect(name)` — the real
allocator is resolved from the context's own registry (registered by
the tool) and the dialect is loaded before multi-threaded execution
begins. If a requested dialect has no registration in the context, a
diagnostic `"can't load dialect '<name>': missing registration?"` is
emitted.
DialectRegistry API changes:
- New `addDialectToPreload(StringRef)` method: records a dialect name
that should be loaded into the MLIRContext but whose allocator lives
in the context's own registry. The registry itself does not load the
dialect — it just carries the request.
- New `preloadSelectDialects(MLIRContext *,
function_ref<InFlightDiagnostic()> = {})`
method: loads every preload-registered dialect into the context,
returning failure (and optionally emitting a diagnostic) for the
first name that cannot be resolved.
- `getDialectNames()` is split into two accessors:
* `getRegisteredDialectNames()` — names of allocator-backed entries
from the registry map.
* `getDialectsToPreload()` — preload-only entries added via
`addDialectToPreload(StringRef)`.
Assisted-by: Claude Code
When we promote an allocation, and a default value for a load from an
uninitialized slot is required, this value used to get inserted in the
same block as the allocation. However, in some cases, the default value
needs to be available in the predecessor blocks so that they can pass it
to the block of the allocation as an argument. For example, this is the
case for loops containing an allocation where the promoted value will
become and IV.
Make sure the default value is always available to all blocks by
creating it in the entry block of the region.
For reference, this is what used to be the output for the test. Note the
use of `%1`.
```
"func.func"() <{function_type = (f64) -> (), sym_name = "poison_insertion_point"}> ({
^bb0(%arg0: f64):
"cf.br"(%1)[^bb1] : (f64) -> ()
^bb1(%0: f64): // 2 preds: ^bb0, ^bb2
%1 = "ub.poison"() <{value = #ub.poison}> : () -> f64
%2 = "test.get"() : () -> i1
"cf.cond_br"(%2)[^bb2, ^bb3] <{operandSegmentSizes = array<i32: 1, 0, 0>}> : (i1) -> ()
^bb2: // pred: ^bb1
%3 = "test.get"() : () -> i1
%4 = "scf.if"(%3) ({
"scf.yield"(%arg0) : (f64) -> ()
}, {
"scf.yield"(%0) : (f64) -> ()
}) : (i1) -> f64
"test.use"(%4) : (f64) -> ()
"cf.br"(%4)[^bb1] : (f64) -> ()
^bb3: // pred: ^bb1
"func.return"() : () -> ()
}) : () -> ()
```
The greedy pattern rewrite driver previously only deduplicated constant
ops between iterations (via the operation folder). Structurally
identical non-constant subexpressions remained distinct SSA values,
blocking fold patterns that only fire when operands match. Reaching the
true fixpoint required chaining an external `cse,canonicalize,...`
pipeline.
Add an opt-in `cseBetweenIterations` flag on `GreedyRewriteConfig` that
runs full CSE on the scoped region after each pattern-application
iteration, and surface it as a `cse-between-iterations` option on the
canonicalizer pass. Off by default to preserve existing performance
characteristics.
Assisted-by: Claude Code
This PR avoids calling `simplifyRegion` on dead region ops.
`simplifyRegion` attempts to perform CSE optimization on the ops within
the region, which is unnecessary for ops that are already trivially
dead.
The condition on detecting cache insertion was reversed. The consequence
was that we would always go through the path of "cache hit" first, but
find that the entry was the just-inserted one and then proceed with
updating it. Subsequent attempt could go through the "cache miss" part
and the cache would never be used.
The CSE pass calls `markAnalysesPreserved<DominanceInfo,
PostDominanceInfo>()` at the end. While CSE erases operations, it does
not remove their corresponding dominator trees, causing them to be
unnecessarily preserved in memory. This PR addresses the issue by
explicitly calling invalidate within CSE to clean up the dominator trees
for those erased operations.
This PR addresses an issue where dead code within a region prevents
essentially identical region ops from being CSE'd. Currently, CSE only
removes dead ops at the end of the pass. This PR fixes the issue by
deleting dead code immediately upon encounter.
The original comment claimed that DominanceInfo and PostDominanceInfo
could be preserved because region operations are not removed. However,
the real reason was that the original CSE only deleted redundant
operations without moving any operation to a different block, leaving
the dominance tree structure unchanged. Part of
https://github.com/llvm/llvm-project/pull/180556.
This PR fixes a regression where the numCSE statistic was being
incremented twice for a single operation elimination. The numCSE counter
is already internally incremented within the replaceUsesAndDelete
function. Manually incrementing it again after the function call leads
to an inaccurate total count. This is part of the
https://github.com/llvm/llvm-project/pull/180556.
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>
Fix two issues in `MatchingSubsets::populateSubsetOpsAtIterArg`:
1. The `collectHoistableOps` parameter was declared but never used when
inserting subset ops via `insert(subsetOp)`. As a result, when recursing
into nested loops with `collectHoistableOps=false`, the nested loop's
subset ops were incorrectly added to the hoistable extraction/insertion
pairs of the parent loop. This caused spurious failures in the
`allDisjoint` check, preventing valid hoisting when nested loop ops
overlapped with outer loop ops. Fix by passing the parameter:
`insert(subsetOp, collectHoistableOps)`.
2. In the nested loop handling branch, there was no guard to detect when
a value has multiple nested loop uses (i.e., is used as an init arg in
more than one nested loop). Without the guard, `nextValue` would be
silently overwritten, leading to an incorrect use-def chain traversal.
Add `if (nextValue) return failure()` before setting `nextValue` for the
nested loop case, mirroring the existing guard for insertion ops.
Fixes#147096
Assisted-by: Claude Code
The RemoveDeadValuesPass previously emitted an error and skipped
optimization when the IR contained non-function symbol ops, non-call
symbol user ops, or branch ops. This restriction was later removed, but
the comments in RemoveDeadValues.cpp and Passes.td still described the
pass as operating "iff the IR doesn't have any non-function symbol ops,
non-call symbol user ops and branch ops."
Remove the stale restriction text from both the .cpp file comment and
the Passes.td description. Also add a test that verifies dead function
arguments are correctly removed inside a module that defines a symbol
(has a sym_name attribute), which was the original failure case reported
in issue #98700.
Fixes#98700
Assisted-by: Claude Code
WalkPatternRewriteDriver's ErasedOpsListener incorrectly flagged
erasures of ops/blocks that were created during the current pattern
application. Since those ops were never in the walk schedule, erasing
them is safe.
Track newly inserted ops and blocks per visited op; skip the erasure
check for them. Also fix two related issues:
- DropUnitDims: use a fresh IRRewriter (not inheriting the walk pattern
rewriter's listener) for replaceUnitDimIndexOps on cloned ops.
- TestPatterns CloneRegionBeforeOp: wrap op->setAttr() in
modifyOpInPlace to properly notify the rewriter of the in-place change.
Assisted-by: Claude Code
Fix a failure present with MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.
Co-authored-by: Claude Sonnet 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.
This reverts commit e5adddc5be.
This commit broke a lot of Fortran compilations in our tests and builds.
I am working on an additional fix, but I would like to revert this
in the meantime. I will reupload it with the fix.
We have identified a problem with DominanceInfo caching in Mem2Reg. It
appears to also be subject to incorrect cache hits when regions are
deleted, causing sporadic bugs that are difficult to test for.
This quick fix invalidates region that could be invalidated. This
attempts to not be too eager by only invalidating regions that are
exposed to a `finalizePromotion` call.
Ultimately it would be nice to have the ability to move the cached
information from one region to the next, but this is currently not
supported by DominanceInfo.
I was not able to produce a test for this as it is very sporadic. We
would need to be testing for a case where a region is re-allocated at
the same address as a previously erased region. If you know how to make
this sort of behavior consistent, I would be interested. Otherwise this
might require no testing.
There was a problem with the way Mem2Reg was removing unused block
arguments, as it was incorrectly assuming the reaching definition was
still available when connecting the successor operands but it may have
been removed as part of the mem2reg process. This approach instead
places successor operands eagerly, and removes them along with the block
argument if unused (similarly to how it was done before the region
support).
This also fixes what I think was a long-standing issue where a block
argument only used by operations that will be deleted would not be
considered unused.
Fixes#188252
We need to add the regions with the direct uses into the list
for processing, otherwise the direct uses will not be removed
and will use the slot after the promotion.
The added LIT test was triggering "after promotion, the slot pointer
should not be used anymore" assertion.
`transformCFGToSCF` would crash with a use-list assertion when it
encountered an op like `spirv.BranchConditional` that implements
`BranchOpInterface` (passing the existing precondition checks) but is
not handled by `createStructuredBranchRegionOp`. The algorithm mutated
the IR significantly before discovering the op was unsupported, leaving
it in a corrupt half-transformed state that triggered the assertion on
teardown.
Fix by adding `canConvertBranchOp` to `CFGToSCFInterface` (default:
accept all ops) and calling it inside `checkTransformationPreconditions`
for every block with more than one successor, before any IR
modifications are made. `ControlFlowToSCFTransformation` overrides the
method to accept only `cf.cond_br` and `cf.switch`.
Fixes#173566
Assisted-by: Claude Code
This PR adds support for region control-flow. Region control-flow and
CFG can be mixed together in the same program. See the [accompanying
RFC](https://discourse.llvm.org/t/rfc-support-region-control-flow-in-mem2reg/90082)
for some design considerations.
Beyond the considerations in the RFC, a few minor changes were
introduced:
- Calling the visitor hook for defined values is now deferred to the end
of promotion.
- The lazy creation of default values has been moved to the places where
it happens to prepare for a future change where it is actually lazy.
Documentation about it not working as intended for now was also added.
All SCF operations are supported, including `forall` and `parallel`,
which is pretty cool I think.
I am sorry in advance for git diff displaying a really bad diff for
Mem2Reg.cpp around where the liveness analysis used to be. Do consider
simply reading this part of the code off the file.
As a disclaimer, I designed all the test cases myself, but I used a
large amount of matrix multiplications to produce the corresponding IR
and FileCheck tests. I have reviewed them carefully and they correspond
to my intent.
---------
Co-authored-by: Slava Zakharin <szakharin@nvidia.com>
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 patch allows creating a hierarchy of `SideEffects::Resource`s by adding
a virtual `getParent()` method, so that effects on *disjoint* resources
can be proven non-conflicting. It also adds virtual `isAddressable()` method
that represents a property of a resource to be addressable via a pointer
value. The non-addressable resources may not be affected via any pointer.
This is unblocking CSE, LICM and alias analysis without per-pass
special-casing.
RFC:
https://discourse.llvm.org/t/rfc-mlir-memory-region-hierarchy-for-mlir-side-effects/89811
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.
When transformToStructuredCFBranches encountered a control flow op not
handled by the CFGToSCFInterface (e.g., spirv.BranchConditional with
--lift-cf-to-scf), it correctly emitted an error and returned failure.
However, blocks had already been moved from the parent region into
temporary local Region objects before the failure was detected.
When those temporary Region objects went out of scope, their destructor
tried to destroy the contained blocks. But those blocks still had live
predecessor references from the parent region (the regionEntry's
terminator still pointed to them), causing an assertion failure:
use_empty() && "Cannot destroy a value that still has uses\!"
Fix: on failure from createStructuredBranchRegionOp, move the blocks
from the temporary conditionalRegions back into the parent region before
returning failure. This restores IR consistency and allows the Region
destructor to run safely.
Fixes#120883Fixes#118454
Assisted-by: Claude Code
`processFuncOp` asserts that all symbol uses of a function are
`CallOpInterface` operations. This is violated when a function is
referenced by a non-call operation such as `spirv.EntryPoint`, which
uses the function symbol for metadata purposes without calling it.
Fix this by replacing the assertion with an early return: if any user of
the function symbol is not a `CallOpInterface`, skip the function
entirely. This is safe because the pass cannot determine the semantics
of arbitrary non-call references, so it should leave such functions
alone.
Fixes#180416
Before erasing the operation, replace all result values with live-uses
by
ub.poison values. This is important to maintain IR validity. For
example,
if we have an op with one of its results used by another op, erasing the
op without replacing its corresponding result would leave us with a
dangling operand in the user op. By replacing the result with a
ub.poison
value, we ensure that the user op still has a valid operand, even though
it's a poison value which will be cleaned up later if it can be cleaned
up. This keeps the IR valid for further simplification and
canonicalization while fixing a related crash in the canonicalizer.
Fixes https://github.com/llvm/llvm-project/issues/179944
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 patch adds a side-effect check to `moveOperationDependencies` to
match the behavior of `moveValueDefinitions`. Previously,
`moveOperationDependencies` would move operations with side-effecting
dependencies, which could change program semantics.
**Note** that the existing test changes are needed because unregistered
operations (e.g., "moved_op"()) are treated as side-effecting. These
tests were updated to use pure operations for operations in the moved
slice, while keeping unregistered ops for operations that aren't moved
(e.g., "before"(), "foo"()). This ensures that tests continue to
exercise their intended functionality without being blocked by the new
side-effect check.
This commit simplifies the `remove-dead-values` pass and fixes a bug in
the handling of `RegionBranchOpInterface` ops. The pass used to produce
invalid IR ("null value found") for the newly added test case.
`remove-dead-values` is a pass for additional IR simplification that
cannot be performed by the canonicalizer pass. Based on a liveness
analysis, it erases dead values / IR. (The liveness analysis is a
dataflow analysis that has more information about the IR than a
canonicalization pattern, which can see only "local" information.)
Region-based ops are difficult. The liveness analysis may determine that
an SSA value is dead. However, that does not mean that the value can
actually be removed. Doing so may violate an region data flow (as
modeled by the `RegionBranchOpInterface`). As an example, consider the
case where a region branch terminator may dispatch to one of two region
successor with the same forwarded values. A successor input (block
argument) can be erased only if it is dead on both successors.
Before this commit, there used to be complex logic to determine when it
is safe to erase an SSA value. That logic was broken. The new
implementation does not remove any block arguments or op results of
region-based ops. Instead, operands of region-based ops and region
branch terminators are replaced with `ub.poison` if all of their
successor values are dead. This simplifies the IR good enough for the
canonicalizer to perform the remaining region simplification (i.e.,
dropping block arguments etc.).
RFC:
https://discourse.llvm.org/t/rfc-delegate-simplification-of-region-based-ops-from-remove-dead-values-to-canonicalizer/89194
dropRedundantArguments was incorrectly indexing into forwardedOperands
using the block argument index directly. This crashes when the block has
produced operands (generated by the terminator, not forwarded from
predecessors) because forwardedOperands doesn't include them.
The fix checks isOperandProduced() to skip produced arguments and uses
SuccessorOperands::operator[] which handles the offset correctly.
`remove-dead-values` performs various cleanups:
1. Erasing block arguments
2. Erasing successor operands
3. Erasing operations
4. Erasing function arguments / results
5. Erasing operands
6. Erasing results
This commit moves Step 3 (erasing operations) to the end. While that
does not fix any bugs by itself, it is potentially safer. If an
operation is erased, we must be careful that the operation is not
accessed in the following steps. That can no longer happen if IR is
erased only in the final step and not before.
This commit is prefetching a change from #173505 (to keep that PR
shorter). With #173505, it will become necessary to erase IR in the
final step.
There are various places in the code base where op results are removed.
E.g., some canonicalization patterns remove op results. This commit adds
a new helper function to `RewriterBase` to reduce code duplication and
simplify patterns. The existing implementation from
`RemoveDeadValues.cpp` is moved into the rewriter API.
There is now a uniform API for removing operands and values:
* `Block::eraseArguments(BitVector)`
* `Operation::eraseOperands(BitVector)`
* NEW: `RewriterBase::eraseOpResults(Operation *, BitVector)`
This commit is preparation of adding new canonicalizations for
region-based ops, which will add yet another place where op results must
be erased.
Add a helper for querying the successor operands for a region branch
`src -> dst`. Both `src` and `dst` may be the region branch op itself or
a terminator.
This helper allows users to query successor operands for the region
branch op and the terminators in a uniform way. This is similar to
`getSuccessorRegions(RegionBranchPoint)`, which works both for region
branch ops and terminators.
