[PATCH] D119111: [X86] Invert a vector select IR canonicalization with a binop identity constant

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:48950
+      case ISD::FMUL: // X * 1.0 --> X
+      case ISD::FDIV: // X * 1.0 --> X
+        return C->isExactlyValue(1.0);
----------------
`/`


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:49012-49015
+  if (SDValue Sel = combineBinopWithSelect(N, DAG, Subtarget))
+    return Sel;
+
+  return SDValue();
----------------
Equals to `return combineBinopWithSelect(N, DAG, Subtarget)`?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:53889
   case ISD::FSUB:           return combineFaddFsub(N, DAG, Subtarget);
+  case ISD::FMUL:           return combineFmul(N, DAG, Subtarget);
+  case ISD::FDIV:           return combineFdiv(N, DAG, Subtarget);
----------------
Or call `combineBinopWithSelect` directly?


================
Comment at: llvm/test/CodeGen/X86/vector-bo-select.ll:345-346
 ; AVX512-NEXT:    vptestmd %zmm0, %zmm0, %k1
-; AVX512-NEXT:    vbroadcastss {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %zmm2 {%k1}
 ; AVX512-NEXT:    vmulps %zmm2, %zmm1, %zmm0
+; AVX512-NEXT:    vmovaps %zmm1, %zmm0 {%k1}
 ; AVX512-NEXT:    retq
----------------
Curiously: it should be equal to
```
vmulps %zmm2, %zmm1, %zmm1 {%k1}
vmovaps %zmm1, %zmm0
```
Why sometimes we use this way, sometime another?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119111