[Mlir-commits] [mlir] [mlir][dataflow] Add visitBranchRegionArgument interface to SparseBackwardDataFlowAnalysis and apply it in LivenessAnalysis/RemoveDeadValues (PR #169816)

[lonely eagle]

================
@@ -609,12 +609,24 @@ void AbstractSparseBackwardDataFlowAnalysis::visitRegionSuccessors(
            *getLatticeElementFor(getProgramPointAfter(op), input));
       unaccounted.reset(operand.getOperandNumber());
     }
+
+    if (successor.isParent())
+      continue;
+    auto arguments = successor.getSuccessor()->getArguments();
+    for (BlockArgument argument : arguments) {
+      if (llvm::find(inputs, argument) == inputs.end()) {
+        regionArguments.push_back(argument);
+      }
+    }
   }
   // All operands not forwarded to regions are typically parameters of the
   // branch operation itself (for example the boolean for if/else).
   for (int index : unaccounted.set_bits()) {
     visitBranchOperand(op->getOpOperand(index));
   }
+  for (BlockArgument argument : regionArguments) {
+    visitBranchRegionArgument(argument);
----------------
linuxlonelyeagle wrote:

> It looks like we are calling visitBranchRegionArgument only for block arguments that not "control flow block arguments". E.g., for an scf.for, this would be called for the IV but not for the loop-carried variables. Is that correct? And why is that the case?

Yes, as above I say, IV is slightly different for inits/regionIters to me, as it is a property. I think we need a way to access this SSA value.

> Assuming that's correct, should this function maybe be called visitNonControlFlowArguments, similar to the SparseForwardDataFlowAnalysis?
Yes. But I wanted to see more opinions from everyone, so I didn't have time to make changes to everything.

https://github.com/llvm/llvm-project/pull/169816