[PATCH] D79502: [mlir] Support partial folding of affine.min/max

[Alex Zinenko via Phabricator via llvm-commits]

ftynse marked an inline comment as done.
ftynse added a comment.

In D79502#2023207 <https://reviews.llvm.org/D79502#2023207>, @bondhugula wrote:

> > partially folds the affine map by lifting individual constant expression in it
>
> Thanks for the detailed summary, but I'm sorry this part isn't clear. canonicalizeMapAndOperands does propagate constants from the operands into the maps. It isn't clear if you are doing the same or whether you are taking min/max over the subset of the results that are constant: for eg. `affine.min affine_map<(d0, d1, d2) -> (d0, d1, d2)> (2, 3, %N)` is  `affine.min affine_map<(d0) -> (2, d0)> (%N)`. I don't see any test cases of the latter form - so didn't look at the code carefully.


Canonicalization does more than just propagating the constants, it moves dimensions and symbols around, drops them, or does more aggressive simplifications of affine maps. The folding only looks if it can replace one AffineExpr (however complex) with an AffineConstantExpr by substituting the dimensions and symbols with constant operands. It will do the replacement if possible.

Taking a subset of results looks interesting, but this change doesn't do it either (I had a specific use case, TBH, where having any constant in a map suffices to overapproximate the result, so I did just that). I think there are more simplifications that we can do as foldings rather than canonicalizations, including operand/result removal, but leaving this for future work.



================
Comment at: mlir/lib/Dialect/Affine/IR/AffineOps.cpp:2122
   // Fold the affine map.
   // TODO(andydavis, ntv) Fold more cases: partial static information,
   // min(some_affine, some_affine + constant, ...).
----------------
andydavis1 wrote:
> Can you remove this TODO now?
My change doesn't do the folding given as example in the todo.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79502/new/

https://reviews.llvm.org/D79502