[llvm-commits] CVS: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp

[Evan Cheng]

Changes in directory llvm/lib/Target/ARM:

ARMConstantIslandPass.cpp updated: 1.19 -> 1.20
---
Log message:

- Off by one bugs in maximum displacement calculation / testing.
- In thumb mode, a new constpool island BB size should be 4 + 2 to
  compensate for the potential padding due to alignment requirement.


---
Diffs of the changes:  (+28 -15)

 ARMConstantIslandPass.cpp |   43 ++++++++++++++++++++++++++++---------------
 1 files changed, 28 insertions(+), 15 deletions(-)


Index: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
diff -u llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1.19 llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1.20
--- llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1.19	Wed Jan 31 19:09:47 2007
+++ llvm/lib/Target/ARM/ARMConstantIslandPass.cpp	Thu Feb  1 04:16:15 2007
@@ -281,8 +281,10 @@
           Scale = 2;
           break;
         }
-        unsigned MaxDisp = (1 << (Bits-1)) * Scale;
-        ImmBranches.push_back(ImmBranch(I, MaxDisp, isCond, UOpc));
+
+        // Record this immediate branch.
+        unsigned MaxOffs = (1 << (Bits-1)) * Scale;
+        ImmBranches.push_back(ImmBranch(I, MaxOffs, isCond, UOpc));
       }
 
       if (Opc == ARM::tPUSH || Opc == ARM::tPOP_RET)
@@ -293,9 +295,10 @@
         if (I->getOperand(op).isConstantPoolIndex()) {
           // We found one.  The addressing mode tells us the max displacement
           // from the PC that this instruction permits.
-          unsigned MaxOffs = 0;
           
           // Basic size info comes from the TSFlags field.
+          unsigned Bits = 0;
+          unsigned Scale = 1;
           unsigned TSFlags = I->getInstrDescriptor()->TSFlags;
           switch (TSFlags & ARMII::AddrModeMask) {
           default: 
@@ -304,34 +307,42 @@
               continue;
             assert(0 && "Unknown addressing mode for CP reference!");
           case ARMII::AddrMode1: // AM1: 8 bits << 2
-            MaxOffs = 1 << (8+2);   // Taking the address of a CP entry.
+            Bits = 8;
+            Scale = 4;  // Taking the address of a CP entry.
             break;
           case ARMII::AddrMode2:
-            MaxOffs = 1 << 12;   // +-offset_12
+            Bits = 12;
+            Scale = 2;  // +-offset_12
             break;
           case ARMII::AddrMode3:
-            MaxOffs = 1 << 8;   // +-offset_8
+            Bits = 8;
+            Scale = 2;  // +-offset_8
             break;
             // addrmode4 has no immediate offset.
           case ARMII::AddrMode5:
-            MaxOffs = 1 << (8+2);   // +-(offset_8*4)
+            Bits = 8;
+            Scale = 4;  // +-(offset_8*4)
             break;
           case ARMII::AddrModeT1:
-            MaxOffs = 1 << 5;
+            Bits = 5;  // +offset_5
             break;
           case ARMII::AddrModeT2:
-            MaxOffs = 1 << (5+1);
+            Bits = 5;
+            Scale = 2;  // +(offset_5*2)
             break;
           case ARMII::AddrModeT4:
-            MaxOffs = 1 << (5+2);
+            Bits = 5;
+            Scale = 4;  // +(offset_5*4)
             break;
           case ARMII::AddrModeTs:
-            MaxOffs = 1 << (8+2);
+            Bits = 8;
+            Scale = 4;  // +(offset_8*4)
             break;
           }
-          
+
           // Remember that this is a user of a CP entry.
           MachineInstr *CPEMI =CPEMIs[I->getOperand(op).getConstantPoolIndex()];
+          unsigned MaxOffs = (1 << (Bits-1)) * Scale;          
           CPUsers.push_back(CPUser(I, CPEMI, MaxOffs));
           
           // Instructions can only use one CP entry, don't bother scanning the
@@ -514,12 +525,12 @@
   // TODO: Search for the best place to split the code.  In practice, using
   // loop nesting information to insert these guys outside of loops would be
   // sufficient.    
+  bool isThumb = AFI->isThumbFunction();
   if (&UserMBB->back() == UserMI) {
     assert(BBHasFallthrough(UserMBB) && "Expected a fallthrough BB!");
     NewMBB = next(MachineFunction::iterator(UserMBB));
     // Add an unconditional branch from UserMBB to fallthrough block.
     // Note the new unconditional branch is not being recorded.
-    bool isThumb = AFI->isThumbFunction();
     BuildMI(UserMBB, TII->get(isThumb ? ARM::tB : ARM::B)).addMBB(NewMBB);
     BBSizes[UserMBB->getNumber()] += isThumb ? 2 : 4;
   } else {
@@ -539,11 +550,13 @@
   unsigned ID  = NextUID++;
   unsigned CPI = CPEMI->getOperand(1).getConstantPoolIndex();
   unsigned Size = CPEMI->getOperand(2).getImm();
-  
+
   // Build a new CPE for this user.
   U.CPEMI = BuildMI(NewIsland, TII->get(ARM::CONSTPOOL_ENTRY))
                 .addImm(ID).addConstantPoolIndex(CPI).addImm(Size);
   
+  // Compensate for .align 2 in thumb mode.
+  if (isThumb) Size += 2;  
   // Increase the size of the island block to account for the new entry.
   BBSizes[NewIsland->getNumber()] += Size;
   
@@ -574,7 +587,7 @@
                   << *MI);
 
   if (BrOffset <= DestOffset) {
-    if (DestOffset - BrOffset < MaxDisp)
+    if (DestOffset - BrOffset <= MaxDisp)
       return true;
   } else {
     if (BrOffset - DestOffset <= MaxDisp)