[llvm] r217881 - ARM load/store optimizer: Don't materialize a new base register with

[Moritz Roth]

Author: mroth
Date: Tue Sep 16 11:25:07 2014
New Revision: 217881

URL: http://llvm.org/viewvc/llvm-project?rev=217881&view=rev
Log:
ARM load/store optimizer: Don't materialize a new base register with
ADDS/SUBS unless it's safe to clobber the condition flags.

If the merged instructions are in a range where the CPSR is live,
e.g. between a CMP -> Bcc, we can't safely materialize a new base
register.

This problem is quite rare, I couldn't come up with a test case and I've
never actually seen this happen in the tests I'm running - there is a
potential trigger for this in LNT/oggenc (spills being inserted between
a CMP/Bcc), but at the moment this isn't being merged. I'll try to
reduce that into a small test case once I've committed my upcoming patch
to make merging less conservative.

Modified:
    llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp

Modified: llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp?rev=217881&r1=217880&r2=217881&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMLoadStoreOptimizer.cpp Tue Sep 16 11:25:07 2014
@@ -323,6 +323,12 @@ ARMLoadStoreOpt::MergeOps(MachineBasicBl
   if (NumRegs <= 1)
     return false;
 
+  // For Thumb1 targets, it might be necessary to clobber the CPSR to merge.
+  // Compute liveness information for that register to make the decision.
+  bool SafeToClobberCPSR = !isThumb1 ||
+    (MBB.computeRegisterLiveness(TRI, ARM::CPSR, std::prev(MBBI), 15) ==
+     MachineBasicBlock::LQR_Dead);
+
   ARM_AM::AMSubMode Mode = ARM_AM::ia;
   // VFP and Thumb2 do not support IB or DA modes. Thumb1 only supports IA.
   bool isNotVFP = isi32Load(Opcode) || isi32Store(Opcode);
@@ -346,6 +352,11 @@ ARMLoadStoreOpt::MergeOps(MachineBasicBl
     if (NumRegs <= 2)
       return false;
 
+    // On Thumb1, it's not worth materializing a new base register without
+    // clobbering the CPSR (i.e. not using ADDS/SUBS).
+    if (!SafeToClobberCPSR)
+      return false;
+
     unsigned NewBase;
     if (isi32Load(Opcode)) {
       // If it is a load, then just use one of the destination register to
@@ -377,10 +388,10 @@ ARMLoadStoreOpt::MergeOps(MachineBasicBl
 
     if (isThumb1) {
       // Thumb1: depending on immediate size, use either
-      //   ADD NewBase, Base, #imm3
+      //   ADDS NewBase, Base, #imm3
       // or
-      //   MOV NewBase, Base
-      //   ADD NewBase, #imm8.
+      //   MOV  NewBase, Base
+      //   ADDS NewBase, #imm8.
       if (Base != NewBase && Offset >= 8) {
         // Need to insert a MOV to the new base first.
         BuildMI(MBB, MBBI, dl, TII->get(ARM::tMOVr), NewBase)
@@ -390,7 +401,7 @@ ARMLoadStoreOpt::MergeOps(MachineBasicBl
         Base = NewBase;
         BaseKill = false;
       }
-      AddDefaultT1CC(BuildMI(MBB, MBBI, dl, TII->get(BaseOpc), NewBase))
+      AddDefaultT1CC(BuildMI(MBB, MBBI, dl, TII->get(BaseOpc), NewBase), true)
         .addReg(Base, getKillRegState(BaseKill)).addImm(Offset)
         .addImm(Pred).addReg(PredReg);
     } else {