[Patch] [ARM] Fix bug in tryFoldSPUpdateIntoPushPop

[Duncan P. N. Exon Smith]

On 2014 Mar 13, at 16:14, Weiming Zhao <weimingz at codeaurora.org> wrote:

> Hi,
>  
> This patch fixes the issue in http://llvm.org/bugs/show_bug.cgi?id=19136
> There might be two ways to do it: One is to fix tryFoldSPUpdateIntoPushPop() to let it check livness of subregs.
> The other ways is to fix MBB->computeRegisterLiveness().
>  
> The patch uses the first way.
>  
> Please help to review.

diff --git a/lib/Target/ARM/ARMBaseInstrInfo.cpp b/lib/Target/ARM/ARMBaseInstrInfo.cpp
index 1aa146d..8a30d3c 100644
--- a/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ b/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -2160,9 +2160,14 @@ bool llvm::tryFoldSPUpdateIntoPushPop(MachineFunction &MF,
     // registers live within the function we might clobber a return value
     // register; the other way a register can be live here is if it's
     // callee-saved.
-    if (isCalleeSavedRegister(CurReg, CSRegs) ||
-        MBB->computeRegisterLiveness(TRI, CurReg, MI) !=
-            MachineBasicBlock::LQR_Dead) {
+    bool IsRegLive = false;
+    for (MCSubRegIterator Subreg(CurReg, TRI, /* IncludeSelf */true);
+         Subreg.isValid() && !IsRegLive; ++Subreg)
+      if (MBB->computeRegisterLiveness(TRI, *Subreg, MI) !=
+           MachineBasicBlock::LQR_Dead)
+        IsRegLive = true;
+
+    if (isCalleeSavedRegister(CurReg, CSRegs) || IsRegLive) {
       // VFP pops don't allow holes in the register list, so any skip is fatal
       // for our transformation. GPR pops do, so we should just keep looking.
       if (IsVFPPushPop)

I’m not an expert here, but your change looks unnecessarily bad for
compile time.  You now call computeRegisterLiveness() even when you don't
use the result.  You can refactor by moving the calculation into a static
function:

    if (isCalleeSavedRegister(CurReg, CSRegs) ||
        isAnySubRegLive(MBB, CurReg, TRI)) {