Accurately model hardware alignment rounding.
authorJakob Stoklund Olesen <stoklund@2pi.dk>
Tue, 10 Jan 2012 01:34:59 +0000 (01:34 +0000)
committerJakob Stoklund Olesen <stoklund@2pi.dk>
Tue, 10 Jan 2012 01:34:59 +0000 (01:34 +0000)
On Thumb, the displacement computation hardware uses the address of the
current instruction rouned down to a multiple of 4.  Include this
rounding in the UserOffset we compute for each instruction.

When inline asm is present, the instruction alignment may not be known.
Constrain the maximum displacement instead in that case.

This makes it possible for CreateNewWater() and OffsetIsInRange() to
agree about the valid displacements.  When they disagree, infinite
looping happens.

As always, test cases for this stuff are insane.

<rdar://problem/10660175>

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@147825 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/ARM/ARMConstantIslandPass.cpp

index 36be2308c97cfa4d0781eea3a9027dbe4e1190ba..e2ff4905e231b7ac942a0dbaf578be833f62a7de 100644 (file)
@@ -195,14 +195,23 @@ namespace {
       MachineInstr *MI;
       MachineInstr *CPEMI;
       MachineBasicBlock *HighWaterMark;
+    private:
       unsigned MaxDisp;
+    public:
       bool NegOk;
       bool IsSoImm;
+      bool KnownAlignment;
       CPUser(MachineInstr *mi, MachineInstr *cpemi, unsigned maxdisp,
              bool neg, bool soimm)
-        : MI(mi), CPEMI(cpemi), MaxDisp(maxdisp), NegOk(neg), IsSoImm(soimm) {
+        : MI(mi), CPEMI(cpemi), MaxDisp(maxdisp), NegOk(neg), IsSoImm(soimm),
+          KnownAlignment(false) {
         HighWaterMark = CPEMI->getParent();
       }
+      /// getMaxDisp - Returns the maximum displacement supported by MI.
+      /// Correct for unknown alignment.
+      unsigned getMaxDisp() const {
+        return KnownAlignment ? MaxDisp : MaxDisp - 2;
+      }
     };
 
     /// CPUsers - Keep track of all of the machine instructions that use various
@@ -309,6 +318,7 @@ namespace {
 
     void ComputeBlockSize(MachineBasicBlock *MBB);
     unsigned GetOffsetOf(MachineInstr *MI) const;
+    unsigned GetUserOffset(CPUser&) const;
     void dumpBBs();
     void verify();
 
@@ -317,7 +327,7 @@ namespace {
     bool OffsetIsInRange(unsigned UserOffset, unsigned TrialOffset,
                          const CPUser &U) {
       return OffsetIsInRange(UserOffset, TrialOffset,
-                             U.MaxDisp, U.NegOk, U.IsSoImm);
+                             U.getMaxDisp(), U.NegOk, U.IsSoImm);
     }
   };
   char ARMConstantIslands::ID = 0;
@@ -336,11 +346,11 @@ void ARMConstantIslands::verify() {
   }
   for (unsigned i = 0, e = CPUsers.size(); i != e; ++i) {
     CPUser &U = CPUsers[i];
-    unsigned UserOffset = GetOffsetOf(U.MI) + (isThumb ? 4 : 8);
+    unsigned UserOffset = GetUserOffset(U);
     unsigned CPEOffset  = GetOffsetOf(U.CPEMI);
     unsigned Disp = UserOffset < CPEOffset ? CPEOffset - UserOffset :
       UserOffset - CPEOffset;
-    assert(Disp <= U.MaxDisp || "Constant pool entry out of range!");
+    assert(Disp <= U.getMaxDisp() || "Constant pool entry out of range!");
   }
 #endif
 }
@@ -546,7 +556,8 @@ ARMConstantIslands::DoInitialPlacement(std::vector<MachineInstr*> &CPEMIs) {
     CPEs.push_back(CPEntry(CPEMI, i));
     CPEntries.push_back(CPEs);
     ++NumCPEs;
-    DEBUG(dbgs() << "Moved CPI#" << i << " to end of function\n");
+    DEBUG(dbgs() << "Moved CPI#" << i << " to end of function, size = "
+                 << Size << ", align = " << Align <<'\n');
   }
   DEBUG(BB->dump());
 }
@@ -924,19 +935,39 @@ MachineBasicBlock *ARMConstantIslands::SplitBlockBeforeInstr(MachineInstr *MI) {
   return NewBB;
 }
 
+/// GetUserOffset - Compute the offset of U.MI as seen by the hardware
+/// displacement computation.  Update U.KnownAlignment to match its current
+/// basic block location.
+unsigned ARMConstantIslands::GetUserOffset(CPUser &U) const {
+  unsigned UserOffset = GetOffsetOf(U.MI);
+  const BasicBlockInfo &BBI = BBInfo[U.MI->getParent()->getNumber()];
+  unsigned KnownBits = BBI.internalKnownBits();
+
+  // The value read from PC is offset from the actual instruction address.
+  UserOffset += (isThumb ? 4 : 8);
+
+  // Because of inline assembly, we may not know the alignment (mod 4) of U.MI.
+  // Make sure U.getMaxDisp() returns a constrained range.
+  U.KnownAlignment = (KnownBits >= 2);
+
+  // On Thumb, offsets==2 mod 4 are rounded down by the hardware for
+  // purposes of the displacement computation; compensate for that here.
+  // For unknown alignments, getMaxDisp() constrains the range instead.
+  if (isThumb && U.KnownAlignment)
+    UserOffset &= ~3u;
+
+  return UserOffset;
+}
+
 /// OffsetIsInRange - Checks whether UserOffset (the location of a constant pool
 /// reference) is within MaxDisp of TrialOffset (a proposed location of a
 /// constant pool entry).
