[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
+
----------------
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. 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152370