[DAG] Fix and expand fmin/fmax reassociation fold.

[dmgreen]

Summary

This call to reassociateReduction is used by both fminnum/fmaxnum and fminimum/fmaximum. In adding support for fminimum/fmaximum we appear to be fixing the use of an incorrect reduction type, which should have only applied to minnum/maxnum.

I also believe that it doesn't need nsz and reassoc to perform the reassociation. For float min/max it should always be valid.

[nikic]

LGTM

llvm/test/CodeGen/AArch64/double_reduct.ll

112

Can you please also add a test with reduce.fmaximum combined with maxnum (or similar) to show that it does not fold?

[anna]

LGTM w/ a comment. Thanks for catching this!

llvm/test/CodeGen/AArch64/double_reduct.ll

39–40

Since we no longer check any flags for doing the reassociation, can you pls add a test without the fast flag as well?

[dmgreen]

Thanks. I've updated the tests in 71ac2a8e23f1587ca6243378d663aedf10b46642. It removes the fast math flags from llvm.vector.reduce.fmin/max tests, and adds some tests with a combo of fminnum/fminimum. The SVE tests I have left out for the moment until D153288 goes in to enable some scalable-vector lowering.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (AF.isInfinity() || (Flags.hasNoInfs() && AF.isLargest())) {
       // maxnum(X, -inf) -> X if nnan
       // minimum(X, +inf) -> X
       // maximum(X, -inf) -> X
       if (IsMin != AF.isNegative() && (PropagatesNaN || Flags.hasNoNaNs()))
         return N->getOperand(0);
     }
   }
 
