[mlir] Make remove-dead-values pass remove blocks arguments first #165725
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-core Author: lonely eagle (linuxlonelyeagle) ChangesFix #163051. Full diff: https://github.com/llvm/llvm-project/pull/165725.diff 2 Files Affected:
diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index e0c65b0e09774..dc3157bd7b5b2 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -731,7 +731,25 @@ static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la,
static void cleanUpDeadVals(RDVFinalCleanupList &list) {
LDBG() << "Starting cleanup of dead values...";
- // 1. Operations
+ // 1. Blocks
+ LDBG() << "Cleaning up " << list.blocks.size() << " block argument lists";
+ for (auto &b : list.blocks) {
+ // blocks that are accessed via multiple codepaths processed once
+ if (b.b->getNumArguments() != b.nonLiveArgs.size())
+ continue;
+ LDBG() << "Erasing " << b.nonLiveArgs.count()
+ << " non-live arguments from block: " << b.b;
+ // it iterates backwards because erase invalidates all successor indexes
+ for (int i = b.nonLiveArgs.size() - 1; i >= 0; --i) {
+ if (!b.nonLiveArgs[i])
+ continue;
+ LDBG() << " Erasing block argument " << i << ": " << b.b->getArgument(i);
+ b.b->getArgument(i).dropAllUses();
+ b.b->eraseArgument(i);
+ }
+ }
+
+ // 2. Operations
LDBG() << "Cleaning up " << list.operations.size() << " operations";
for (auto &op : list.operations) {
LDBG() << "Erasing operation: "
@@ -740,14 +758,14 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
op->erase();
}
- // 2. Values
+ // 3. Values
LDBG() << "Cleaning up " << list.values.size() << " values";
for (auto &v : list.values) {
LDBG() << "Dropping all uses of value: " << v;
v.dropAllUses();
}
- // 3. Functions
+ // 4. Functions
LDBG() << "Cleaning up " << list.functions.size() << " functions";
// Record which function arguments were erased so we can shrink call-site
// argument segments for CallOpInterface operations (e.g. ops using
@@ -769,7 +787,7 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
(void)f.funcOp.eraseResults(f.nonLiveRets);
}
- // 4. Operands
+ // 5. Operands
LDBG() << "Cleaning up " << list.operands.size() << " operand lists";
for (OperationToCleanup &o : list.operands) {
// Handle call-specific cleanup only when we have a cached callee reference.
@@ -811,7 +829,7 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
}
}
- // 5. Results
+ // 6. Results
LDBG() << "Cleaning up " << list.results.size() << " result lists";
for (auto &r : list.results) {
LDBG() << "Erasing " << r.nonLive.count()
@@ -820,24 +838,6 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
dropUsesAndEraseResults(r.op, r.nonLive);
}
- // 6. Blocks
- LDBG() << "Cleaning up " << list.blocks.size() << " block argument lists";
- for (auto &b : list.blocks) {
- // blocks that are accessed via multiple codepaths processed once
- if (b.b->getNumArguments() != b.nonLiveArgs.size())
- continue;
- LDBG() << "Erasing " << b.nonLiveArgs.count()
- << " non-live arguments from block: " << b.b;
- // it iterates backwards because erase invalidates all successor indexes
- for (int i = b.nonLiveArgs.size() - 1; i >= 0; --i) {
- if (!b.nonLiveArgs[i])
- continue;
- LDBG() << " Erasing block argument " << i << ": " << b.b->getArgument(i);
- b.b->getArgument(i).dropAllUses();
- b.b->eraseArgument(i);
- }
- }
-
// 7. Successor Operands
LDBG() << "Cleaning up " << list.successorOperands.size()
<< " successor operand lists";
diff --git a/mlir/test/Transforms/remove-dead-values.mlir b/mlir/test/Transforms/remove-dead-values.mlir
index e7304505c809e..8f56c656aa859 100644
--- a/mlir/test/Transforms/remove-dead-values.mlir
+++ b/mlir/test/Transforms/remove-dead-values.mlir
@@ -674,3 +674,18 @@ func.func @dead_value_loop_ivs_no_result(%lb: index, %ub: index, %step: index, %
}
return
}
+
+// -----
+
+// CHECK-LABEL: func @op_block_have_dead_arg
+func.func @op_block_have_dead_arg(%arg0: i64, %arg1: i64, %arg2: i64, %arg3: i1) {
+ omp.wsloop {
+ omp.loop_nest (%arg4) : i64 = (%arg0) to (%arg1) step (%arg2) {
+ cf.cond_br %arg3, ^bb1(%arg0 : i64), ^bb1(%arg1 : i64)
+ ^bb1(%0: i64):
+ omp.yield
+ }
+ }
+// CHECK-NEXT: return
+ return
+}
|
Member
Author
|
ping for review @joker-eph, thank you. |
joker-eph
reviewed
Nov 5, 2025
| ^bb1(%0: i64): | ||
| omp.yield | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
Could we have a test using the test dialect? Or scf or something that is less likely to change than omp
joker-eph
approved these changes
Nov 5, 2025
Collaborator
joker-eph
left a comment
joker-eph
left a comment
There was a problem hiding this comment.
LG, but I'd rather avoid using omp dialect in these tests.
linuxlonelyeagle
force-pushed
the
make-remove-dead-values-remove-block-first
branch
from
0782241 to
dd25925
Compare
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/55/builds/19677 Here is the relevant piece of the build log for the reference |
Member
Author
|
cc: @joker-eph https://lab.llvm.org/buildbot/#/builders/169/builds/16768, should we revert it? |
linuxlonelyeagle
added a commit
that referenced
this pull request
Nov 6, 2025
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Nov 6, 2025
…arguments first" (#166746) Reverts llvm/llvm-project#165725. See https://lab.llvm.org/buildbot/#/builders/169/builds/16768,
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Nov 20, 2025
…perands before delete ops (#166766) Reland llvm/llvm-project#165725, fix the Failed test by removing successor operands before delete operations. Following the deletion of cond.branch, its successor operands will subsequently be removed.
nekoshirro
pushed a commit
to nekoshirro/Alchemist-LLVM
that referenced
this pull request
Nov 24, 2025
…ore delete ops (#166766) Reland llvm/llvm-project#165725, fix the Failed test by removing successor operands before delete operations. Following the deletion of cond.branch, its successor operands will subsequently be removed. Signed-off-by: Hafidz Muzakky <[email protected]>
aadeshps-mcw
pushed a commit
to aadeshps-mcw/llvm-project
that referenced
this pull request
Nov 26, 2025
…ore delete ops (llvm#166766) Reland llvm#165725, fix the Failed test by removing successor operands before delete operations. Following the deletion of cond.branch, its successor operands will subsequently be removed.
Priyanshu3820
pushed a commit
to Priyanshu3820/llvm-project
that referenced
this pull request
Nov 26, 2025
…ore delete ops (llvm#166766) Reland llvm#165725, fix the Failed test by removing successor operands before delete operations. Following the deletion of cond.branch, its successor operands will subsequently be removed.
augusto2112
pushed a commit
to augusto2112/llvm-project
that referenced
this pull request
Dec 3, 2025
…ore delete ops (llvm#166766) Reland llvm#165725, fix the Failed test by removing successor operands before delete operations. Following the deletion of cond.branch, its successor operands will subsequently be removed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #163051. Some Ops which have multiple blocks, before deleting the ops, first remove the dead parameters within its blocks.