[PATCH] D53112: Rename minnan and maxnan to minimum and maximum

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added a comment.

- I guess this should be marked as NFC
- Does this mean you'd like to match the semantics of the existing `minnan`/`maxnan` in the backend to match the new `minimum`/`maximum` intrinsics? Then are you planning to update ValueTracking optimizations for them as well?



================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:389
+    setOperationAction(ISD::FMINIMUM,    MVT::f16, Promote);
+    setOperationAction(ISD::FMAXIMUM,    MVT::f16, Promote);
 
----------------
For `Promote`, I guess we can honor the existing indentation (even though clang-format does not like it)


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:820
+    for (unsigned Opcode : {ISD::FMINIMUM, ISD::FMAXIMUM,
                             ISD::FMINNUM, ISD::FMAXNUM})
       setOperationAction(Opcode, VT, Legal);
----------------
I know it is pre-existing, but as we are modifying it anyway, we can clang-format I guess


================
Comment at: test/CodeGen/WebAssembly/simd-arith.ll:804
 ; SIMD128-NEXT: f32x4.splat $push[[L1:[0-9]+]]=, $pop[[L0]]
-; SIMD128-NEXT: f32x4.min $push[[R:[0-9]+]]=, $pop[[L1]], $0{{$}}
+; SIMD128-NEXT: f32x4.min $push[[R:[0-9]+]]=, $0, $pop[[L1]]{{$}}
 ; SIMD128-NEXT: return $pop[[R]]{{$}}
----------------
Why is the operand order changed? The same for three more changes below this


Repository:
  rL LLVM

https://reviews.llvm.org/D53112