[PATCH] D152370: [Intrinsic] Introduce reduction intrinsics for minimum/maximum

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/vecreduce-fmaximum.ll:3
+; 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
+
----------------
anna wrote:
> nikic wrote:
> > I'd suggest to copy over the full set of tests from llvm/test/CodeGen/AArch64/vecreduce-fmax-legalization.ll here, which should have test coverage for most of the legalizations.
> okay, I tried that and this test fails at AArch64ISel:
> ```
> define half @test_v4f16(<4 x half> %a) nounwind {
>   %b = call half @llvm.vector.reduce.fmaximum.v4f16(<4 x half> %a)
>   ret half %b
> }
> ```
> It passes with `+fullfp16` flag passed in. The reason looks like for AArch64 for f16 type, the default `setOperationAction` is PROMOTE for the baseOpCode being `FMAXIMUM`. When FP16 flag is passed in, `setOperationAction` is LEGAL, which satisfies the constraint `isOperationLegalOrCustom ` for vector types in `expandVecReduce`. I think this needs fixing in AArch64 backend lowering (by adding some custom lowering for FMAXIMUM?), but I do not know this enough to fix it. 
> 
> 
RootCaused the issue through debugging the selection DAG: we had a latent bug where we missed promotion for FPMINIMUM and FPMAXIMUM nodes in LegalizeDAG. I'll either fix it within this patch or add a separate one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152370/new/

https://reviews.llvm.org/D152370