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

[Marius Brehler]

marbre wrote:

> 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.

Thanks for your suggestion! I am currently looking into how to adapt torch-mlir but I am unfortunately not familiar with the data-flow analysis and might have missed something. Therefore please allow to ask for further clarification.

While the argument to the `visit(ProgramPoint point)` method still is of a `ProgramPoint`, the latter was changed and now essentially is a `PointerUnion`. Before this change, `point` could be casted to a `Value` (within `visit()`) which is no longer possible. To get the value, one could instead cast `point` to an `Operation` and obtain the result value from this op. Furthermore, `getProgramPoint` needs to be replaced with `getLatticeAnchor`, thus
```C++
auto *flatSymbolRefPoint = getProgramPoint<FlatSymbolRefProgramPoint>(globalSlotGet.getSlotAttr());
auto *valueState = getOrCreateFor<InlineGlobalSlotsAnalysisState>(flatSymbolRefPoint, globalSlotGet.getResult()):
```
becomes
```C++
auto *flatSymbolRefPoint = getLatticeAnchor<FlatSymbolRefProgramPoint>(globalSlotGet.getSlotAttr());
auto *valueState = getOrCreateFor<InlineGlobalSlotsAnalysisState>(*flatSymbolRefPoint, globalSlotGet.getResult()):
```
What is failing here is `getOrCreateFor` as the first argument needs to be a `ProgramPoint` but now is a `FlatSymbolRefProgramPoint`. Any pointers would be appreciated!

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