[InstCombine] sink FP negation of operands through select

[spatel]

Summary

I was looking at an independent problem for FP min/max patterns, when I noticed we don't always get this:

Cond ? -X : -Y --> -(Cond ? X : Y)

...even with the legacy IR form of fneg in the case with extra uses.

This fold isn't included in D61447. We might want to split that up into pieces that are similar to this patch, so we can gradually introduce the new 'fneg' into IR without causing regressions.

[cameron.mcinally]

LGTM.

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

347 ↗

(On Diff #198312)

Do you know what pass needs updating so that we can use the UnOp form? I could work on that next...

If you don't know, that's fine. I can dig it up.

[spatel]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

347 ↗

(On Diff #198312)

I think it's mostly within instcombine - anywhere we're implicitly or explicitly (m_BinOp) matching generic binop patterns will not match with the new fneg in IR.

[cameron.mcinally]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

347 ↗

(On Diff #198312)

Ok. @kpn already has the bulk of InstCombine covered, so I'll leave that to him. If that's okay with you, Kevin.

llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp

if (TI->getNumOperands() == 1 && TI->isCast()) {
     // Fold this by inserting a select from the input values.
     Value *NewSI =
         Builder.CreateSelect(Cond, TI->getOperand(0), FI->getOperand(0),
                              SI.getName() + ".v", &SI);
     return CastInst::Create(Instruction::CastOps(TI->getOpcode()), NewSI,
                             TI->getType());
   }
 
+  // Cond ? -X : -Y --> -(Cond ? X : Y)
+  Value *X, *Y;
+  if (match(TI, m_FNeg(m_Value(X))) && match(FI, m_FNeg(m_Value(Y))) &&
+      (TI->hasOneUse() || FI->hasOneUse())) {
+    Value *NewSel = Builder.CreateSelect(Cond, X, Y, SI.getName() + ".v", &SI);
+    // TODO: Remove the hack for the binop form when the unary op is optimized
+    //       properly with all IR passes.
+    if (TI->getOpcode() != Instruction::FNeg)
+      return BinaryOperator::CreateFNegFMF(NewSel, cast<BinaryOperator>(TI));
+    return UnaryOperator::CreateFNeg(NewSel);
+  }
+
   // Only handle binary operators (including two-operand getelementptr) with
   // one-use here. As with the cast case above, it may be possible to relax the
   // one-use constraint, but that needs be examined carefully since it may not
   // reduce the total number of instructions.
   if (TI->getNumOperands() != 2 || FI->getNumOperands() != 2 ||
       (!isa<BinaryOperator>(TI) && !isa<GetElementPtrInst>(TI)) ||
       !TI->hasOneUse() || !FI->hasOneUse())
     return nullptr;

llvm/trunk/test/Transforms/InstCombine/fneg.ll

;
   ret <4 x double> %r
 }
 
 ; Sink FP negation through a select:
 ; c ? -x : -y --> -(c ? x : y)
 
 define <2 x double> @fneg_fneg_sel(<2 x double> %x, <2 x double> %y, i1 %cond) {
 ; CHECK-LABEL: @fneg_fneg_sel(
-; CHECK-NEXT:    [[N1:%.*]] = fneg <2 x double> [[X:%.*]]
-; CHECK-NEXT:    [[N2:%.*]] = fneg <2 x double> [[Y:%.*]]
-; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[COND:%.*]], <2 x double> [[N1]], <2 x double> [[N2]]
+; CHECK-NEXT:    [[SEL_V:%.*]] = select i1 [[COND:%.*]], <2 x double> [[X:%.*]], <2 x double> [[Y:%.*]]
+; CHECK-NEXT:    [[SEL:%.*]] = fneg <2 x double> [[SEL_V]]
 ; CHECK-NEXT:    ret <2 x double> [[SEL]]
 ;
   %n1 = fneg <2 x double> %x
   %n2 = fneg <2 x double> %y
   %sel = select i1 %cond, <2 x double> %n1, <2 x double> %n2
   ret <2 x double> %sel
 }
 
 ; An extra use is allowed.
 
 define float @fneg_fneg_sel_extra_use1(float %x, float %y, i1 %cond) {
 ; CHECK-LABEL: @fneg_fneg_sel_extra_use1(
 ; CHECK-NEXT:    [[N1:%.*]] = fneg float [[X:%.*]]
 ; CHECK-NEXT:    call void @use(float [[N1]])
-; CHECK-NEXT:    [[N2:%.*]] = fneg float [[Y:%.*]]
-; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[COND:%.*]], float [[N1]], float [[N2]]
+; CHECK-NEXT:    [[SEL_V:%.*]] = select i1 [[COND:%.*]], float [[X]], float [[Y:%.*]]
+; CHECK-NEXT:    [[SEL:%.*]] = fneg float [[SEL_V]]
 ; CHECK-NEXT:    ret float [[SEL]]
 ;
   %n1 = fneg float %x
   call void @use(float %n1)
   %n2 = fneg float %y
   %sel = select i1 %cond, float %n1, float %n2
   ret float %sel
 }
 
 define float @fneg_fneg_sel_extra_use2(float %x, float %y, i1 %cond) {
 ; CHECK-LABEL: @fneg_fneg_sel_extra_use2(
-; CHECK-NEXT:    [[N1:%.*]] = fneg float [[X:%.*]]
 ; CHECK-NEXT:    [[N2:%.*]] = fneg float [[Y:%.*]]
 ; CHECK-NEXT:    call void @use(float [[N2]])
-; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[COND:%.*]], float [[N1]], float [[N2]]
+; CHECK-NEXT:    [[SEL_V:%.*]] = select i1 [[COND:%.*]], float [[X:%.*]], float [[Y]]
+; CHECK-NEXT:    [[SEL:%.*]] = fneg float [[SEL_V]]
 ; CHECK-NEXT:    ret float [[SEL]]
 ;
   %n1 = fneg float %x
   %n2 = fneg float %y
   call void @use(float %n2)
   %sel = select i1 %cond, float %n1, float %n2
   ret float %sel
 }
 
 ; Legacy form of fneg should work too.
 
 define float @fsub_fsub_sel_extra_use1(float %x, float %y, i1 %cond) {
 ; CHECK-LABEL: @fsub_fsub_sel_extra_use1(
 ; CHECK-NEXT:    [[N1:%.*]] = fsub float -0.000000e+00, [[X:%.*]]
 ; CHECK-NEXT:    call void @use(float [[N1]])
-; CHECK-NEXT:    [[N2:%.*]] = fsub float -0.000000e+00, [[Y:%.*]]
-; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[COND:%.*]], float [[N1]], float [[N2]]
+; CHECK-NEXT:    [[SEL_V:%.*]] = select i1 [[COND:%.*]], float [[X]], float [[Y:%.*]]
+; CHECK-NEXT:    [[SEL:%.*]] = fsub float -0.000000e+00, [[SEL_V]]
 ; CHECK-NEXT:    ret float [[SEL]]
 ;
   %n1 = fsub float -0.0, %x
   call void @use(float %n1)
   %n2 = fsub float -0.0, %y
   %sel = select i1 %cond, float %n1, float %n2
   ret float %sel
 }