[PATCH] D65072: [ARM] Rewrite how VCMP are lowered, using a single node

[Dave Green via Phabricator via llvm-commits]

dmgreen marked 6 inline comments as done.
dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:11813
 
+static bool isValidMVECond(unsigned CC) {
+  switch (CC) {
----------------
samparker wrote:
> There seem to be a load of different encodings for VCMP, do we have to think about that here at all when deciding whether a CC is valid?
The VCMPs are:
i: eq ne
s: gt, lt, ge, le
u: cs, hi
and the same set can be used for qq and qr compares.

Except in floating point, where there are only the first 6. The other 2 do not come up in PerformORCombine_i1 below, because we never have the opposite to invert them from. I've added a IsFloatingPoint argument for any other uses this function might get in the future.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:11842
+    CondCode0 = (ARMCC::CondCodes)cast<const ConstantSDNode>(N0->getOperand(2))
+                    ->getZExtValue();
+  if (N0->getOpcode() == ARMISD::VCMPZ)
----------------
samparker wrote:
> This doesn't look like the correct formatting.
Apparently this is. Too many long casts I guess, it splits it where it can.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:11862
 
   std::vector<SDValue> Ops0;
+  Ops0.push_back(N0->getOperand(0));
----------------
samparker wrote:
> I know this isn't you, but these vectors should be SmallVectors instead.
Me in an earlier patch :) I'll change it there before committing.


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

https://reviews.llvm.org/D65072