[llvm-commits] [llvm] r86791 - /llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp

[Evan Cheng]

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