[PATCH] D87836: [ARM]Fold select_cc(vecreduce_[u|s][min|max], x) into VMINV or VMAXV

Mon Sep 21 07:34:13 PDT 2020

dmgreen added inline comments.

================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:5168
+           CC == ISD::SETGT)
+    Opcode = ARMISD::VMAXVs;
+
----------------
else return false;
Then it doesn't need the extra indenting.

================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:5186
+    // so check the first operand in those cases
+    switch (LHS->getOpcode()) {
+    case ISD::AND:
----------------
This looks like it need to be a bit stricter still. The AND needs to go with a umin/umax, and the sign extend with smin/smax (they shouldn't be the other way around). The AND needs a mask of the right size (it'll be 255 for i8 for example, which is the same as a "zero extend inreg"). The sign extend will have a type as the second argument, which should be the same as the vecreduce's scalar type.

Some of this might be difficult to get to come up in practice, but we should make sure it won't be subtly wrong and cause bugs.

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

https://reviews.llvm.org/D87836