[Mlir-commits] [mlir] [mlir][dataflow] Add new visitNonControlFlowArgumentst API to SparseDataFlowAnalysis and apply it in LivenessAnalysis/RemoveDeadValues/IntegerRangeAnalysis (PR #169816)

[Oleksandr Alex Zinenko]

ftynse wrote:

> Frankly, I find the name of the function visitNonControlFlowArguments to be slightly confusing. It visits the regionOp's arguments (excluding SSA values such as Induction Variables (IVs)) and the operands of the yield Op within the regionOp, and then analyzes the resulting lattices for the regionOp's results. 

I commented inline here https://github.com/llvm/llvm-project/pull/169816#discussion_r2652733954. I agree it is confusing, and the terminology doesn't make it easy. This function takes a `RegionSuccessor`, but it is essentially only used to indicate which of the successors is being processed, e.g., `scf.while` has two regions with an alternating flow between them. We don't care about the forwarded values stored in `RegionSuccessor`. It also takes lattices of the region-carrying operation's _operands_, confusingly calling them arguments. These _may_ be relevant for the computation. For example, if we think about integer range lattice, the range of the the loop induction variable will depend on ranges of the loop bounds, that is, the operands of the loop operation itself. Arguably, the function could just call `getLatticeFor` internally instead, but let's ignore that for now.

>  You can see the implementation of IntegerRangeAnalysis for LoopInterface, which uses getLatticeElement(iv) within visitNonControlFlowArguments.

Yes, it is exactly what the function is supposed to do: update the lattice for the block argument that is not being forwarded to. It shouldn't update any other lattice.

> If my understanding is correct, we will first resolve the lifetime issues, which means completing the backward analysis. Then, we will submit a separate PR for the forward analysis.

Yes, please only keep the lifetime/backward part here. I think we need a `visitNonControlFlowArguments` in backward, specifically for the case where a block argument is not used directly as an operand anywhere, so no other function will back-propagate a lattice into it, but we want a non-default lattice for it. For forward analysis, I think we already have what we need.

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