[DAGCombiner] Fold fmin/fmax with INF

[nikic]

Summary

Similar to D87415, this folds the various float min/max opcodes with a constant INF or -INF operand, or FLT_MAX / -FLT_MAX operand if the ninf flag is set. Some of the folds are only possible under nnan.

The fminnum(X, INF) with nnan and fmaxnum(X, -INF) with nnan cases are needed to improve the VECREDUCE_FMIN/FMAX lowerings on X86, the rest is here for the sake of completeness.

[spatel]

LGTM. Should we common/integrate the caller functions/switch cases? Ie, we're switching on min/max opcode, but then translating that knowledge into the bool flags and then predicating on those flags instead of the opcodes.

Comparing this to InstSimplify...I think we have 2 problems there:

1. D52766 introduced a miscompile for maximum/minimum (need more tests)
2. Optimizations are missing (noted with TODO comments)

There's also another set of potential optimization to include here (based on ninf):

// TODO: minnum(nnan ninf x, flt_max) -> x
// TODO: maxnum(nnan ninf x, -flt_max) -> x

Let me know if you plan to work on those. If not, I can do it.

[nikic]

Also fold FLT_MAX/-FLT_MAX if ninf flag is set.

[nikic]

In D87571#2270204, @spatel wrote:

LGTM. Should we common/integrate the caller functions/switch cases? Ie, we're switching on min/max opcode, but then translating that knowledge into the bool flags and then predicating on those flags instead of the opcodes.

I've kept the flags but moved their definition into visitFMinMax, based on the opcode. Does that look reasonable? I think unpacking into the two flags reads a bit nicer than checking opcodes everywhere.

Comparing this to InstSimplify...I think we have 2 problems there:

1. D52766 introduced a miscompile for maximum/minimum (need more tests)
2. Optimizations are missing (noted with TODO comments)

There's also another set of potential optimization to include here (based on ninf):

// TODO: minnum(nnan ninf x, flt_max) -> x
// TODO: maxnum(nnan ninf x, -flt_max) -> x

Let me know if you plan to work on those. If not, I can do it.

For the sake of completeness, I've extended this patch to handle the +/-flt_max case as well. I'd be happy to leave the InstSimplify part of this to you though :)

[nikic]

Fix confusion between isSmallest and isLargest+isNegative.

[spatel]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

14078–14080

There's a bug here that should be addressed first (probably lacking test coverage) - we dropped the FMF when creating the new node.
It would be more efficient to std::swap(N0, N1) and just use those local names in all of the later code?

14097–14098

If we're using the same order of functions as in the code comment above here, the max/min names are inverted.

14105

"+inf" should be "-inf" on this line?

[nikic]

Preserve flags when commuting operands, fix typos in comments.

[nikic]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

14078–14080

I added some tests with nnan and commuted operands. Turns out that for the scalar case this canonicalization happens somewhere else already (maybe node building already handles it?) and this code only affects the vector constant case.

I've kept the code (with fmf propagation) as the same canonicalization seems to also be done for other commutative nodes.

[spatel]

Expanded patch still LGTM.
Side note: change "isLargest" name to "isLargestMagnitude" to make that API less confusing?

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

