[PATCH] D146404: Improve min/max vector reductions on arm
Caleb Zulawski via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 20 20:14:50 PDT 2023
calebzulawski marked 7 inline comments as done.
calebzulawski added a comment.
Thanks for the great review! I haven't touched codegen before, this helped quite a bit.
================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:10288
+static SDValue LowerVecReduceMinMax(SDValue Op, SelectionDAG &DAG,
+ const ARMSubtarget *ST) {
----------------
dmgreen wrote:
> Is there a missing chunk where this gets called from LowerOperation?
Whoops, something was a little wonky with the last patch
================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:10335
+ if (EltVT != Op->getValueType(0))
+ Res = DAG.getNode(ISD::ANY_EXTEND, dl, Op->getValueType(0), Res);
+ return Res;
----------------
dmgreen wrote:
> Should this do SIGN_EXTEND or ZERO_EXTEND, depending on the sign of the min/max?
Yes, I think so. I copied it from the MVE vector reduce, but maybe that's doing something different.
================
Comment at: llvm/test/CodeGen/ARM/vecreduce-minmax.ll:4
+
+define i8 @test_umin_v8i8(<8 x i8> %x) {
+; CHECK-LABEL: test_umin_v8i8:
----------------
dmgreen wrote:
> Can you add some tests for <16 x i8> and other 128bit vector types. They will be quite common.
Aha, this also had the desired effect on much longer vectors, since they appear to break into 128-bit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146404/new/
https://reviews.llvm.org/D146404
More information about the llvm-commits
mailing list