[PATCH] D113442: [InstCombine] Enable fold select into operand for FAdd, FMul, and FSub.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:257
   case Instruction::Sub:   // Can only fold on the amount subtracted.
+  case Instruction::FSub:
   case Instruction::Shl:   // Can only fold on the shift amount.
----------------
Why exclude fdiv?


================
Comment at: llvm/test/Transforms/InstCombine/select-binop-cmp.ll:307
 
+define float @select_fadd_fcmp_7_fold_select_into_op(float %x, float %y, float %z) {
+; CHECK-LABEL: @select_fadd_fcmp_7_fold_select_into_op(
----------------
Please pre-commit the baseline tests. Some of these tests should include fast-math-flags on the FP binop, so we can verify that FMF propagates as expected.


================
Comment at: llvm/test/Transforms/InstCombine/select-binop-cmp.ll:330
+  %B = fadd float %x, %z
+  %C = select i1 %A, float %B, float %z
+  ret float %C
----------------
Why is the compare constant (0.0 or -0.0) relevant for this fold?

The true/false operands should be swapped so we have coverage for the pattern that replaces the true value with a constant. Similarly for `fmul`, there should be two tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113442