[Mlir-commits] [mlir] NFC. Move out and expose affine expression simplification utility out of AffineOps lib (PR #69813)
Kunwar Grover
llvmlistbot at llvm.org
Wed Oct 25 18:31:10 PDT 2023
Groverkss wrote:
> > I'm not sure I understand why something like FlatLinearConstraints would call out to this IR method instead of Presburger methods to do the same.
> > Im not very supportive of doing such analysis on AffineMap. In my view, anything that requires a flattening of the AffineMap should be done be done on the flattened form, i.e. to get the bounds, first flatten it and then use Presburger methods to get these bounds (can be restricted to fast checks only). I do not have strong opinions on this, and would like to hear what others think.
>
> @Groverkss Note that this method, which is being moved (`getBoundForAffineExpr`) does **not** work on the flat form. It works on the "native" tree form. Did you miss that? (It's a linear time method.) Actually, `detectAsMod` (which is in FlatLinearConstraints.cpp), and similar utilities have a use case for this method. `detectAsMod` is used by `FlatLinearConstraints::getSliceBounds`.
>
> Anyone wanting to do any basic simplification on an affine expression (by exploiting lower or upper bounds on it) will need this method - `FlatLinearConstraints::getSliceBounds` is one example -- those aren't restricted to AffineOps or AffineAnalysis libraries.
Thank you for the example on getSliceBounds. I understand what you mean better now.
I do see that the method works on native form, but I also looked at the implementation, and it essentially flattens the AffineMap and then does analysis on it. I know it handles some divs/mods differently, but that can also be handled easily in flattened form.
What I'm asking is that instead of having such methods that flatten and do analysis, the user should do explicit conversion to the flattened form (concert to Presburger) and then do analysis there. I do not want to end up in case where we duplicate a lot of simplification functionality directly in AffineMap.
This is just a discussion that I felt should be had at some point, and I'm okay with landing this and coming and removing these methods later if we come to a consensus.
https://github.com/llvm/llvm-project/pull/69813
More information about the Mlir-commits
mailing list