[PATCH] D73848: Add AffineMaxOp

[ouhang via Phabricator via llvm-commits]

OuHangKresnik added a comment.

Reply



================
Comment at: mlir/include/mlir/Dialect/AffineOps/AffineOps.td:237
 
-def AffineMinOp : Affine_Op<"min"> {
+class Affine_ReduceOp <string mnemonic, list<OpTrait> traits = []> :
+    Op<Affine_Dialect, mnemonic, traits> {
----------------
nicolasvasilache wrote:
> this should be in a separate revision.
Since there are many common code between AffineMax and AffineMIn.
The introduction of reduceOp (class not def) here is to try to avoid repeated code.

I think the same code could be applied to Ops like avg / sum ops

Any better way ? If there is no such ReduceOp ?


================
Comment at: mlir/lib/Dialect/AffineOps/AffineOps.cpp:2027
+
+OpFoldResult AffineMaxOp::fold(ArrayRef<Attribute> operands) {
+  // Fold the affine map.
----------------
nicolasvasilache wrote:
> this should be in a separate revision and have a test in canonicalize.mlir.
I wrote two tests in canonicalize.mlir which follows the tests of AffineMin,  do you mean more tests needed there ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73848