[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,
   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)))) {
     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)
+    else if (LHS->hasOneUse() && RHS->hasOneUse() && V2)
       SI = Builder.CreateSelect(A, V2, Builder.CreateBinOp(Opcode, C, E));
-    else if (V1)
+    else if (LHS->hasOneUse() && RHS->hasOneUse() && V1)
       SI = Builder.CreateSelect(A, Builder.CreateBinOp(Opcode, B, D), V1);
 
     if (SI)
       SI->takeName(&I);
   }
 
   return SI;
 }

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
 }
 
+declare void @use_float(float)
+; Tests folding constants if the selects have multiple uses but
+; we can fold away the binary op with a select.
+define float @test4(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_float(float %tmp)
+  ret float %tmp2
+; CHECK-LABEL: @test4(
+; CHECK: select i1 %arg, float 5.000000e+00, float 6.000000e+00
+; CHECK-NEXT: select i1 %arg, float 5.000000e+00, float 5.400000e+01
+; CHECK-NEXT: call void @use_float(float %tmp)
+; CHECK: ret float %tmp2
+}
+
+; Tests not folding constants if we cannot fold away any of the selects.
+define float @test5(i1 zeroext %arg, float %div) {
+  %tmp = select i1 %arg, float %div, float 5.000000e+00
+  %mul = fmul contract float %tmp, %tmp
+  call void @use_float(float %tmp)
+  ret float %mul
+; CHECK-LABEL: @test5(
+; CHECK: [[TMP:%.*]] = select i1 %arg, float %div, float 5.000000e+00
+; CHECK-NEXT: [[MUL:%.*]] = fmul contract float [[TMP]], [[TMP]]
+; CHECK-NOT: fmul contract float %div, %div
+; CHECK: ret float [[MUL]]
+}