Skip to content

Commit a72fe96

Browse files
linuxlonelyeaglejoker-eph
authored andcommitted
[mlir][dataflow] Fix LivenessAnalysis/RemoveDeadValues handling of loop induction variables (llvm#161117)
Fix llvm#157934. In liveness analysis, variables that are not analyzed are set as dead variables, but some variables are definitely live. --------- Co-authored-by: Mehdi Amini <[email protected]>
1 parent 2f0ae82 commit a72fe96

File tree

2 files changed

+57
-1
lines changed

2 files changed

+57
-1
lines changed

mlir/lib/Analysis/DataFlow/LivenessAnalysis.cpp

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,8 @@ void LivenessAnalysis::visitBranchOperand(OpOperand &operand) {
137137
// Populating such blocks in `blocks`.
138138
bool mayLive = false;
139139
SmallVector<Block *, 4> blocks;
140-
if (isa<RegionBranchOpInterface>(op)) {
140+
SmallVector<BlockArgument> argumentNotOperand;
141+
if (auto regionBranchOp = dyn_cast<RegionBranchOpInterface>(op)) {
141142
if (op->getNumResults() != 0) {
142143
// This mark value of type 1.c liveness as may live, because the region
143144
// branch operation has a return value, and the non-forwarded operand can
@@ -165,6 +166,25 @@ void LivenessAnalysis::visitBranchOperand(OpOperand &operand) {
165166
blocks.push_back(&block);
166167
}
167168
}
169+
170+
// In the block of the successor block argument of RegionBranchOpInterface,
171+
// there may be arguments of RegionBranchOpInterface, such as the IV of
172+
// scf.forOp. Explicitly set this argument to live.
173+
for (Region &region : op->getRegions()) {
174+
SmallVector<RegionSuccessor> successors;
175+
regionBranchOp.getSuccessorRegions(region, successors);
176+
for (RegionSuccessor successor : successors) {
177+
if (successor.isParent())
178+
continue;
179+
auto arguments = successor.getSuccessor()->getArguments();
180+
ValueRange regionInputs = successor.getSuccessorInputs();
181+
for (auto argument : arguments) {
182+
if (llvm::find(regionInputs, argument) == regionInputs.end()) {
183+
argumentNotOperand.push_back(argument);
184+
}
185+
}
186+
}
187+
}
168188
} else if (isa<BranchOpInterface>(op)) {
169189
// We cannot track all successor blocks of the branch operation(More
170190
// specifically, it's the successor's successor). Additionally, different
@@ -224,13 +244,24 @@ void LivenessAnalysis::visitBranchOperand(OpOperand &operand) {
224244
Liveness *operandLiveness = getLatticeElement(operand.get());
225245
LDBG() << "Marking branch operand live: " << operand.get();
226246
propagateIfChanged(operandLiveness, operandLiveness->markLive());
247+
for (BlockArgument argument : argumentNotOperand) {
248+
Liveness *argumentLiveness = getLatticeElement(argument);
249+
LDBG() << "Marking RegionBranchOp's argument live: " << argument;
250+
// TODO: this is overly conservative: we should be able to eliminate
251+
// unused values in a RegionBranchOpInterface operation but that may
252+
// requires removing operation results which is beyond current
253+
// capabilities of this pass right now.
254+
propagateIfChanged(argumentLiveness, argumentLiveness->markLive());
255+
}
227256
}
228257

229258
// Now that we have checked for memory-effecting ops in the blocks of concern,
230259
// we will simply visit the op with this non-forwarded operand to potentially
231260
// mark it "live" due to type (1.a/3) liveness.
232261
SmallVector<Liveness *, 4> operandLiveness;
233262
operandLiveness.push_back(getLatticeElement(operand.get()));
263+
for (BlockArgument argument : argumentNotOperand)
264+
operandLiveness.push_back(getLatticeElement(argument));
234265
SmallVector<const Liveness *, 4> resultsLiveness;
235266
for (const Value result : op->getResults())
236267
resultsLiveness.push_back(getLatticeElement(result));

mlir/test/Transforms/remove-dead-values.mlir

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,3 +649,28 @@ func.func @callee(%arg0: index, %arg1: index, %arg2: index) -> index {
649649
%res = call @mutl_parameter(%arg0, %arg1, %arg2) : (index, index, index) -> (index)
650650
return %res : index
651651
}
652+
653+
// -----
654+
655+
// This test verifies that the induction variables in loops are not deleted, the loop has results.
656+
657+
// CHECK-LABEL: func @dead_value_loop_ivs
658+
func.func @dead_value_loop_ivs_has_result(%lb: index, %ub: index, %step: index, %b: i1) -> i1 {
659+
%loop_ret = scf.for %iv = %lb to %ub step %step iter_args(%iter = %b) -> (i1) {
660+
cf.assert %b, "loop not dead"
661+
scf.yield %b : i1
662+
}
663+
return %loop_ret : i1
664+
}
665+
666+
// -----
667+
668+
// This test verifies that the induction variables in loops are not deleted, the loop has no results.
669+
670+
// CHECK-LABEL: func @dead_value_loop_ivs_no_result
671+
func.func @dead_value_loop_ivs_no_result(%lb: index, %ub: index, %step: index, %input: memref<?xf32>, %value: f32, %pos: index) {
672+
scf.for %iv = %lb to %ub step %step {
673+
memref.store %value, %input[%pos] : memref<?xf32>
674+
}
675+
return
676+
}

0 commit comments

Comments
 (0)