[llvm-commits] CVS: llvm/lib/Transforms/Utils/LoopSimplify.cpp

[Chris Lattner]

>        // Can we eliminate this phi node now?
>        if (Value *V = PN->hasConstantValue(true)) {
> -        if (!isa<Instruction>(V) ||
> -            getAnalysis<DominatorSet>().dominates(cast<Instruction> 
> (V), PN)) {
> +        Instruction *I = dyn_cast<Instruction>(V);
> +        if (!I || (I->getParent() != NewBB &&
> +                   getAnalysis<ETForest>().dominates(I, PN))) {

Please add a comment that describes what is going on here, this is  
subtle/non-obvious.

> @@ -727,14 +700,16 @@
>    bool NewBBDominatesNewBBSucc = true;
>    {
>      BasicBlock *OnePred = PredBlocks[0];
> -    unsigned i, e = PredBlocks.size();
> -    for (i = 1; !DS.isReachable(OnePred); ++i) {
> +    unsigned i = 1, e = PredBlocks.size();
> +    for (i = 1; !ETF.dominates(&OnePred->getParent()->getEntryBlock 
> (), OnePred);
> +         ++i) {

Please add a ETForest::isReachableFromEntry(x).

> @@ -885,8 +844,14 @@
>      for (unsigned i = 0, e = PredBlocks.size(); i != e; ++i) {
>        BasicBlock *Pred = PredBlocks[i];
>        // Get all of the dominators of the predecessor...
> -      const DominatorSet::DomSetType &PredDoms = DS.getDominators 
> (Pred);
> +      // FIXME: There's probably a better way to do this...
> +      std::vector<BasicBlock*> PredDoms;
> +      for (Function::iterator I = Pred->getParent()->begin(),
> +           E = Pred->getParent()->end(); I != E; ++I)
> +        if (ETF.dominates(&(*I), Pred))
> +          PredDoms.push_back(I);
> +
> -      for (DominatorSet::DomSetType::const_iterator PDI =  
> PredDoms.begin(),
> +      for (std::vector<BasicBlock*>::const_iterator PDI =  
> PredDoms.begin(),
>               PDE = PredDoms.end(); PDI != PDE; ++PDI) {
>          BasicBlock *PredDom = *PDI;

This does need to be fixed, it is not very efficient.  Minor things:  
(&(*I)) -> (I).

I would suggest an approach like this, if it works:

foreach BB in function:
   if (NewBBSucc is in DF(BB))
     if (any BB dominates any of Preds)
       .. inner check for 'shouldremove' etc ..

-Chris