[PATCH] D66483: [ARM] Generate 8.1-m CSINC, CSNEG and CSINV instructions.

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:46
 
+extern cl::opt<bool> ARMGenerateCSEL;
+
----------------
Freaky way to control code gen! Do we really need this if code size is better and performance is on par or better..?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:4829
+      LHS.getValueType() == MVT::i32 && RHS.getValueType() == MVT::i32) {
+    if (CFVal && CTVal) {
+      unsigned TVal = CTVal->getZExtValue();
----------------
Reduce the nesting here.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:4853
+        // condition to get there.
+        if (FVal == 0 && Opcode != ARMISD::CSINC) {
+          std::swap(TrueVal, FalseVal);
----------------
It's not obvious to me why CSINC is excluded here, could you add a comment?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:4854
+        if (FVal == 0 && Opcode != ARMISD::CSINC) {
+          std::swap(TrueVal, FalseVal);
+          std::swap(TVal, FVal);
----------------
But FalseVal = TrueVal?


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

https://reviews.llvm.org/D66483