[PATCH] D145634: [X86] Support llvm.{min,max}imum.f{32,64}

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:53215
 
+static SDValue combineFMinimumFMaximum(SDNode *N, SelectionDAG &DAG,
+                                       const X86Subtarget &Subtarget) {
----------------
The code doesn't combine anything, should be moved to `LowerFMinimumFMaximum`?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:53256
+  // to ensure the correct zero is returned.
+  auto IsPreferredZero = [PreferredZero](SDValue Op) {
+    Op = peekThroughBitcasts(Op);
----------------
Should we not do it for `hasNoSignedZeros`?


================
Comment at: llvm/test/CodeGen/X86/fminimum-fmaximum.ll:181
+; SSE2:       # %bb.0:
+; SSE2-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
+; SSE2-NEXT:    retq
----------------
The test does show anything interesting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145634