SDValue DAGCombiner::visitFNEG(SDNode *N) {
   if (SDValue Cast = foldSignChangeInBitcast(N))
     return Cast;
 
   return SDValue();
 }
 
 static SDValue visitFMinMax(SelectionDAG &DAG, SDNode *N,
                             APFloat (*Op)(const APFloat &, const APFloat &),
-                            bool PropagatesNaN) {
+                            bool IsMin, bool PropagatesNaN) {
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   EVT VT = N->getValueType(0);
   const ConstantFPSDNode *N0CFP = isConstOrConstSplatFP(N0);
   const ConstantFPSDNode *N1CFP = isConstOrConstSplatFP(N1);
+  const SDNodeFlags Flags = N->getFlags();
 
   if (N0CFP && N1CFP) {
     const APFloat &C0 = N0CFP->getValueAPF();
     const APFloat &C1 = N1CFP->getValueAPF();
     return DAG.getConstantFP(Op(C0, C1), SDLoc(N), VT);
   }
 
   // Canonicalize to constant on RHS.
   if (isConstantFPBuildVectorOrConstantFP(N0) &&
       !isConstantFPBuildVectorOrConstantFP(N1))
     return DAG.getNode(N->getOpcode(), SDLoc(N), VT, N1, N0);
 
+  if (N1CFP) {
     // minnum(X, nan) -> X
     // maxnum(X, nan) -> X
     // minimum(X, nan) -> nan
     // maximum(X, nan) -> nan
-  if (N1CFP && N1CFP->isNaN())
+    if (N1CFP->isNaN())
       return PropagatesNaN ? N->getOperand(1) : N->getOperand(0);
 
+    // minnum(X, -inf) -> -inf
+    // maxnum(X, +inf) -> +inf
+    // maximum(X, -inf) -> -inf if nnan
+    // minimum(X, +inf) -> +inf if nnan
+    if (N1CFP->isInfinity() && N1CFP->isNegative() == IsMin &&
+        (!PropagatesNaN || Flags.hasNoNaNs()))
+      return N->getOperand(1);
+
+    // minnum(X, +inf) -> X if nnan
+    // maxnum(X, -inf) -> X if nnan
+    // minimum(X, +inf) -> X
+    // maximum(X, +inf) -> X
+    if (N1CFP->isInfinity() && N1CFP->isNegative() != IsMin &&
+        (PropagatesNaN || Flags.hasNoNaNs()))
+      return N->getOperand(0);
+  }
+
   return SDValue();
 }
 
 SDValue DAGCombiner::visitFMINNUM(SDNode *N) {
-  return visitFMinMax(DAG, N, minnum, /* PropagatesNaN */ false);
+  return visitFMinMax(DAG, N, minnum, /* IsMin */ true,
+                      /* PropagatesNaN */ false);
 }
 
 SDValue DAGCombiner::visitFMAXNUM(SDNode *N) {
-  return visitFMinMax(DAG, N, maxnum, /* PropagatesNaN */ false);
+  return visitFMinMax(DAG, N, maxnum, /* IsMin */ false,
+                      /* PropagatesNaN */ false);
 }
 
 SDValue DAGCombiner::visitFMINIMUM(SDNode *N) {
-  return visitFMinMax(DAG, N, minimum, /* PropagatesNaN */ true);
+  return visitFMinMax(DAG, N, minimum, /* IsMin */ true,
+                      /* PropagatesNaN */ true);
 }
 
 SDValue DAGCombiner::visitFMAXIMUM(SDNode *N) {
-  return visitFMinMax(DAG, N, maximum, /* PropagatesNaN */ true);
+  return visitFMinMax(DAG, N, maximum, /* IsMin */ false,
+                      /* PropagatesNaN */ true);
 }
 
 SDValue DAGCombiner::visitFABS(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   EVT VT = N->getValueType(0);
 
   // fold (fabs c1) -> fabs(c1)
   if (isConstantFPBuildVectorOrConstantFP(N0))

llvm/test/CodeGen/ARM/fminmax-folds.ll

 ; CHECK-NEXT:    .long 0x7f800000 @ float +Inf
   %r = call float @llvm.minnum.f32(float %x, float 0x7ff0000000000000)
   ret float %r
 }
 
 define float @test_maxnum_const_inf(float %x) {
 ; CHECK-LABEL: test_maxnum_const_inf:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI5_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vmaxnm.f32 s0, s2, s0
-; CHECK-NEXT:    vmov r0, s0
+; CHECK-NEXT:    movw r0, #0
+; CHECK-NEXT:    movt r0, #32640
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI5_0:
-; CHECK-NEXT:    .long 0x7f800000 @ float +Inf
   %r = call float @llvm.maxnum.f32(float %x, float 0x7ff0000000000000)
   ret float %r
 }
 
 define float @test_maximum_const_inf(float %x) {
 ; CHECK-LABEL: test_maximum_const_inf:
 ; CHECK:       @ %bb.0:
 ; CHECK-NEXT:    vldr s0, .LCPI6_0
 ; CHECK-NEXT:    vmov s2, r0
 ; CHECK-NEXT:    vmax.f32 d0, d1, d0
 ; CHECK-NEXT:    vmov r0, s0
 ; CHECK-NEXT:    bx lr
 ; CHECK-NEXT:    .p2align 2
 ; CHECK-NEXT:  @ %bb.1:
 ; CHECK-NEXT:  .LCPI6_0:
 ; CHECK-NEXT:    .long 0x7f800000 @ float +Inf
   %r = call float @llvm.maximum.f32(float %x, float 0x7ff0000000000000)
   ret float %r
 }
 
 define float @test_minimum_const_inf(float %x) {
 ; CHECK-LABEL: test_minimum_const_inf:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI7_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vmin.f32 d0, d1, d0
-; CHECK-NEXT:    vmov r0, s0
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI7_0:
-; CHECK-NEXT:    .long 0x7f800000 @ float +Inf
   %r = call float @llvm.minimum.f32(float %x, float 0x7ff0000000000000)
   ret float %r
 }
 
 define float @test_minnum_const_ninf(float %x) {
 ; CHECK-LABEL: test_minnum_const_ninf:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI8_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vminnm.f32 s0, s2, s0
-; CHECK-NEXT:    vmov r0, s0
+; CHECK-NEXT:    movw r0, #0
+; CHECK-NEXT:    movt r0, #65408
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI8_0:
-; CHECK-NEXT:    .long 0xff800000 @ float -Inf
   %r = call float @llvm.minnum.f32(float %x, float 0xfff0000000000000)
   ret float %r
 }
 
 define float @test_maxnum_const_ninf(float %x) {
 ; CHECK-LABEL: test_maxnum_const_ninf:
 ; CHECK:       @ %bb.0:
 ; CHECK-NEXT:    vldr s0, .LCPI9_0
 ; CHECK-NEXT:    vmov s2, r0
 ; CHECK-NEXT:    vmaxnm.f32 s0, s2, s0
 ; CHECK-NEXT:    vmov r0, s0
 ; CHECK-NEXT:    bx lr
 ; CHECK-NEXT:    .p2align 2
 ; CHECK-NEXT:  @ %bb.1:
 ; CHECK-NEXT:  .LCPI9_0:
 ; CHECK-NEXT:    .long 0xff800000 @ float -Inf
   %r = call float @llvm.maxnum.f32(float %x, float 0xfff0000000000000)
   ret float %r
 }
 
 define float @test_maximum_const_ninf(float %x) {
 ; CHECK-LABEL: test_maximum_const_ninf:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI10_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vmax.f32 d0, d1, d0
-; CHECK-NEXT:    vmov r0, s0
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI10_0:
-; CHECK-NEXT:    .long 0xff800000 @ float -Inf
   %r = call float @llvm.maximum.f32(float %x, float 0xfff0000000000000)
   ret float %r
 }
 
 define float @test_minimum_const_ninf(float %x) {
 ; CHECK-LABEL: test_minimum_const_ninf:
 ; CHECK:       @ %bb.0:
 ; CHECK-NEXT:    vldr s0, .LCPI11_0
 ; CHECK-NEXT:    vmov s2, r0
 ; CHECK-NEXT:    vmin.f32 d0, d1, d0
 ; CHECK-NEXT:    vmov r0, s0
 ; CHECK-NEXT:    bx lr
 ; CHECK-NEXT:    .p2align 2
 ; CHECK-NEXT:  @ %bb.1:
 ; CHECK-NEXT:  .LCPI11_0:
 ; CHECK-NEXT:    .long 0xff800000 @ float -Inf
   %r = call float @llvm.minimum.f32(float %x, float 0xfff0000000000000)
   ret float %r
 }
 
 define float @test_minnum_const_inf_nnan(float %x) {
 ; CHECK-LABEL: test_minnum_const_inf_nnan:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI12_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vminnm.f32 s0, s2, s0
-; CHECK-NEXT:    vmov r0, s0
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI12_0:
-; CHECK-NEXT:    .long 0x7f800000 @ float +Inf
   %r = call nnan float @llvm.minnum.f32(float %x, float 0x7ff0000000000000)
   ret float %r
 }
 
 define float @test_maxnum_const_inf_nnan(float %x) {
 ; CHECK-LABEL: test_maxnum_const_inf_nnan:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI13_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vmaxnm.f32 s0, s2, s0
-; CHECK-NEXT:    vmov r0, s0
+; CHECK-NEXT:    movw r0, #0
+; CHECK-NEXT:    movt r0, #32640
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI13_0:
-; CHECK-NEXT:    .long 0x7f800000 @ float +Inf
   %r = call nnan float @llvm.maxnum.f32(float %x, float 0x7ff0000000000000)
   ret float %r
 }
 
 define float @test_maximum_const_inf_nnan(float %x) {
 ; CHECK-LABEL: test_maximum_const_inf_nnan:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI14_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vmax.f32 d0, d1, d0
-; CHECK-NEXT:    vmov r0, s0
+; CHECK-NEXT:    movw r0, #0
+; CHECK-NEXT:    movt r0, #32640
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI14_0:
-; CHECK-NEXT:    .long 0x7f800000 @ float +Inf
   %r = call nnan float @llvm.maximum.f32(float %x, float 0x7ff0000000000000)
   ret float %r
 }
 
 define float @test_minimum_const_inf_nnan(float %x) {
 ; CHECK-LABEL: test_minimum_const_inf_nnan:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI15_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vmin.f32 d0, d1, d0
-; CHECK-NEXT:    vmov r0, s0
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI15_0:
-; CHECK-NEXT:    .long 0x7f800000 @ float +Inf
   %r = call nnan float @llvm.minimum.f32(float %x, float 0x7ff0000000000000)
   ret float %r
 }
 
 define float @test_minnum_const_ninf_nnan(float %x) {
 ; CHECK-LABEL: test_minnum_const_ninf_nnan:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI16_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vminnm.f32 s0, s2, s0
-; CHECK-NEXT:    vmov r0, s0
+; CHECK-NEXT:    movw r0, #0
+; CHECK-NEXT:    movt r0, #65408
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI16_0:
-; CHECK-NEXT:    .long 0xff800000 @ float -Inf
   %r = call nnan float @llvm.minnum.f32(float %x, float 0xfff0000000000000)
   ret float %r
 }
 
 define float @test_maxnum_const_ninf_nnan(float %x) {
 ; CHECK-LABEL: test_maxnum_const_ninf_nnan:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI17_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vmaxnm.f32 s0, s2, s0
-; CHECK-NEXT:    vmov r0, s0
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI17_0:
-; CHECK-NEXT:    .long 0xff800000 @ float -Inf
   %r = call nnan float @llvm.maxnum.f32(float %x, float 0xfff0000000000000)
   ret float %r
 }
 
 define float @test_maximum_const_ninf_nnan(float %x) {
 ; CHECK-LABEL: test_maximum_const_ninf_nnan:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI18_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vmax.f32 d0, d1, d0
-; CHECK-NEXT:    vmov r0, s0
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI18_0:
-; CHECK-NEXT:    .long 0xff800000 @ float -Inf
   %r = call nnan float @llvm.maximum.f32(float %x, float 0xfff0000000000000)
   ret float %r
 }
 
 define float @test_minimum_const_ninf_nnan(float %x) {
 ; CHECK-LABEL: test_minimum_const_ninf_nnan:
 ; CHECK:       @ %bb.0:
-; CHECK-NEXT:    vldr s0, .LCPI19_0
-; CHECK-NEXT:    vmov s2, r0
-; CHECK-NEXT:    vmin.f32 d0, d1, d0
-; CHECK-NEXT:    vmov r0, s0
+; CHECK-NEXT:    movw r0, #0
+; CHECK-NEXT:    movt r0, #65408
 ; CHECK-NEXT:    bx lr
-; CHECK-NEXT:    .p2align 2
-; CHECK-NEXT:  @ %bb.1:
-; CHECK-NEXT:  .LCPI19_0:
-; CHECK-NEXT:    .long 0xff800000 @ float -Inf
   %r = call nnan float @llvm.minimum.f32(float %x, float 0xfff0000000000000)
   ret float %r
 }