[PATCH] D39004: [ARM] Swap cmp operands for automatic shifts.

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:3869
+  if (SwapOperands) {
+    CondCode = ARMCC::getOppositeCondition(CondCode);
+    std::swap(LHS, RHS);
----------------
efriedma wrote:
> Wait a sec.
> 
> In IR, we have two related, but distinct methods on comparisons: getInversePredicate, and getSwappedPredicate.  Similarly, in SelectionDAG we have getSetCCInverse and getSetCCSwappedOperands. ARMCC::getOppositeCondition is equivalent to getInversePredicate, but I think you want getSwappedPredicate here?
> 
> And given that oversight, I have to ask: how did you test this?
Very badly it seems.

Thanks for catching this and sorry for making the dumb mistake. I'll use the SwappedOperands function and add some vital checks to the tests.

Thanks again.


https://reviews.llvm.org/D39004