[InstCombine] Restrict transforming shared selects using SimplifySelectsFeedingBinaryOp when we cannot simplify the binary op.

[bmakam]

Summary

We will still keep the original select if there are multiple uses thus increasing
the number of instructions, so enable this transform for shared selects only when
we can fold the binary op with a select.

[mcrosier]

I think we're on the right track, but what about this test?

declare void @use_float(double)
define float @test3(i1 zeroext %arg) #0 {
  %tmp = select i1 %arg, float 5.000000e+00, float 6.000000e+00
  %tmp1 = select i1 %arg, float 1.000000e+00, float 9.000000e+00
  %tmp2 = fmul float %tmp, %tmp1
  call void @use_double(double %tmp)
  ret float %tmp2
}

I think we still want to transform this case (because we'll replace the fmul with a select) even though the first select has multiple uses.

[mcrosier]

lib/Transforms/InstCombine/InstructionCombining.cpp

740

I wonder if something like this would work:

bool SelectsHaveOneUse = LHS->hasOneUse() && RHS->hasOneUse();
if (V1 && V2)
  SI = Builder.CreateSelect(A, V2, V1);
else if (V2 && SelectsHaveOneUse)
  SI = Builder.CreateSelect(A, V2, Builder.CreateBinOp(Opcode, C, E));
else if (V1 && SelectsHaveOneUse)
  SI = Builder.CreateSelect(A, Builder.CreateBinOp(Opcode, B, D), V1);

[bmakam]

Address Chad's testcase.

[mcrosier]

LGTM, but I'll defer to @majnemer and company for the final approval.

[mcberg2017]

lib/Transforms/InstCombine/InstructionCombining.cpp

740

An earlier review was opened here: https://reviews.llvm.org/D38263, from which the snippet above originates. Just to let folks know that are two reviews open on this topic.

[mcrosier]

lib/Transforms/InstCombine/InstructionCombining.cpp

740

Somehow I did a drive by comment on D38263 without realizing the two fixes were the same. I'd probably just go ahead and abandon this revision, Balaram.

[bmakam]

abandon this revision since it is a duplicate of D38263

lib/Transforms/InstCombine/InstructionCombining.cpp

 
 Value *InstCombiner::SimplifySelectsFeedingBinaryOp(BinaryOperator &I,
                                                     Value *LHS, Value *RHS) {
   Instruction::BinaryOps Opcode = I.getOpcode();
   // (op (select (a, b, c)), (select (a, d, e))) -> (select (a, (op b, d), (op
   // c, e)))
   Value *A, *B, *C, *D, *E;
   Value *SI = nullptr;
-  if (match(LHS, m_Select(m_Value(A), m_Value(B), m_Value(C))) &&
-      match(RHS, m_Select(m_Specific(A), m_Value(D), m_Value(E)))) {
+  if (match(LHS, m_OneUse(m_Select(m_Value(A), m_Value(B), m_Value(C)))) &&
+      match(RHS, m_OneUse(m_Select(m_Specific(A), m_Value(D), m_Value(E))))) {
     BuilderTy::FastMathFlagGuard Guard(Builder);
     if (isa<FPMathOperator>(&I))
       Builder.setFastMathFlags(I.getFastMathFlags());
 
     Value *V1 = SimplifyBinOp(Opcode, C, E, SQ.getWithInstruction(&I));
     Value *V2 = SimplifyBinOp(Opcode, B, D, SQ.getWithInstruction(&I));
     if (V1 && V2)
       SI = Builder.CreateSelect(A, V2, V1);
     else if (V2)
       SI = Builder.CreateSelect(A, V2, Builder.CreateBinOp(Opcode, C, E));
     else if (V1)
       SI = Builder.CreateSelect(A, Builder.CreateBinOp(Opcode, B, D), V1);
 
     if (SI)
       SI->takeName(&I);

test/Transforms/InstCombine/select_arithmetic.ll

define float @test3(i1 zeroext %arg) #0 {
   %tmp2 = fmul float %tmp, %tmp1
   ret float %tmp2
 ; CHECK-LABEL: @test3(
 ; CHECK: %tmp2 = select i1 %arg, float 5.000000e+00, float 5.400000e+01
 ; CHECK-NOT: fmul
 ; CHECK: ret float %tmp2
 }
 
+; Tests not folding constants if the selects have more than one use.
+declare void @use_double(double)
+define double @test4(i1 zeroext %arg, double %div) {
+  %tmp = select i1 %arg, double %div, double 5.000000e-03
+  %mul = fmul contract double %tmp, %tmp
+  call void @use_double(double %tmp)
+  ret double %mul
+; CHECK-LABEL: @test4(
+; CHECK: [[TMP:%.*]] = select i1 %arg, double %div, double 5.000000e-03
+; CHECK-NEXT: [[MUL:%.*]] = fmul contract double [[TMP]], [[TMP]]
+; CHECK-NOT: fmul contract double %div, %div
+; CHECK: ret double [[MUL]]
+}