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

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:11813
 
+static bool isValidMVECond(unsigned CC) {
+  switch (CC) {
----------------
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?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:11842
+    CondCode0 = (ARMCC::CondCodes)cast<const ConstantSDNode>(N0->getOperand(2))
+                    ->getZExtValue();
+  if (N0->getOpcode() == ARMISD::VCMPZ)
----------------
This doesn't look like the correct formatting.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:11843
+                    ->getZExtValue();
+  if (N0->getOpcode() == ARMISD::VCMPZ)
+    CondCode0 = (ARMCC::CondCodes)cast<const ConstantSDNode>(N0->getOperand(1))
----------------
else if


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:11849
+                    ->getZExtValue();
+  if (N1->getOpcode() == ARMISD::VCMPZ)
+    CondCode1 = (ARMCC::CondCodes)cast<const ConstantSDNode>(N1->getOperand(1))
----------------
else if


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:11862
 
   std::vector<SDValue> Ops0;
+  Ops0.push_back(N0->getOperand(0));
----------------
I know this isn't you, but these vectors should be SmallVectors instead.


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

https://reviews.llvm.org/D65072