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

[Evan Cheng]

Hi Jim,

Some comments for 86999 and friends:

/// ReorderThumb2JumpTables - Use tbb / tbh instructions to generate smaller                                                                                             
/// jumptables when it's possible.                                                                                                                                        
bool ARMConstantIslands::ReorderThumb2JumpTables(MachineFunction &MF) {

This comment is misleading. tbb / tbh are formed later, right?

+  // FIXME: If it's a small block terminated by an unconditional branch,
+  // try to move it; otherwise, create a new block following the jump
+  // table that branches back to the actual target. This is an overly
+  // simplistic heuristic here for proof-of-concept.

What do you plan as a replacement? Jump + jump seems bad for performance? Why not just add a unconditional branch to replace the fall through?

+  // If the block is small and ends in an unconditional branch, move it.
+  if (Size < 50 && Cond.empty()) 

Where is 50 coming from?

Evan

On Nov 14, 2009, at 12:10 PM, Jim Grosbach wrote:

> Author: grosbach
> Date: Sat Nov 14 14:10:18 2009
> New Revision: 88805
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=88805&view=rev
> Log:
> Cleanup flow, and only update the jump table we're analyzing when replacing a destination MBB.
> 
> 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=88805&r1=88804&r2=88805&view=diff
> 
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp Sat Nov 14 14:10:18 2009
> @@ -1749,7 +1749,7 @@
>         MachineBasicBlock *NewBB =
>           AdjustJTTargetBlockForward(MBB, MI->getParent());
>         if (NewBB)
> -          MJTI->ReplaceMBBInJumpTables(JTBBs[j], NewBB);
> +          MJTI->ReplaceMBBInJumpTable(JTI, JTBBs[j], NewBB);
>         MadeChange = true;
>       }
>     }
> @@ -1772,16 +1772,16 @@
>   int Size = BBSizes[BBI];
>   MachineBasicBlock *TBB = 0, *FBB = 0;
>   SmallVector<MachineOperand, 4> Cond;
> -  // If the block terminator isn't analyzable, don't try to move the block
> -  if (TII->AnalyzeBranch(*BB, TBB, FBB, Cond))
> -    return NULL;
> -
>   // If the block is small and ends in an unconditional branch, move it.
>   if (Size < 50 && Cond.empty()) {
> +    // If the block terminator isn't analyzable, don't try to move the block
> +    if (TII->AnalyzeBranch(*BB, TBB, FBB, Cond))
> +      return NULL;
> +
>     MachineFunction::iterator OldPrior = prior(BB);
>     BB->moveAfter(JTBB);
>     OldPrior->updateTerminator();
> -    //BB->updateTerminator();
> +    BB->updateTerminator();
>     ++NumJTMoved;
>     return NULL;
>   }
> @@ -1792,20 +1792,22 @@
>   MachineFunction::iterator MBBI = JTBB; ++MBBI;
>   MF.insert(MBBI, NewBB);
> 
> +  //MF.splice(MBBI, NewBB, 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 internal data structures to account for the newly inserted MBB.
> +  MF.RenumberBlocks(NewBB);
> +
>   // Update the CFG.
>   NewBB->addSuccessor(BB);
>   JTBB->removeSuccessor(BB);
>   JTBB->addSuccessor(NewBB);
> 
> -  // Update internal data structures to account for the newly inserted MBB.
> -  MF.RenumberBlocks();
> -
>   // Insert a size into BBSizes to align it properly with the (newly
>   // renumbered) block numbers.
>   BBSizes.insert(BBSizes.begin()+NewBB->getNumber(), 0);
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits