[llvm-commits] [llvm] r44687 - in /llvm/trunk: include/llvm/CodeGen/Passes.h lib/CodeGen/LLVMTargetMachine.cpp lib/CodeGen/MachineLICM.cpp lib/Target/PowerPC/PPCInstrInfo.td

[Bill Wendling]

On Dec 7, 2007 5:01 PM, Chris Lattner <clattner at apple.com> wrote:
> >> +    // Visit all of the instructions of the loop. We want to visit
> >> the subloops
> >> +    // first, though, so that we can hoist their invariants first
> >> into their
> >> +    // containing loop before we process that loop.
> >> +    SmallVector<MachineLoop*, 16> Loops;
> >> +    GatherAllLoops(L, Loops);
> >
> > Seems to me this can be smarter. When you are gathering the loops, put
> > loops of greater depth in the front of the queue then HoistRegion
> > won't have to check if the BB is in the current loop level?
>
> This should do a simple postorder traversal of the loop nest, there
> is no need to make a vector of the loops.  This ensures you visit
> inner loops before outer loops etc.
>
Doh! Yeah, it's easier to just do it without the vector.

> >> +  // Now move the instructions to the predecessors.
> >> +  for (std::vector<MachineBasicBlock*>::iterator
> >> +         I = Preds.begin(), E = Preds.end(); I != E; ++I)
> >> +    MoveInstToBlock(*I, MI.clone());
> >
> > This will create multiple copies, it seems bad. Again, this should be
> > fixable with a loop preheader?
>
> Furthermore, this won't work, you'd have to insert phi nodes etc.
> You really really really don't want to do this bill.  Only handle
> loops that have a header with one predecessor that is outside the
> loop plz,
>
Ah! Good point. When there's pre-headers, I'll be able to handle this
much nicer. In the meantime, I'll make sure that there's only one
predecessor.

-bw