Changes in directory llvm/lib/Target/ARM:
ARMConstantIslandPass.cpp updated: 1.27 -> 1.28 README.txt updated: 1.12 -> 1.13 --- Log message: Removed WaterListOffset, inserted BBOffsets. Remove TODO item about this from README. When no water available, use end of block if in range. (More to do here.) --- Diffs of the changes: (+114 -109) ARMConstantIslandPass.cpp | 207 +++++++++++++++++++++++----------------------- README.txt | 16 ++- 2 files changed, 114 insertions(+), 109 deletions(-) Index: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp diff -u llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1.27 llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1.28 --- llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1.27 Thu Feb 22 23:02:36 2007 +++ llvm/lib/Target/ARM/ARMConstantIslandPass.cpp Sat Feb 24 18:47:03 2007 @@ -4,6 +4,7 @@ // // This file was developed by Chris Lattner and is distributed under the // University of Illinois Open Source License. See LICENSE.TXT for details. +// Substantial modifications by Evan Cheng and Dale Johannesen. // //===----------------------------------------------------------------------===// // @@ -49,21 +50,19 @@ class VISIBILITY_HIDDEN ARMConstantIslands : public MachineFunctionPass { /// NextUID - Assign unique ID's to CPE's. unsigned NextUID; - + /// BBSizes - The size of each MachineBasicBlock in bytes of code, indexed /// by MBB Number. std::vector<unsigned> BBSizes; + /// BBOffsets - the offset of each MBB in bytes, starting from 0. + std::vector<unsigned> BBOffsets; + /// WaterList - A sorted list of basic blocks where islands could be placed /// (i.e. blocks that don't fall through to the following block, due /// to a return, unreachable, or unconditional branch). std::vector<MachineBasicBlock*> WaterList; - // WaterListOffsets - the offset of the beginning of each WaterList block. - // This is computed as needed in HandleConstantPoolUser; not necessarily - // valid at arbitrary times. - std::vector<unsigned> WaterListOffsets; - /// CPUser - One user of a constant pool, keeping the machine instruction /// pointer, the constant pool being referenced, and the max displacement /// allowed from the instruction to the CP. @@ -139,16 +138,17 @@ const std::vector<MachineInstr*> &CPEMIs); MachineBasicBlock *SplitBlockBeforeInstr(MachineInstr *MI); void UpdateForInsertedWaterBlock(MachineBasicBlock *NewBB); + void AdjustBBOffsetsAfter(MachineBasicBlock *BB, int delta); bool DecrementOldEntry(unsigned CPI, MachineInstr* CPEMI, unsigned Size); - void ComputeWaterListOffsets(MachineFunction &Fn); int LookForExistingCPEntry(CPUser& U, unsigned UserOffset); bool HandleConstantPoolUser(MachineFunction &Fn, CPUser &U); bool CPEIsInRange(MachineInstr *MI, unsigned UserOffset, MachineInstr *CPEMI, unsigned Disp, bool DoDump); - bool WaterIsInRange(unsigned UserOffset, - std::vector<MachineBasicBlock*>::iterator IP, + bool WaterIsInRange(unsigned UserOffset, MachineBasicBlock *Water, unsigned Disp); + bool OffsetIsInRange(unsigned UserOffset, unsigned TrialOffset, + unsigned Disp, bool NegativeOK); bool BBIsInRange(MachineInstr *MI, MachineBasicBlock *BB, unsigned Disp); bool FixUpImmediateBr(MachineFunction &Fn, ImmBranch &Br); bool FixUpConditionalBr(MachineFunction &Fn, ImmBranch &Br); @@ -156,7 +156,6 @@ bool UndoLRSpillRestore(); unsigned GetOffsetOf(MachineInstr *MI) const; - unsigned GetOffsetOf(MachineBasicBlock *MBB) const; }; } @@ -213,6 +212,7 @@ MadeChange |= UndoLRSpillRestore(); BBSizes.clear(); + BBOffsets.clear(); WaterList.clear(); CPUsers.clear(); CPEntries.clear(); @@ -293,6 +293,7 @@ /// and finding all of the constant pool users. void ARMConstantIslands::InitialFunctionScan(MachineFunction &Fn, const std::vector<MachineInstr*> &CPEMIs) { + unsigned Offset = 0; for (MachineFunction::iterator MBBI = Fn.begin(), E = Fn.end(); MBBI != E; ++MBBI) { MachineBasicBlock &MBB = *MBBI; @@ -419,6 +420,8 @@ MBBSize += 2; BBSizes.push_back(MBBSize); + BBOffsets.push_back(Offset); + Offset += MBBSize; } } @@ -431,11 +434,7 @@ // The offset is composed of two things: the sum of the sizes of all MBB's // before this instruction's block, and the offset from the start of the block // it is in. - unsigned Offset = 0; - - // Sum block sizes before MBB. - for (unsigned BB = 0, e = MBB->getNumber(); BB != e; ++BB) - Offset += BBSizes[BB]; + unsigned Offset = BBOffsets[MBB->getNumber()]; // Sum instructions before MI in MBB. for (MachineBasicBlock::iterator I = MBB->begin(); ; ++I) { @@ -445,18 +444,6 @@ } } -/// GetOffsetOf - Return the current offset of the specified machine BB -/// from the start of the function. This offset changes as stuff is moved -/// around inside the function. -unsigned ARMConstantIslands::GetOffsetOf(MachineBasicBlock *MBB) const { - // Sum block sizes before MBB. - unsigned Offset = 0; - for (unsigned BB = 0, e = MBB->getNumber(); BB != e; ++BB) - Offset += BBSizes[BB]; - - return Offset; -} - /// CompareMBBNumbers - Little predicate function to sort the WaterList by MBB /// ID. static bool CompareMBBNumbers(const MachineBasicBlock *LHS, @@ -474,6 +461,9 @@ // Insert a size into BBSizes to align it properly with the (newly // renumbered) block numbers. BBSizes.insert(BBSizes.begin()+NewBB->getNumber(), 0); + + // Likewise for BBOffsets. + BBOffsets.insert(BBOffsets.begin()+NewBB->getNumber(), 0); // Next, update WaterList. Specifically, we need to add NewMBB as having // available water after it. @@ -528,6 +518,9 @@ // renumbered) block numbers. BBSizes.insert(BBSizes.begin()+NewBB->getNumber(), 0); + // Likewise for BBOffsets. + BBOffsets.insert(BBOffsets.begin()+NewBB->getNumber(), 0); + // Next, update WaterList. Specifically, we need to add OrigMBB as having // available water after it (but not if it's already there, which happens // when splitting before a conditional branch that is followed by an @@ -547,44 +540,57 @@ I != E; ++I) NewBBSize += ARM::GetInstSize(I); + unsigned OrigBBI = OrigBB->getNumber(); + unsigned NewBBI = NewBB->getNumber(); // Set the size of NewBB in BBSizes. - BBSizes[NewBB->getNumber()] = NewBBSize; + BBSizes[NewBBI] = NewBBSize; // We removed instructions from UserMBB, subtract that off from its size. // Add 2 or 4 to the block to count the unconditional branch we added to it. - BBSizes[OrigBB->getNumber()] -= NewBBSize - (isThumb ? 2 : 4); + unsigned delta = isThumb ? 2 : 4; + BBSizes[OrigBBI] -= NewBBSize - delta; + + // ...and adjust BBOffsets for NewBB accordingly. + BBOffsets[NewBBI] = BBOffsets[OrigBBI] + BBSizes[OrigBBI]; + + // All BBOffsets following these blocks must be modified. + AdjustBBOffsetsAfter(NewBB, delta); return NewBB; } +//// OffsetIsInRange - Checks whether UserOffset is within MaxDisp of +/// TrialOffset. +bool ARMConstantIslands::OffsetIsInRange(unsigned UserOffset, + unsigned TrialOffset, unsigned MaxDisp, bool NegativeOK) { + if (UserOffset <= TrialOffset) { + // User before the Trial. + if (TrialOffset-UserOffset <= MaxDisp) + return true; + } else if (NegativeOK) { + if (UserOffset-TrialOffset <= MaxDisp) + return true; + } + return false; +} + /// WaterIsInRange - Returns true if a CPE placed after the specified /// Water (a basic block) will be in range for the specific MI. bool ARMConstantIslands::WaterIsInRange(unsigned UserOffset, - std::vector<MachineBasicBlock*>::iterator IP, - unsigned MaxDisp) + MachineBasicBlock* Water, unsigned MaxDisp) { - MachineBasicBlock *Water = *IP; - unsigned Index = IP - WaterList.begin(); - unsigned CPEOffset = WaterListOffsets[Index] + + bool isThumb = AFI->isThumbFunction(); + unsigned CPEOffset = BBOffsets[Water->getNumber()] + BBSizes[Water->getNumber()]; // If the Water is a constpool island, it has already been aligned. // If not, align it. - if (AFI->isThumbFunction() && + if (isThumb && (Water->empty() || Water->begin()->getOpcode() != ARM::CONSTPOOL_ENTRY)) CPEOffset += 2; - if (UserOffset <= CPEOffset) { - // User before the CPE. - if (CPEOffset-UserOffset <= MaxDisp) - return true; - } else if (!AFI->isThumbFunction()) { - // Thumb LDR cannot encode negative offset. - if (UserOffset-CPEOffset <= MaxDisp) - return true; - } - return false; + return OffsetIsInRange (UserOffset, CPEOffset, MaxDisp, !isThumb); } /// CPEIsInRange - Returns true if the distance between specific MI and @@ -594,7 +600,8 @@ unsigned MaxDisp, bool DoDump) { // In thumb mode, pessimistically assumes the .align 2 before the first CPE // in the island adds two byte padding. - unsigned AlignAdj = AFI->isThumbFunction() ? 2 : 0; + bool isThumb = AFI->isThumbFunction(); + unsigned AlignAdj = isThumb ? 2 : 0; unsigned CPEOffset = GetOffsetOf(CPEMI) + AlignAdj; if (DoDump) { @@ -605,16 +612,7 @@ << " offset=" << int(CPEOffset-UserOffset) << "\t" << *MI; } - if (UserOffset <= CPEOffset) { - // User before the CPE. - if (CPEOffset-UserOffset <= MaxDisp) - return true; - } else if (!AFI->isThumbFunction()) { - // Thumb LDR cannot encode negative offset. - if (UserOffset-CPEOffset <= MaxDisp) - return true; - } - return false; + return OffsetIsInRange(UserOffset, CPEOffset, MaxDisp, !isThumb); } /// BBIsJumpedOver - Return true of the specified basic block's only predecessor @@ -631,6 +629,13 @@ return false; } +void ARMConstantIslands::AdjustBBOffsetsAfter(MachineBasicBlock *BB, int delta) +{ + MachineFunction::iterator MBBI = BB->getParent()->end(); + for(int i=BB->getNumber()+1; i<=prior(MBBI)->getNumber(); i++) + BBOffsets[i] += delta; +} + /// DecrementOldEntry - find the constant pool entry with index CPI /// and instruction CPEMI, and decrement its refcount. If the refcount /// becomes 0 remove the entry and instruction. Returns true if we removed @@ -647,14 +652,21 @@ // In thumb mode, the size of island is padded by two to compensate for // the alignment requirement. Thus it will now be 2 when the block is // empty, so fix this. - BBSizes[OldCPEBB->getNumber()] = 0; + // All succeeding offsets have the current size value added in, fix this. + if (BBSizes[OldCPEBB->getNumber()] != 0) { + AdjustBBOffsetsAfter(OldCPEBB, -BBSizes[OldCPEBB->getNumber()]); + BBSizes[OldCPEBB->getNumber()] = 0; + } // An island has only one predecessor BB and one successor BB. Check if // this BB's predecessor jumps directly to this BB's successor. This // shouldn't happen currently. assert(!BBIsJumpedOver(OldCPEBB) && "How did this happen?"); // FIXME: remove the empty blocks after all the work is done? - } else + } else { BBSizes[OldCPEBB->getNumber()] -= Size; + // All succeeding offsets have the current size value added in, fix this. + AdjustBBOffsetsAfter(OldCPEBB, -Size); + } OldCPE->CPEMI->eraseFromParent(); OldCPE->CPEMI = NULL; NumCPEs--; @@ -663,25 +675,6 @@ return false; } -/// ComputeWaterListOffsets - just what you think. -/// This vector is built to avoid re-adding BBSizes for each WaterBB under test -/// (which would cause the algorithm to be n^2). -void ARMConstantIslands::ComputeWaterListOffsets(MachineFunction &Fn) { - unsigned WaterListIndex = 0; - unsigned Offset = 0; - unsigned BB = 0; - WaterListOffsets.clear(); - for (MachineFunction::iterator MBBI = Fn.begin(), E = Fn.end(); - MBBI != E; ++BB, ++MBBI) { - MachineBasicBlock *MBB = MBBI; - if (MBB == WaterList[WaterListIndex]) { - WaterListOffsets.push_back(Offset); - WaterListIndex++; - } - Offset += BBSizes[BB]; - } -} - /// LookForCPEntryInRange - see if the currently referenced CPE is in range; /// if not, see if an in-range clone of the CPE is in range, and if so, /// change the data structures so the user references the clone. Returns: @@ -759,15 +752,10 @@ // we might find some that are backwards). bool WaterFound = false; if (!WaterList.empty()) { - // Compute offsets for the blocks in the current WaterList. - // It is a big compile-time speed win to do this only once - // rather than for each WaterList entry. - ComputeWaterListOffsets(Fn); - assert(WaterList.size() == WaterListOffsets.size()); for (std::vector<MachineBasicBlock*>::iterator IP = prior(WaterList.end()), B = WaterList.begin();; --IP) { MachineBasicBlock* WaterBB = *IP; - if (WaterIsInRange(UserOffset, IP, U.MaxDisp)) { + if (WaterIsInRange(UserOffset, WaterBB, U.MaxDisp)) { WaterFound = true; DOUT << "found water in range\n"; // CPE goes before following block (NewMBB). @@ -786,23 +774,40 @@ if (!WaterFound) { // No water found. - // Solution of last resort: split the user's MBB right after the user - // and insert a clone of the CPE into the newly created water. DOUT << "No water found\n"; MachineBasicBlock *UserMBB = UserMI->getParent(); - - // 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. - if (&UserMBB->back() == UserMI) { - assert(BBHasFallthrough(UserMBB) && "Expected a fallthrough BB!"); + unsigned TrialOffset = BBOffsets[UserMBB->getNumber()] + + BBSizes[UserMBB->getNumber()] + + isThumb ? 2 : 4; /* for branch to be added */ + + // If the use is at the end of the block, or the end of the block + // is within range, make new water there. (If the block ends in + // an unconditional branch already, it is water, and is known to + // be out of range; so it's OK to assume above we'll be adding a Br.) + if (&UserMBB->back() == UserMI || + OffsetIsInRange(UserOffset, TrialOffset, U.MaxDisp, !isThumb)) { + 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. BuildMI(UserMBB, TII->get(isThumb ? ARM::tB : ARM::B)).addMBB(NewMBB); - BBSizes[UserMBB->getNumber()] += isThumb ? 2 : 4; + int delta = isThumb ? 2 : 4; + BBSizes[UserMBB->getNumber()] += delta; + AdjustBBOffsetsAfter(UserMBB, delta); } else { + // What a big block. Find a place within the block to split it. + // This is a little tricky on Thumb since instructions are 2 bytes + // and constant pool entries are 4 bytes: if instruction I references + // island CPE, and instruction I+1 references CPE', it will + // not work well to put CPE as far forward as possible, since then + // CPE' cannot immediately follow it (that location is 2 bytes + // farther away from I+1 than CPE was from I) and we'd need to create + // a new island. + + // Solution of last resort: split the user's MBB right after the user + // and insert a clone of the CPE into the newly created water. MachineInstr *NextMI = next(MachineBasicBlock::iterator(UserMI)); NewMBB = SplitBlockBeforeInstr(NextMI); } @@ -829,6 +834,8 @@ if (isThumb) Size += 2; // Increase the size of the island block to account for the new entry. BBSizes[NewIsland->getNumber()] += Size; + BBOffsets[NewIsland->getNumber()] = BBOffsets[NewMBB->getNumber()]; + AdjustBBOffsetsAfter(NewIsland, Size); // Finally, change the CPI in the instruction operand to be ID. for (unsigned i = 0, e = UserMI->getNumOperands(); i != e; ++i) @@ -848,21 +855,14 @@ unsigned MaxDisp) { unsigned PCAdj = AFI->isThumbFunction() ? 4 : 8; unsigned BrOffset = GetOffsetOf(MI) + PCAdj; - unsigned DestOffset = GetOffsetOf(DestBB); + unsigned DestOffset = BBOffsets[DestBB->getNumber()]; DOUT << "Branch of destination BB#" << DestBB->getNumber() << " from BB#" << MI->getParent()->getNumber() << " max delta=" << MaxDisp << " at offset " << int(DestOffset-BrOffset) << "\t" << *MI; - if (BrOffset <= DestOffset) { - if (DestOffset - BrOffset <= MaxDisp) - return true; - } else { - if (BrOffset - DestOffset <= MaxDisp) - return true; - } - return false; + return OffsetIsInRange(BrOffset, DestOffset, MaxDisp, true); } /// FixUpImmediateBr - Fix up an immediate branch whose destination is too far @@ -894,6 +894,7 @@ Br.MaxDisp = (1 << 21) * 2; MI->setInstrDescriptor(TII->get(ARM::tBfar)); BBSizes[MBB->getNumber()] += 2; + AdjustBBOffsetsAfter(MBB, 2); HasFarJump = true; NumUBrFixed++; @@ -977,7 +978,9 @@ MI->eraseFromParent(); // Increase the size of MBB to account for the new unconditional branch. - BBSizes[MBB->getNumber()] += ARM::GetInstSize(&MBB->back()); + int delta = ARM::GetInstSize(&MBB->back()); + BBSizes[MBB->getNumber()] += delta; + AdjustBBOffsetsAfter(MBB, delta); return true; } Index: llvm/lib/Target/ARM/README.txt diff -u llvm/lib/Target/ARM/README.txt:1.12 llvm/lib/Target/ARM/README.txt:1.13 --- llvm/lib/Target/ARM/README.txt:1.12 Thu Feb 22 23:02:36 2007 +++ llvm/lib/Target/ARM/README.txt Sat Feb 24 18:47:03 2007 @@ -21,17 +21,19 @@ previous version of this document have been addressed. However, there are still things that can be done: -1. When there isn't an existing water, the current MBB is split right after the -use. It would be profitable to look farther forward, especially on Thumb, +1. When there isn't an existing water, the current MBB is split right after +the use. It would be profitable to look farther forward, especially on Thumb, where negative offsets won't work. +Now it will put the island at the end of the block if that is in range. If it +is not in range things still work as above, which is poor on Thumb. -2. WaterBlockListOffsets might be maintained throughout, rather than computed -when it is needed. This would probably lead to faster compile times. -Similarly, the offsets of blocks might be maintained throughout. - -3. There may be some advantage to trying to be smarter about the initial +2. There may be some advantage to trying to be smarter about the initial placement, rather than putting everything at the end. +3. The handling of 2-byte padding for Thumb is overly conservative. There +would be a small gain to keeping accurate track of the padding (which would +require aligning functions containing constant pools to 4-byte boundaries). + //===---------------------------------------------------------------------===// We need to start generating predicated instructions. The .td files have a way _______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits