[PATCH] D73848: Add AffineMaxOp

[ouhang via Phabricator via llvm-commits]

OuHangKresnik added a comment.

Thank you for the review! Replied with some questions.



================
Comment at: mlir/include/mlir/Dialect/AffineOps/AffineOps.td:238
+class Affine_ReduceOp <string mnemonic, list<OpTrait> traits = []> :
+    Op<Affine_Dialect, mnemonic, traits> {
+  let arguments = (ins AffineMapAttr:$map, Variadic<Index>:$operands);
----------------
ftynse wrote:
> I think we should mark both min and max as not having side effects (in a follow-up commit since min was not marked as such and the change may break something)
Ok, I will do this change in next commit?


================
Comment at: mlir/test/AffineOps/ops.mlir:84
+func @affine_max(%arg0 : index, %arg1 : index, %arg2 : index) {
+  // CHECK: affine.max #[[MAP0]](%arg0)[%arg1]
+  %0 = affine.max affine_map<(d0)[s0] -> (1000, d0 + 512, s0)> (%arg0)[%arg1]
----------------
ftynse wrote:
> Please don't use SSA names in CHECK lines, even if the file around does. See https://mlir.llvm.org/getting_started/TestingGuide/ for more details.
Here follows the affine_min test.
Change them both ? or I just change affine_max


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73848