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

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Yeah, nice. This is a lot easier to follow.



================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:12112
+  SDValue FalseVal;
+  if (N->getOpcode() == ISD::SELECT) {
+    SetCC = N->getOperand(0);
----------------
I think it needs to check that operand is a Setcc too, to be sure it's not something strange.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:12185
+  if (VectorScalarType != MVT::i32)
+    LHS = DCI.DAG.getNode(IsUnsigned ? ISD::ZERO_EXTEND : ISD::SIGN_EXTEND, dl,
+                          MVT::i32, LHS, DCI.DAG.getValueType(LeftType));
----------------
The top bits of this are not read by the instruction. Can we change it to an ANY_EXTEND?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:12186
+    LHS = DCI.DAG.getNode(IsUnsigned ? ISD::ZERO_EXTEND : ISD::SIGN_EXTEND, dl,
+                          MVT::i32, LHS, DCI.DAG.getValueType(LeftType));
+
----------------
I think the last operand isn't needed here.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:16157
+  case ISD::SELECT:
+    if (Subtarget->hasMVEIntegerOps())
+      return PerformSELECTCombine(N, DCI);
----------------
Move the hasMVEIntegerOps check into the function? So it can look like the rest.


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

https://reviews.llvm.org/D87836