-  const TargetOptions &Options = DAG.getTarget().Options;
-  if ((Options.UnsafeFPMath && Options.NoSignedZerosFPMath) ||
-      (Flags.hasAllowReassociation() && Flags.hasNoSignedZeros()))
-    if (SDValue SD = reassociateReduction(IsMin ? ISD::VECREDUCE_FMIN
-                                                : ISD::VECREDUCE_FMAX,
-                                          Opc, SDLoc(N), VT, N0, N1, Flags))
-      return SD;
+  if (SDValue SD = reassociateReduction(
+          PropagatesNaN
+              ? (IsMin ? ISD::VECREDUCE_FMINIMUM : ISD::VECREDUCE_FMAXIMUM)
+              : (IsMin ? ISD::VECREDUCE_FMIN : ISD::VECREDUCE_FMAX),
 
   return SDValue();
 }
 
 SDValue DAGCombiner::visitFABS(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   EVT VT = N->getValueType(0);
 

llvm/test/CodeGen/AArch64/double_reduct.ll

; CHECK-NEXT:    ret
   %r = fmul fast float %r1, %r2
   ret float %r
 }
 
 define float @fmin_f32(<8 x float> %a, <4 x float> %b) {
 ; CHECK-LABEL: fmin_f32:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    fminnm v0.4s, v0.4s, v1.4s
-; CHECK-NEXT:    fminnmv s2, v2.4s
+; CHECK-NEXT:    fminnm v0.4s, v0.4s, v2.4s
 ; CHECK-NEXT:    fminnmv s0, v0.4s
-; CHECK-NEXT:    fminnm s0, s0, s2
 ; CHECK-NEXT:    ret
   %r1 = call float @llvm.vector.reduce.fmin.v8f32(<8 x float> %a)
   %r2 = call float @llvm.vector.reduce.fmin.v4f32(<4 x float> %b)
   %r = call float @llvm.minnum.f32(float %r1, float %r2)
   ret float %r
 }
 
 define float @fmax_f32(<8 x float> %a, <4 x float> %b) {
 ; CHECK-LABEL: fmax_f32:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    fmaxnm v0.4s, v0.4s, v1.4s
-; CHECK-NEXT:    fmaxnmv s2, v2.4s
+; CHECK-NEXT:    fmaxnm v0.4s, v0.4s, v2.4s
 ; CHECK-NEXT:    fmaxnmv s0, v0.4s
-; CHECK-NEXT:    fmaxnm s0, s0, s2
 ; CHECK-NEXT:    ret
   %r1 = call float @llvm.vector.reduce.fmax.v8f32(<8 x float> %a)
   %r2 = call float @llvm.vector.reduce.fmax.v4f32(<4 x float> %b)
   %r = call float @llvm.maxnum.f32(float %r1, float %r2)
   ret float %r
 }
 
 define float @fminimum_f32(<8 x float> %a, <4 x float> %b) {
 ; CHECK-LABEL: fminimum_f32:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    fmin v0.4s, v0.4s, v1.4s
-; CHECK-NEXT:    fminv s2, v2.4s
+; CHECK-NEXT:    fmin v0.4s, v0.4s, v2.4s
 ; CHECK-NEXT:    fminv s0, v0.4s
-; CHECK-NEXT:    fmin s0, s0, s2
 ; CHECK-NEXT:    ret
   %r1 = call float @llvm.vector.reduce.fminimum.v8f32(<8 x float> %a)
   %r2 = call float @llvm.vector.reduce.fminimum.v4f32(<4 x float> %b)
   %r = call float @llvm.minimum.f32(float %r1, float %r2)
   ret float %r
 }
 
 define float @fmaximum_f32(<8 x float> %a, <4 x float> %b) {
 ; CHECK-LABEL: fmaximum_f32:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    fmax v0.4s, v0.4s, v1.4s
-; CHECK-NEXT:    fmaxv s2, v2.4s
+; CHECK-NEXT:    fmax v0.4s, v0.4s, v2.4s
 ; CHECK-NEXT:    fmaxv s0, v0.4s
-; CHECK-NEXT:    fmax s0, s0, s2
 ; CHECK-NEXT:    ret
   %r1 = call float @llvm.vector.reduce.fmaximum.v8f32(<8 x float> %a)
   %r2 = call float @llvm.vector.reduce.fmaximum.v4f32(<4 x float> %b)
   %r = call float @llvm.maximum.f32(float %r1, float %r2)
   ret float %r
 }
 
 ; These next two tests have incorrect minnum/minimum combinations

 ; CHECK-NEXT:    fmaxnmv s2, v2.4s
 ; CHECK-NEXT:    fmaxnmv s0, v0.4s
 ; CHECK-NEXT:    fmax s0, s0, s2
 ; CHECK-NEXT:    ret
   %r1 = call float @llvm.vector.reduce.fmax.v8f32(<8 x float> %a)
   %r2 = call float @llvm.vector.reduce.fmax.v4f32(<4 x float> %b)
   %r = call float @llvm.maximum.f32(float %r1, float %r2)
   ret float %r
 }
 
 
 define i32 @add_i32(<8 x i32> %a, <4 x i32> %b) {
 ; CHECK-LABEL: add_i32:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    add v0.4s, v0.4s, v1.4s
 ; CHECK-NEXT:    add v0.4s, v0.4s, v2.4s
 ; CHECK-NEXT:    addv s0, v0.4s

llvm/test/CodeGen/AArch64/sve-doublereduct.ll

 ;  ret float %r
 ;}
 
 define float @fmin_f32(<vscale x 8 x float> %a, <vscale x 4 x float> %b) {
 ; CHECK-LABEL: fmin_f32:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ptrue p0.s
 ; CHECK-NEXT:    fminnm z0.s, p0/m, z0.s, z1.s
-; CHECK-NEXT:    fminnmv s2, p0, z2.s
+; CHECK-NEXT:    fminnm z0.s, p0/m, z0.s, z2.s
 ; CHECK-NEXT:    fminnmv s0, p0, z0.s
-; CHECK-NEXT:    fminnm s0, s0, s2
+; CHECK-NEXT:    // kill: def $s0 killed $s0 killed $z0
 ; CHECK-NEXT:    ret
   %r1 = call fast float @llvm.vector.reduce.fmin.nxv8f32(<vscale x 8 x float> %a)
   %r2 = call fast float @llvm.vector.reduce.fmin.nxv4f32(<vscale x 4 x float> %b)
   %r = call float @llvm.minnum.f32(float %r1, float %r2)
   ret float %r
 }
 
 define float @fmax_f32(<vscale x 8 x float> %a, <vscale x 4 x float> %b) {
 ; CHECK-LABEL: fmax_f32:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ptrue p0.s
 ; CHECK-NEXT:    fmaxnm z0.s, p0/m, z0.s, z1.s
-; CHECK-NEXT:    fmaxnmv s2, p0, z2.s
+; CHECK-NEXT:    fmaxnm z0.s, p0/m, z0.s, z2.s
 ; CHECK-NEXT:    fmaxnmv s0, p0, z0.s
-; CHECK-NEXT:    fmaxnm s0, s0, s2
+; CHECK-NEXT:    // kill: def $s0 killed $s0 killed $z0
 ; CHECK-NEXT:    ret
   %r1 = call fast float @llvm.vector.reduce.fmax.nxv8f32(<vscale x 8 x float> %a)
   %r2 = call fast float @llvm.vector.reduce.fmax.nxv4f32(<vscale x 4 x float> %b)
   %r = call float @llvm.maxnum.f32(float %r1, float %r2)
   ret float %r
 }
 
 

llvm/test/CodeGen/RISCV/double_reduct.ll

