[PATCH] D32563: Add LiveRangeShrink pass to shrink live range within BB.

[Geoff Berry via Phabricator via llvm-commits]

gberry added inline comments.


================
Comment at: lib/CodeGen/LiveRangeShrink.cpp:112
+      ++Next;
+      if (MI->isPHI() || MI->isCompare() || MI->isDebugValue())
+        continue;
----------------
danielcdh wrote:
> gberry wrote:
> > Why are you skipping compares?
> In many architectures, cmp should stay together with jmp instruction for performance purpose. We do not want to break this in this pass.
For targets that don't have allocatable flag regsiters, your later check for physical regs should prevent moving these compares (and any other non-compare flag setting instructions).  For targets that keep the results of compares in general purpose registers, this seems like a missed optimization.  Also, if it is important that the compare be just before the branch, the scheduler should be taking care of that.

If you don't agree that this check should be removed can you put a comment explaining why this check for compares is being done?


================
Comment at: lib/CodeGen/LiveRangeShrink.cpp:153
+        // Do not move the instruction if it def/uses a physical register,
+        // unless it is a constant physicall register.
+        if (TargetRegisterInfo::isPhysicalRegister(MO.getReg()) &&
----------------
Typo: 'physicall' -> 'physical'


https://reviews.llvm.org/D32563