[PATCH] D42312: [ARM] Cleanup part of ARMBaseInstrInfo::optimizeCompareInstr (NFCI).

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:2754
+    if ((Instr.modifiesRegister(ARM::CPSR, TRI) ||
+         Instr.readsRegister(ARM::CPSR, TRI)) && I != E)
       // This instruction modifies or uses CPSR after the one we want to
----------------
jgalenson wrote:
> efriedma wrote:
> > Maybe it would be more clear to write "if (I == E) break;" before this check?
> Sure, I can do that, but it does re-introduce an early exit into the loop, which I remember you didn't like before.
> 
> This, by the way, is exactly why I considered splitting this into two loops, which is another possibility, although it seemed a bit more complicated (and less efficient) to me.
An early break isn't always ideal, but I think it makes sense here; it has a similar meaning to the other break in the `if (isRedundantFlagInstr...` block.


https://reviews.llvm.org/D42312