[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:36:56 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`.
> 
> I double-checked the code - for one of the paths, it does use the flattener, but it also works through the AffineExpr tree. From a layering standpoint, lib/IR/AffineExpr.cpp itself contains that flattener and also a method that depends on it (mlir::simplifyAffineExpr). So from a layering and consistency standpoint, adding `mlir::getBoundForAffineExpr` in AffineExpr.cpp looks appropriate to me; in fact, `mlir::getBoundForAffineExpr` is a lower-level method than `mlir::simplifyAffineExpr`.

If other methods already do this, then let's land this and have this discussion later on discord/discourse. Thank you for the patch and the discussion! 

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


More information about the Mlir-commits mailing list