[AArch64] Legalize horizontal fmax/fmin reductions on f16 vectors

[LemonBoy]

Summary

Expand the horizontal reduction during the instruction selection phase, but only if the target doesn't support the full fp16 instruction set.

Fixes https://bugs.llvm.org/show_bug.cgi?id=49401

[david-arm]

llvm/test/CodeGen/AArch64/vecreduce-fmax-legalization.ll

3

Hi, instead of having two RUN lines here would it be better to just use function attributes, i.e. for test_v4f16 have a function attribute that adds "fullfp16" support? That way you only need one RUN line and can avoid all the additional FP16 check lines too, since most of them seem to be the same as the first RUN line.

llvm/test/CodeGen/AArch64/vecreduce-fmin-legalization.ll

69

It looks like you've added FP16 checks here without the additional RUN line. Similar to my comment in the fmax case perhaps you can just use function attributes instead?

[fhahn]

llvm/test/CodeGen/AArch64/vecreduce-fmax-legalization.ll

3

You could also use multiple check prefixes to avoid having repeated check for both RUN lines, if they are equal, e.g. FileCheck %s --check-prefix=CHECK --check-prefix=NOFP16 ..., FileCheck %s --check-prefix=CHECK --check-prefix=FP16 ...

With that, we should only need separate FP16/NOFP16 check lines for functions where there is a difference.

[LemonBoy]

llvm/test/CodeGen/AArch64/vecreduce-fmax-legalization.ll

3

The main idea is to check the expanded reduction is emitted when fullfp16 is not specified, I guess the FP16 check can be removed altogether if we don't care about checking the case where the intrinsic can be lowered.

llvm/test/CodeGen/AArch64/vecreduce-fmin-legalization.ll

69

Good catch, I keep forgetting pieces when I git add.

[david-arm]

llvm/test/CodeGen/AArch64/vecreduce-fmax-legalization.ll

3

Hi @fhahn sure that would work too and it's a good point - I was just thinking that fewer RUN lines meant faster test suites that's all.

[nikic]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

10489

Might be better to not mark them Custom in the first place? https://github.com/llvm/llvm-project/blob/3b47bd32f9df4a57db98db5f35e680c7bd9fde3e/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp#L1019-L1023

[fhahn]

llvm/test/CodeGen/AArch64/vecreduce-fmax-legalization.ll

3

I was just thinking that fewer RUN lines meant faster test suites that's all.

That's also a good point. I'm not sure how things will shake out with more tests for half; I think we should have fp16 tests both with and without fullfp16, as long we have that and can avoid must of the redundant check lines, I am happy whichever way we go :)

49

can you also throw in a test with vectors that are not directly legal, regardless of fullfp16?

[LemonBoy]

Don't mark the fmin/fmax ops as legal if fullfp16 is not available.
Update test cases.

[LemonBoy]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

10489

Indeed, fixed.
Thanks for the suggestion!

llvm/test/CodeGen/AArch64/vecreduce-fmax-legalization.ll

49

That's hitting a different problem in the legalization step, I'll try to address that in a separate patch.

[aemerson]

LGTM.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