+/// UserOffset is computed by GetUserOffset above to include PC adjustments. If
+/// the mod 4 alignment of UserOffset is not known, the uncertainty must be
+/// subtracted from MaxDisp instead. CPUser::getMaxDisp() does that.
 bool ARMConstantIslands::OffsetIsInRange(unsigned UserOffset,
                                          unsigned TrialOffset, unsigned MaxDisp,
                                          bool NegativeOK, bool IsSoImm) {
-  // On Thumb offsets==2 mod 4 are rounded down by the hardware for
-  // purposes of the displacement computation; compensate for that here.
-  // Effectively, the valid range of displacements is 2 bytes smaller for such
-  // references.
-  if (isThumb && UserOffset%4 !=0)
-    UserOffset -= 2;
-
   if (UserOffset <= TrialOffset) {
     // User before the Trial.
     if (TrialOffset - UserOffset <= MaxDisp)
@@ -1086,7 +1117,7 @@ int ARMConstantIslands::LookForExistingCPEntry(CPUser& U, unsigned UserOffset)
   MachineInstr *CPEMI  = U.CPEMI;
 
   // Check to see if the CPE is already in-range.
-  if (CPEIsInRange(UserMI, UserOffset, CPEMI, U.MaxDisp, U.NegOk, true)) {
+  if (CPEIsInRange(UserMI, UserOffset, CPEMI, U.getMaxDisp(), U.NegOk, true)) {
     DEBUG(dbgs() << "In range\n");
     return 1;
   }
@@ -1101,7 +1132,8 @@ int ARMConstantIslands::LookForExistingCPEntry(CPUser& U, unsigned UserOffset)
     // Removing CPEs can leave empty entries, skip
     if (CPEs[i].CPEMI == NULL)
       continue;
-    if (CPEIsInRange(UserMI, UserOffset, CPEs[i].CPEMI, U.MaxDisp, U.NegOk)) {
+    if (CPEIsInRange(UserMI, UserOffset, CPEs[i].CPEMI, U.getMaxDisp(),
+                     U.NegOk)) {
       DEBUG(dbgs() << "Replacing CPE#" << CPI << " with CPE#"
                    << CPEs[i].CPI << "\n");
       // Point the CPUser node to the replacement
@@ -1202,8 +1234,7 @@ void ARMConstantIslands::CreateNewWater(unsigned CPUserIndex,
   // If the block does not end in an unconditional branch already, and if the
   // end of the block is within range, make new water there.  (The addition
   // below is for the unconditional branch we will be adding: 4 bytes on ARM +
-  // Thumb2, 2 on Thumb1.  Possible Thumb1 alignment padding is allowed for
-  // inside OffsetIsInRange.
+  // Thumb2, 2 on Thumb1.
   if (BBHasFallthrough(UserMBB)) {
     // Size of branch to insert.
     unsigned Delta = isThumb1 ? 2 : 4;
@@ -1256,7 +1287,7 @@ void ARMConstantIslands::CreateNewWater(unsigned CPUserIndex,
   assert(LogAlign >= CPELogAlign && "Over-aligned constant pool entry");
   unsigned KnownBits = UserBBI.internalKnownBits();
   unsigned UPad = UnknownPadding(LogAlign, KnownBits);
-  unsigned BaseInsertOffset = UserOffset + U.MaxDisp;
+  unsigned BaseInsertOffset = UserOffset + U.getMaxDisp();
   DEBUG(dbgs() << format("Split in middle of big block before %#x",
                          BaseInsertOffset));
 
@@ -1337,9 +1368,8 @@ bool ARMConstantIslands::HandleConstantPoolUser(unsigned CPUserIndex) {
   MachineInstr *CPEMI  = U.CPEMI;
   unsigned CPI = CPEMI->getOperand(1).getIndex();
   unsigned Size = CPEMI->getOperand(2).getImm();
-  // Compute this only once, it's expensive.  The 4 or 8 is the value the
-  // hardware keeps in the PC.
-  unsigned UserOffset = GetOffsetOf(UserMI) + (isThumb ? 4 : 8);
+  // Compute this only once, it's expensive.
+  unsigned UserOffset = GetUserOffset(U);
 
   // See if the current entry is within range, or there is a clone of it
   // in range.
@@ -1677,8 +1707,13 @@ bool ARMConstantIslands::OptimizeThumb2Instructions() {
     if (!NewOpc)
       continue;
 
-    unsigned UserOffset = GetOffsetOf(U.MI) + 4;
+    unsigned UserOffset = GetUserOffset(U);
     unsigned MaxOffs = ((1 << Bits) - 1) * Scale;
+
+    // Be conservative with inline asm.
+    if (!U.KnownAlignment)
+      MaxOffs -= 2;
+
     // FIXME: Check if offset is multiple of scale if scale is not 4.
     if (CPEIsInRange(U.MI, UserOffset, U.CPEMI, MaxOffs, false, true)) {
       U.MI->setDesc(TII->get(NewOpc));