[PATCH] D42263: [ARM] Fix perf regression in compare optimization.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:2743
     // could still potentially be SubAdd.
     --E;
   }
----------------
Is there some reason to expect that --E is a valid, given E could be the first instruction in its basic block?


================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:2750
   --I;
   for (; I != E; --I) {
     const MachineInstr &Instr = *I;
----------------
This loop is confusing.  The initial value of I is the compare.  You're looping until it equals E... but you also have an early exit if you hit the beginning of the basic block.

Could you fix the loop to explicitly compute its exit condition in some more intuitive way?  Maybe you could express this more clearly using MachineBasicBlock::reverse_iterator.  And ideally you could use range-based for loop with "make_range" rather an explicit increment/decrement.


https://reviews.llvm.org/D42263