; CHECK-NEXT:    ret
   %r2 = call fast float @llvm.vector.reduce.fmul.f32.v4f32(float 1.0, <4 x float> %b)
   %r = fmul fast float %r1, %r2
   ret float %r
 }
 
 define float @fmin_f32(<4 x float> %a, <4 x float> %b) {
 ; CHECK-LABEL: fmin_f32:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    lui a0, %hi(.LCPI2_0)
-; CHECK-NEXT:    flw fa5, %lo(.LCPI2_0)(a0)
+; CHECK-NEXT:    lui a0, 523264
 ; CHECK-NEXT:    vsetivli zero, 4, e32, m1, ta, ma
-; CHECK-NEXT:    vfmv.s.f v10, fa5
+; CHECK-NEXT:    vmv.s.x v10, a0
+; CHECK-NEXT:    vfmin.vv v8, v8, v9
 ; CHECK-NEXT:    vfredmin.vs v8, v8, v10
-; CHECK-NEXT:    vfmv.f.s fa5, v8
-; CHECK-NEXT:    vfredmin.vs v8, v9, v10
-; CHECK-NEXT:    vfmv.f.s fa4, v8
-; CHECK-NEXT:    fmin.s fa0, fa5, fa4
+; CHECK-NEXT:    vfmv.f.s fa0, v8
 ; CHECK-NEXT:    ret
   %r1 = call fast float @llvm.vector.reduce.fmin.v4f32(<4 x float> %a)
   %r2 = call fast float @llvm.vector.reduce.fmin.v4f32(<4 x float> %b)
   %r = call float @llvm.minnum.f32(float %r1, float %r2)
   ret float %r
 }
 
 define float @fmax_f32(<4 x float> %a, <4 x float> %b) {
 ; CHECK-LABEL: fmax_f32:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    lui a0, %hi(.LCPI3_0)
-; CHECK-NEXT:    flw fa5, %lo(.LCPI3_0)(a0)
+; CHECK-NEXT:    lui a0, 1047552
 ; CHECK-NEXT:    vsetivli zero, 4, e32, m1, ta, ma
-; CHECK-NEXT:    vfmv.s.f v10, fa5
+; CHECK-NEXT:    vmv.s.x v10, a0
+; CHECK-NEXT:    vfmax.vv v8, v8, v9
 ; CHECK-NEXT:    vfredmax.vs v8, v8, v10
-; CHECK-NEXT:    vfmv.f.s fa5, v8
-; CHECK-NEXT:    vfredmax.vs v8, v9, v10
-; CHECK-NEXT:    vfmv.f.s fa4, v8
-; CHECK-NEXT:    fmax.s fa0, fa5, fa4
+; CHECK-NEXT:    vfmv.f.s fa0, v8
 ; CHECK-NEXT:    ret
   %r1 = call fast float @llvm.vector.reduce.fmax.v4f32(<4 x float> %a)
   %r2 = call fast float @llvm.vector.reduce.fmax.v4f32(<4 x float> %b)
   %r = call float @llvm.maxnum.f32(float %r1, float %r2)
   ret float %r
 }
 
 

llvm/test/CodeGen/Thumb2/mve-doublereduct.ll

; CHECK-NEXT:    bx lr
   %r = fmul fast float %r1, %r2
   ret float %r
 }
 
 define float @fmin_f32(<8 x float> %a, <4 x float> %b) {
 ; CHECK-LABEL: fmin_f32:
 ; CHECK:       @ %bb.0:
 ; CHECK-NEXT:    vminnm.f32 q0, q0, q1
-; CHECK-NEXT:    vminnm.f32 s4, s8, s9
+; CHECK-NEXT:    vminnm.f32 q0, q0, q2
 ; CHECK-NEXT:    vminnm.f32 s2, s2, s3
 ; CHECK-NEXT:    vminnm.f32 s0, s0, s1
 ; CHECK-NEXT:    vminnm.f32 s0, s0, s2
-; CHECK-NEXT:    vminnm.f32 s2, s10, s11
-; CHECK-NEXT:    vminnm.f32 s2, s4, s2
-; CHECK-NEXT:    vminnm.f32 s0, s0, s2
 ; CHECK-NEXT:    bx lr
   %r1 = call fast float @llvm.vector.reduce.fmin.v8f32(<8 x float> %a)
   %r2 = call fast float @llvm.vector.reduce.fmin.v4f32(<4 x float> %b)
   %r = call float @llvm.minnum.f32(float %r1, float %r2)
   ret float %r
 }
 
 define float @fmax_f32(<8 x float> %a, <4 x float> %b) {
 ; CHECK-LABEL: fmax_f32:
 ; CHECK:       @ %bb.0:
 ; CHECK-NEXT:    vmaxnm.f32 q0, q0, q1
-; CHECK-NEXT:    vmaxnm.f32 s4, s8, s9
+; CHECK-NEXT:    vmaxnm.f32 q0, q0, q2
 ; CHECK-NEXT:    vmaxnm.f32 s2, s2, s3
 ; CHECK-NEXT:    vmaxnm.f32 s0, s0, s1
 ; CHECK-NEXT:    vmaxnm.f32 s0, s0, s2
-; CHECK-NEXT:    vmaxnm.f32 s2, s10, s11
-; CHECK-NEXT:    vmaxnm.f32 s2, s4, s2
-; CHECK-NEXT:    vmaxnm.f32 s0, s0, s2
 ; CHECK-NEXT:    bx lr
   %r1 = call fast float @llvm.vector.reduce.fmax.v8f32(<8 x float> %a)
   %r2 = call fast float @llvm.vector.reduce.fmax.v4f32(<4 x float> %b)
   %r = call float @llvm.maxnum.f32(float %r1, float %r2)
   ret float %r
 }