[llvm-commits] [llvm] r86791 - /llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp
Jim Grosbach
grosbach at apple.com
Wed Nov 11 09:22:41 PST 2009
Yeah, it definitely should be moved. That's one of the things I'm
going to do this morning.
We could do it in the placement optimization pass, but I'm a bit
hesitant at this point since a fair bit of the information we may use
for determining how and whether to make transformations will be target
specific. The ranges of the TB[BH] instructions being the most
straightforward example. That may be handleable via target hooks or
something, though. Once we have a better handle on the specifics of
the heuristics, that should be more apparent.
Why was the code placement opt unsafe for arm before? I don't recall
the history there.
-Jim
On Nov 10, 2009, at 11:05 PM, Evan Cheng wrote:
> Is this done during OptimizeThumb2JumpTables? Then I don't think
> that would work. It will mess up constant island placements. Perhaps
> this has to be done before the constant islands part? Or even in the
> bb placement optimization pass (which should be safe to turn on for
> ARM now)?
>
> Evan
>
> On Nov 10, 2009, at 6:47 PM, Jim Grosbach wrote:
>
>> Author: grosbach
>> Date: Tue Nov 10 20:47:19 2009
>> New Revision: 86791
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=86791&view=rev
>> Log:
>>
>> The TBB and TBH instructions for Thumb2 are really handy for jump
>> tables, but
>> can only branch forward. To best take advantage of them, we'd like
>> to adjust
>> the basic blocks around a bit when reasonable. This patch puts
>> basics in place
>> to do that, with a super-simple algorithm for backwards jump table
>> targets that
>> creates a new branch after the jump table which branches backwards.
>> Real
>> heuristics for reordering blocks or other modifications rather than
>> inserting
>> branches will follow.
>>
>>
>> Modified:
>> llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp
>>
>> Modified: llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp?rev=86791&r1=86790&r2=86791&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp Tue Nov 10
>> 20:47:19 2009
>> @@ -31,6 +31,7 @@
>> #include "llvm/ADT/SmallVector.h"
>> #include "llvm/ADT/STLExtras.h"
>> #include "llvm/ADT/Statistic.h"
>> +#include "llvm/Support/CommandLine.h"
>> #include <algorithm>
>> using namespace llvm;
>>
>> @@ -42,6 +43,12 @@
>> STATISTIC(NumT2CPShrunk, "Number of Thumb2 constantpool
>> instructions shrunk");
>> STATISTIC(NumT2BrShrunk, "Number of Thumb2 immediate branches
>> shrunk");
>> STATISTIC(NumCBZ, "Number of CBZ / CBNZ formed");
>> +STATISTIC(NumJTMoved, "Number of jump table destination blocks
>> moved");
>> +
>> +
>> +static cl::opt<bool>
>> +AdjustJumpTableBlocks("arm-adjust-jump-tables", cl::Hidden,
>> cl::init(false),
>> + cl::desc("Adjust basic block layout to better use TB
>> [BH]"));
>>
>> namespace {
>> /// ARMConstantIslands - Due to limited PC-relative displacements,
>> ARM
>> @@ -202,6 +209,8 @@
>> bool OptimizeThumb2Instructions(MachineFunction &MF);
>> bool OptimizeThumb2Branches(MachineFunction &MF);
>> bool OptimizeThumb2JumpTables(MachineFunction &MF);
>> + MachineBasicBlock *AdjustJTTargetBlockForward
>> (MachineBasicBlock *BB,
>> +
>> MachineBasicBlock *JTBB);
>>
>> unsigned GetOffsetOf(MachineInstr *MI) const;
>> void dumpBBs();
>> @@ -1560,7 +1569,7 @@
>>
>> // FIXME: After the tables are shrunk, can we get rid some of the
>> // constantpool tables?
>> - const MachineJumpTableInfo *MJTI = MF.getJumpTableInfo();
>> + MachineJumpTableInfo *MJTI = MF.getJumpTableInfo();
>> const std::vector<MachineJumpTableEntry> &JT = MJTI->getJumpTables
>> ();
>> for (unsigned i = 0, e = T2JumpTables.size(); i != e; ++i) {
>> MachineInstr *MI = T2JumpTables[i];
>> @@ -1571,10 +1580,33 @@
>> unsigned JTI = JTOP.getIndex();
>> assert(JTI < JT.size());
>>
>> - bool ByteOk = true;
>> - bool HalfWordOk = true;
>> + // We prefer if target blocks for the jump table come after
>> the jump
>> + // instruction so we can use TB[BH]. Loop through the target
>> blocks
>> + // and try to adjust them such that that's true.
>> unsigned JTOffset = GetOffsetOf(MI) + 4;
>> const std::vector<MachineBasicBlock*> &JTBBs = JT[JTI].MBBs;
>> + if (AdjustJumpTableBlocks) {
>> + for (unsigned j = 0, ee = JTBBs.size(); j != ee; ++j) {
>> + MachineBasicBlock *MBB = JTBBs[j];
>> + unsigned DstOffset = BBOffsets[MBB->getNumber()];
>> +
>> + if (DstOffset < JTOffset) {
>> + // The destination precedes the switch. Try to move the
>> block forward
>> + // so we have a positive offset.
>> + MachineBasicBlock *NewBB =
>> + AdjustJTTargetBlockForward(MBB, MI->getParent());
>> + if (NewBB) {
>> + MJTI->ReplaceMBBInJumpTables(JTBBs[j], NewBB);
>> + JTOffset = GetOffsetOf(MI) + 4;
>> + DstOffset = BBOffsets[MBB->getNumber()];
>> + }
>> + }
>> + }
>> + }
>> +
>> + bool ByteOk = true;
>> + bool HalfWordOk = true;
>> + JTOffset = GetOffsetOf(MI) + 4;
>> for (unsigned j = 0, ee = JTBBs.size(); j != ee; ++j) {
>> MachineBasicBlock *MBB = JTBBs[j];
>> unsigned DstOffset = BBOffsets[MBB->getNumber()];
>> @@ -1660,3 +1692,64 @@
>>
>> return MadeChange;
>> }
>> +
>> +MachineBasicBlock *ARMConstantIslands::
>> +AdjustJTTargetBlockForward(MachineBasicBlock *BB,
>> MachineBasicBlock *JTBB)
>> +{
>> + MachineFunction &MF = *BB->getParent();
>> +
>> + // FIXME: For now, instead of moving the block, we'll create a
>> new block
>> + // immediate following the jump that's an unconditional branch
>> to the
>> + // actual target. This is obviously not what we want for a real
>> solution,
>> + // but it's useful for proof of concept, and it may be a useful
>> fallback
>> + // later for cases where we otherwise can't move a block.
>> +
>> + // Create a new MBB for the code after the jump BB.
>> + MachineBasicBlock *NewBB =
>> + MF.CreateMachineBasicBlock(JTBB->getBasicBlock());
>> + MachineFunction::iterator MBBI = JTBB; ++MBBI;
>> + MF.insert(MBBI, NewBB);
>> +
>> + // Add an unconditional branch from NewBB to BB.
>> + // There doesn't seem to be meaningful DebugInfo available; this
>> doesn't
>> + // correspond directly to anything in the source.
>> + assert (isThumb2 && "Adjusting for TB[BH] but not in Thumb2?");
>> + BuildMI(NewBB, DebugLoc::getUnknownLoc(), TII->get
>> (ARM::t2B)).addMBB(BB);
>> +
>> + // Update the CFG.
>> + NewBB->addSuccessor(BB);
>> + JTBB->removeSuccessor(BB);
>> + JTBB->addSuccessor(NewBB);
>> +
>> + // Update internal data structures to account for the newly
>> inserted MBB.
>> + // This is almost the same as UpdateForInsertedWaterBlock,
>> except that
>> + // the Water goes after OrigBB, not NewBB.
>> + MF.RenumberBlocks(NewBB);
>> +
>> + // 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);
>> +
>> + // Figure out how large the first NewMBB is.
>> + unsigned NewBBSize = 0;
>> + for (MachineBasicBlock::iterator I = NewBB->begin(), E = NewBB-
>> >end();
>> + I != E; ++I)
>> + NewBBSize += TII->GetInstSizeInBytes(I);
>> +
>> + unsigned NewBBI = NewBB->getNumber();
>> + unsigned JTBBI = JTBB->getNumber();
>> + // Set the size of NewBB in BBSizes.
>> + BBSizes[NewBBI] = NewBBSize;
>> +
>> + // ...and adjust BBOffsets for NewBB accordingly.
>> + BBOffsets[NewBBI] = BBOffsets[JTBBI] + BBSizes[JTBBI];
>> +
>> + // All BBOffsets following these blocks must be modified.
>> + AdjustBBOffsetsAfter(NewBB, 4);
>> +
>> + ++NumJTMoved;
>> + return NewBB;
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list