for (MVT VT : { MVT::v8i8, MVT::v4i16, MVT::v2i32,
       setOperationAction(ISD::UADDSAT, VT, Legal);
       setOperationAction(ISD::SSUBSAT, VT, Legal);
       setOperationAction(ISD::USUBSAT, VT, Legal);
     }
 
     // Vector reductions
     for (MVT VT : { MVT::v4f16, MVT::v2f32,
                     MVT::v8f16, MVT::v4f32, MVT::v2f64 }) {
+      if (VT.getVectorElementType() != MVT::f16 || Subtarget->hasFullFP16()) {
         setOperationAction(ISD::VECREDUCE_FMAX, VT, Custom);
         setOperationAction(ISD::VECREDUCE_FMIN, VT, Custom);
 
-      if (VT.getVectorElementType() != MVT::f16 || Subtarget->hasFullFP16())
         setOperationAction(ISD::VECREDUCE_FADD, VT, Legal);
       }
+    }
     for (MVT VT : { MVT::v8i8, MVT::v4i16, MVT::v2i32,
                     MVT::v16i8, MVT::v8i16, MVT::v4i32 }) {
       setOperationAction(ISD::VECREDUCE_ADD, VT, Custom);
       setOperationAction(ISD::VECREDUCE_SMAX, VT, Custom);
       setOperationAction(ISD::VECREDUCE_SMIN, VT, Custom);
       setOperationAction(ISD::VECREDUCE_UMAX, VT, Custom);
       setOperationAction(ISD::VECREDUCE_UMIN, VT, Custom);
     }
case ISD::VECREDUCE_SMIN:
     return getReductionSDNode(AArch64ISD::SMINV, dl, Op, DAG);
   case ISD::VECREDUCE_UMAX:
     return getReductionSDNode(AArch64ISD::UMAXV, dl, Op, DAG);
   case ISD::VECREDUCE_UMIN:
     return getReductionSDNode(AArch64ISD::UMINV, dl, Op, DAG);
   case ISD::VECREDUCE_FMAX: {
     return DAG.getNode(
         ISD::INTRINSIC_WO_CHAIN, dl, Op.getValueType(),
         DAG.getConstant(Intrinsic::aarch64_neon_fmaxnmv, dl, MVT::i32),
         Src);
   }
   case ISD::VECREDUCE_FMIN: {
     return DAG.getNode(
         ISD::INTRINSIC_WO_CHAIN, dl, Op.getValueType(),
         DAG.getConstant(Intrinsic::aarch64_neon_fminnmv, dl, MVT::i32),
         Src);
   }

llvm/test/CodeGen/AArch64/vecreduce-fmax-legalization.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=aarch64-none-linux-gnu -mattr=+neon | FileCheck %s --check-prefix=CHECK
+; RUN: llc < %s -mtriple=aarch64-none-linux-gnu -mattr=+neon | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NOFP
+; RUN: llc < %s -mtriple=aarch64-none-linux-gnu -mattr=+neon,+fullfp16 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-FP
 
 declare half @llvm.vector.reduce.fmax.v1f16(<1 x half> %a)
 declare float @llvm.vector.reduce.fmax.v1f32(<1 x float> %a)
 declare double @llvm.vector.reduce.fmax.v1f64(<1 x double> %a)
 declare fp128 @llvm.vector.reduce.fmax.v1f128(<1 x fp128> %a)
 
+declare half @llvm.vector.reduce.fmax.v4f16(<4 x half> %a)
 declare float @llvm.vector.reduce.fmax.v3f32(<3 x float> %a)
 declare fp128 @llvm.vector.reduce.fmax.v2f128(<2 x fp128> %a)
 declare float @llvm.vector.reduce.fmax.v16f32(<16 x float> %a)
 
 define half @test_v1f16(<1 x half> %a) nounwind {
 ; CHECK-LABEL: test_v1f16:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ret

 define fp128 @test_v1f128(<1 x fp128> %a) nounwind {
 ; CHECK-LABEL: test_v1f128:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ret
   %b = call nnan fp128 @llvm.vector.reduce.fmax.v1f128(<1 x fp128> %a)
   ret fp128 %b
 }
 
+define half @test_v4f16(<4 x half> %a) nounwind {
+; CHECK-NOFP-LABEL: test_v4f16:
+; CHECK-NOFP:       // %bb.0:
+; CHECK-NOFP-NEXT:    // kill: def $d0 killed $d0 def $q0
+; CHECK-NOFP-NEXT:    mov h3, v0.h[1]
+; CHECK-NOFP-NEXT:    mov h1, v0.h[3]
+; CHECK-NOFP-NEXT:    mov h2, v0.h[2]
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fcvt s3, h3
+; CHECK-NOFP-NEXT:    fmaxnm s0, s0, s3
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    fcvt s2, h2
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fmaxnm s0, s0, s2
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fcvt s1, h1
+; CHECK-NOFP-NEXT:    fmaxnm s0, s0, s1
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    ret
+;
+; CHECK-FP-LABEL: test_v4f16:
+; CHECK-FP:       // %bb.0:
+; CHECK-FP-NEXT:    fmaxnmv h0, v0.4h
+; CHECK-FP-NEXT:    ret
+  %b = call nnan half @llvm.vector.reduce.fmax.v4f16(<4 x half> %a)
+  ret half %b
+}
+
+define half @test_v4f16_ninf(<4 x half> %a) nounwind {
+; CHECK-NOFP-LABEL: test_v4f16_ninf:
+; CHECK-NOFP:       // %bb.0:
+; CHECK-NOFP-NEXT:    // kill: def $d0 killed $d0 def $q0
+; CHECK-NOFP-NEXT:    mov h3, v0.h[1]
+; CHECK-NOFP-NEXT:    mov h1, v0.h[3]
+; CHECK-NOFP-NEXT:    mov h2, v0.h[2]
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fcvt s3, h3
+; CHECK-NOFP-NEXT:    fmaxnm s0, s0, s3
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    fcvt s2, h2
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fmaxnm s0, s0, s2
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fcvt s1, h1
+; CHECK-NOFP-NEXT:    fmaxnm s0, s0, s1
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    ret
+;
+; CHECK-FP-LABEL: test_v4f16_ninf:
+; CHECK-FP:       // %bb.0:
+; CHECK-FP-NEXT:    fmaxnmv h0, v0.4h
+; CHECK-FP-NEXT:    ret
+  %b = call nnan ninf half @llvm.vector.reduce.fmax.v4f16(<4 x half> %a)
+  ret half %b
+}
+
 define float @test_v3f32(<3 x float> %a) nounwind {
 ; CHECK-LABEL: test_v3f32:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    mov w8, #-8388608
 ; CHECK-NEXT:    fmov s1, w8
 ; CHECK-NEXT:    mov v0.s[3], v1.s[0]
 ; CHECK-NEXT:    fmaxnmv s0, v0.4s
 ; CHECK-NEXT:    ret

llvm/test/CodeGen/AArch64/vecreduce-fmin-legalization.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=aarch64-none-linux-gnu -mattr=+neon | FileCheck %s --check-prefix=CHECK
+; RUN: llc < %s -mtriple=aarch64-none-linux-gnu -mattr=+neon | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NOFP
+; RUN: llc < %s -mtriple=aarch64-none-linux-gnu -mattr=+neon,+fullfp16 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-FP
 
 declare half @llvm.vector.reduce.fmin.v1f16(<1 x half> %a)
 declare float @llvm.vector.reduce.fmin.v1f32(<1 x float> %a)
 declare double @llvm.vector.reduce.fmin.v1f64(<1 x double> %a)
 declare fp128 @llvm.vector.reduce.fmin.v1f128(<1 x fp128> %a)
 
+declare half @llvm.vector.reduce.fmin.v4f16(<4 x half> %a)
 declare float @llvm.vector.reduce.fmin.v3f32(<3 x float> %a)
 declare fp128 @llvm.vector.reduce.fmin.v2f128(<2 x fp128> %a)
 declare float @llvm.vector.reduce.fmin.v16f32(<16 x float> %a)
 
 define half @test_v1f16(<1 x half> %a) nounwind {
 ; CHECK-LABEL: test_v1f16:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ret

 define fp128 @test_v1f128(<1 x fp128> %a) nounwind {
 ; CHECK-LABEL: test_v1f128:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ret
   %b = call nnan fp128 @llvm.vector.reduce.fmin.v1f128(<1 x fp128> %a)
   ret fp128 %b
 }
 
+define half @test_v4f16(<4 x half> %a) nounwind {
+; CHECK-NOFP-LABEL: test_v4f16:
+; CHECK-NOFP:       // %bb.0:
+; CHECK-NOFP-NEXT:    // kill: def $d0 killed $d0 def $q0
+; CHECK-NOFP-NEXT:    mov h3, v0.h[1]
+; CHECK-NOFP-NEXT:    mov h1, v0.h[3]
+; CHECK-NOFP-NEXT:    mov h2, v0.h[2]
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fcvt s3, h3
+; CHECK-NOFP-NEXT:    fminnm s0, s0, s3
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    fcvt s2, h2
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fminnm s0, s0, s2
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fcvt s1, h1
+; CHECK-NOFP-NEXT:    fminnm s0, s0, s1
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    ret
+;
+; CHECK-FP-LABEL: test_v4f16:
+; CHECK-FP:       // %bb.0:
+; CHECK-FP-NEXT:    fminnmv h0, v0.4h
+; CHECK-FP-NEXT:    ret
+  %b = call nnan half @llvm.vector.reduce.fmin.v4f16(<4 x half> %a)
+  ret half %b
+}
+
+define half @test_v4f16_ninf(<4 x half> %a) nounwind {
+; CHECK-NOFP-LABEL: test_v4f16_ninf:
+; CHECK-NOFP:       // %bb.0:
+; CHECK-NOFP-NEXT:    // kill: def $d0 killed $d0 def $q0
+; CHECK-NOFP-NEXT:    mov h3, v0.h[1]
+; CHECK-NOFP-NEXT:    mov h1, v0.h[3]
+; CHECK-NOFP-NEXT:    mov h2, v0.h[2]
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fcvt s3, h3
+; CHECK-NOFP-NEXT:    fminnm s0, s0, s3
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    fcvt s2, h2
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fminnm s0, s0, s2
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    fcvt s0, h0
+; CHECK-NOFP-NEXT:    fcvt s1, h1
+; CHECK-NOFP-NEXT:    fminnm s0, s0, s1
+; CHECK-NOFP-NEXT:    fcvt h0, s0
+; CHECK-NOFP-NEXT:    ret
+;
+; CHECK-FP-LABEL: test_v4f16_ninf:
+; CHECK-FP:       // %bb.0:
+; CHECK-FP-NEXT:    fminnmv h0, v0.4h
+; CHECK-FP-NEXT:    ret
+  %b = call nnan ninf half @llvm.vector.reduce.fmin.v4f16(<4 x half> %a)
+  ret half %b
+}
+
 define float @test_v3f32(<3 x float> %a) nounwind {
 ; CHECK-LABEL: test_v3f32:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    mov w8, #2139095040
 ; CHECK-NEXT:    fmov s1, w8
 ; CHECK-NEXT:    mov v0.s[3], v1.s[0]
 ; CHECK-NEXT:    fminnmv s0, v0.4s
 ; CHECK-NEXT:    ret