>> + // 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. >> + // If the loop contains the definition of an operand, then the >> instruction >> + // isn't loop invariant. >> + if (CurLoop->contains(VRegDefs[Reg]->getParent())) >> + return false; > > Hmmm. I am not sure about this. What if the definition is in the > preheader? Does contains() returns true? Chris, Owen? a preheader is outside the loop. The header is the "top" of the loop. >> + // FIXME: We are assuming at first that the basic blocks coming >> into this >> + // loop have only one successor each. This isn't the case in >> general because >> + // we haven't broken critical edges or added preheaders. >> + if (MBB->succ_size() != 1) return false; >> + assert(*MBB->succ_begin() == CurLoop->getHeader() && >> + "The predecessor doesn't feed directly into the loop >> header!"); >> + } > > Are we going to create a preheader to hoist LICM to? Then we won't > need to do all these checking? I am entirely sure about it. Someone > please chime in. Step #0 is to only work with simple loop forms. Going forward, bill will generalize this to handle more cases. >> >> + >> + // 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, Very nice bill! -Chris _______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
