[PATCH] D48907: [ARM] Treat cmn immediates as legal in isLegalICmpImmediate.

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:3861
+    if (!isLegalICmpImmediate((int32_t)C) ||
+        (C == -1U && (CC == ISD::SETLE || CC == ISD::SETGT))) {
+      // Constant does not fit, try adjusting it by one.
----------------
efriedma wrote:
> samparker wrote:
> > I don't understand what this is trying to do, why the special case? Surely the fix in isLegalICmpImmediate is enough?
> I probably should have added a comment to this.
> 
> The special case for "-1" isn't strictly necessary; it just prefers "cmp r0, #0" over "cmn r0, #1" like the old code does, to reduce the number of regression test changes.  Maybe that doesn't matter?
It really confused me, so I feel that changing the tests is a necessary chore to make the code easier to understand.


Repository:
  rL LLVM

https://reviews.llvm.org/D48907