Mark arith.minf, arith.maxf as commutative.

[herhut]

Thanks!

[kiranchandramohan]

Just a drive through question, Is this valid when there are NaNs? Should this be restricted to situations like fast math?

[jpienaar]

+1, this also gets interesting with +0 and -0. It is possible to do this, but then backends will need to work harder to ensure this behavior. Could you add motivation to description here?

[herhut]

The arith::minf operation is defined as

Returns the minimum of the two arguments, treating -0.0 as less than +0.0.
    If one of the arguments is NaN, then the result is also NaN.

which makes it commutative.

[csigg]

The motivation is to put constants on the right, so that parts of the minf/maxf expansion with constants can be folded (see D117011).

But it might indeed be better to not mark minf/maxf commutative.
Otherwise, we would likely need specify that all NaNs are 'same' (i.e. ignoring the payload).

If 'same' is IEEE equal, they are clearly not commutative for NaNs.
If 'same' is memcmp, the expansion in D117011 will not be valid.

I can't think of how +0/-0 would be special wrt to commutativity.

mlir/include/mlir/Dialect/Arithmetic/IR/ArithmeticOps.td

def Arith_SubFOp : Arith_FloatBinaryOp<"subf"> {
   let summary = "floating point subtraction operation";
   let hasFolder = 1;
 }
 
 //===----------------------------------------------------------------------===//
 // MaxFOp
 //===----------------------------------------------------------------------===//
 
-def Arith_MaxFOp : Arith_FloatBinaryOp<"maxf"> {
+def Arith_MaxFOp : Arith_FloatBinaryOp<"maxf", [Commutative]> {
   let summary = "floating-point maximum operation";
   let description = [{
     Syntax:
 
     ```
     operation ::= ssa-id `=` `arith.maxf` ssa-use `,` ssa-use `:` type
     ```
 
def Arith_MaxUIOp : Arith_IntBinaryOp<"maxui"> {
   let summary = "unsigned integer maximum operation";
   let hasFolder = 1;
 }
 
 //===----------------------------------------------------------------------===//
 // MinFOp
 //===----------------------------------------------------------------------===//
 
-def Arith_MinFOp : Arith_FloatBinaryOp<"minf"> {
+def Arith_MinFOp : Arith_FloatBinaryOp<"minf", [Commutative]> {
   let summary = "floating-point minimum operation";
   let description = [{
     Syntax:
 
     ```
     operation ::= ssa-id `=` `arith.minf` ssa-use `,` ssa-use `:` type
     ```
 

mlir/test/Dialect/Arithmetic/canonicalize.mlir

 // CHECK-LABEL: @constant_UItoFP(
 func @constant_UItoFP() -> f32 {
   // CHECK: %[[C0:.+]] = arith.constant 2.000000e+00 : f32
   // CHECK: return %[[C0]]
   %c0 = arith.constant 2 : i32
   %res = arith.sitofp %c0 : i32 to f32
   return %res : f32
 }
+
+// -----
+// CHECK-LABEL: @constant_MinMax(
+func @constant_MinMax(%arg0 : f32) -> f32 {
+  // CHECK:  %[[const:.+]] = arith.constant
+  // CHECK:  %[[min:.+]] = arith.minf %arg0, %[[const]] : f32
+  // CHECK:  %[[res:.+]] = arith.maxf %[[min]], %[[const]] : f32
+  // CHECK:   return %[[res]]
+  %const = arith.constant 0.0 : f32
+  %min = arith.minf %const, %arg0 : f32
+  %res = arith.maxf %const, %min : f32
+  return %res : f32
+}