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

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/vecreduce-fmax-legalization.ll:3
 ; 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,+fullfp16 | FileCheck %s --check-prefix=FP16
 
----------------
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.


================
Comment at: llvm/test/CodeGen/AArch64/vecreduce-fmin-legalization.ll:68
+; CHECK-NEXT:    ret
+; FP16-LABEL: test_v4f16:
+; FP16:       // %bb.0:
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97840