[Mlir-commits] [mlir] [mlir] [dataflow] Refactoring the definition of program points in dat… (PR #105656)

[donald chen]

cxy-1993 wrote:

> This change seems to have broken `torch-mlir`. I fully expect that this is probably an issue in torch-mlir itself, but The way the data flow framework is used in [`InlineGlobalSlots.cpp`](https://github.com/llvm/torch-mlir/blob/main/lib/Dialect/Torch/Transforms/InlineGlobalSlots.cpp) seems incompatible with this change. I havent been able to make changes to this to get this to build.
> 
> I dont have a repro or anything, and I am not fully upto speed on the DataFlow analysis or how it is used in Torch-MLIR but just leaving a note here that this seems to cause a breakage (due to potentially mis-use of the data flow framework)

Thanks for the feedback. It's clear that our current analysis doesn't align well with the patch's modifications. After a brief review of the code (please point out any errors), I believe this analysis is essentially a more robust version of sparse data flow analysis, extended to track attrs. The distinguishing characteristic is that the visit process might be triggered by an attr (symbol ref) rather than an operation.

In my opinion, this doesn't fundamentally deviate from the essence of current data flow analysis. When we update flat symbol attrs or op result, we still rely on initializeGlobalSlotsOp/GlobalSlotGetOp to update operand states. It's just that the current updates only modify the states of some operands in initializeGlobalSlotsOp(visit op will update all). Therefore, I suggest modifying the existing visit op result at (https://github.com/llvm/torch-mlir/blob/98e08023bbf71e00ab81e980eac9f7c96f1f24b4/lib/Dialect/Torch/Transforms/InlineGlobalSlots.cpp#L193C31-L193C46) to visit GlobalSlotGetOp, and modifying the visit flatSymbolRefPoint at (https://github.com/llvm/torch-mlir/blob/98e08023bbf71e00ab81e980eac9f7c96f1f24b4/lib/Dialect/Torch/Transforms/InlineGlobalSlots.cpp#L210) to visit initializeGlobalSlotsOp. This would align with the current dataflow analysis's definition of a program point.

If you have any further questions, please don't hesitate to discuss them with me. Additionally, I'm more than happy to assist you in modifying this section of code if needed.

I believe this modification is a step in the right direction for data flow analysis, and I hope it can be accepted by everyone. I plan to further standardize the semantics of program points, which may introduce some incompatibilities. I'm more than happy to help fix any issues that may arise.

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