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

[Reid Spencer]

On Sun, 2007-01-14 at 12:33 -0600, Chris Lattner wrote:
> 
> Changes in directory llvm/lib/Transforms/Utils:
> 
> BreakCriticalEdges.cpp updated: 1.38 -> 1.39
> ---
> Log message:
> 
> Fix PR1110: http://llvm.org/PR1110  and Analysis/Dominators/2007-01-14-BreakCritEdges.ll by being
> more careful about unreachable code when updating dominator info.

I believe this patch has now broken:
test/Regression/Analysis/Dominators/2006-10-02-BreakCritEdges.ll

It is reporting: 
Printing analysis 'Dominator Tree Construction' for function 'f':
=============================--------------------------------
Inorder Dominator Tree:
  [1]  %entry
    [2]  %brtrue
      [3]  %brtrue.brfalse_crit_edge
      [3]  %brtrue.brtrue_crit_edge
    [2]  %brfalse

which doesn't match the expected pattern of grep '3.*%brtrue$'

I'm unsure of whether this is an intended change and the test case
should be updated or whether the test case is correct and the patch
broke -break-crit-edges.

Reid.

> 
> 
> ---
> Diffs of the changes:  (+40 -35)
> 
>  BreakCriticalEdges.cpp |   75 ++++++++++++++++++++++++++-----------------------
>  1 files changed, 40 insertions(+), 35 deletions(-)
> 
> 
> Index: llvm/lib/Transforms/Utils/BreakCriticalEdges.cpp
> diff -u llvm/lib/Transforms/Utils/BreakCriticalEdges.cpp:1.38 llvm/lib/Transforms/Utils/BreakCriticalEdges.cpp:1.39
> --- llvm/lib/Transforms/Utils/BreakCriticalEdges.cpp:1.38	Tue Dec 19 16:17:40 2006
> +++ llvm/lib/Transforms/Utils/BreakCriticalEdges.cpp	Sun Jan 14 12:33:35 2007
> @@ -182,49 +182,54 @@
>    
>    // Should we update DominatorSet information?
>    if (DominatorSet *DS = P->getAnalysisToUpdate<DominatorSet>()) {
> -    // The blocks that dominate the new one are the blocks that dominate TIBB
> -    // plus the new block itself.
> -    DominatorSet::DomSetType DomSet = DS->getDominators(TIBB);
> -    DomSet.insert(NewBB);  // A block always dominates itself.
> -    DS->addBasicBlock(NewBB, DomSet);
> -    
> -    // If NewBBDominatesDestBB hasn't been computed yet, do so with DS.
> -    if (!OtherPreds.empty()) {
> -      while (!OtherPreds.empty() && NewBBDominatesDestBB) {
> -        NewBBDominatesDestBB = DS->dominates(DestBB, OtherPreds.back());
> -        OtherPreds.pop_back();
> +    DominatorSet::iterator DSI = DS->find(TIBB);
> +    if (DSI != DS->end()) {    // TIBB is reachable?
> +      // The blocks that dominate the new one are the blocks that dominate TIBB
> +      // plus the new block itself.
> +      DominatorSet::DomSetType DomSet = DSI->second;  // Copy domset.
> +      DomSet.insert(NewBB);  // A block always dominates itself.
> +      DS->addBasicBlock(NewBB, DomSet);
> +      
> +      // If NewBBDominatesDestBB hasn't been computed yet, do so with DS.
> +      if (!OtherPreds.empty()) {
> +        while (!OtherPreds.empty() && NewBBDominatesDestBB) {
> +          NewBBDominatesDestBB = DS->dominates(DestBB, OtherPreds.back());
> +          OtherPreds.pop_back();
> +        }
> +        OtherPreds.clear();
> +      }
> +      
> +      // If NewBBDominatesDestBB, then NewBB dominates DestBB, otherwise it
> +      // doesn't dominate anything.  If NewBB does dominates DestBB, then it
> +      // dominates everything that DestBB dominates.
> +      if (NewBBDominatesDestBB) {
> +        for (DominatorSet::iterator I = DS->begin(), E = DS->end(); I != E; ++I)
> +          if (I->second.count(DestBB))
> +            I->second.insert(NewBB);
>        }
> -      OtherPreds.clear();
> -    }
> -    
> -    // If NewBBDominatesDestBB, then NewBB dominates DestBB, otherwise it
> -    // doesn't dominate anything.  If NewBB does dominates DestBB, then it
> -    // dominates everything that DestBB dominates.
> -    if (NewBBDominatesDestBB) {
> -      for (DominatorSet::iterator I = DS->begin(), E = DS->end(); I != E; ++I)
> -        if (I->second.count(DestBB))
> -          I->second.insert(NewBB);
>      }
>    }
>  
>    // Should we update ImmediateDominator information?
>    if (ImmediateDominators *ID = P->getAnalysisToUpdate<ImmediateDominators>()) {
> -    // TIBB is the new immediate dominator for NewBB.
> -    ID->addNewBlock(NewBB, TIBB);
> -    
> -    // If NewBBDominatesDestBB hasn't been computed yet, do so with ID.
> -    if (!OtherPreds.empty()) {
> -      while (!OtherPreds.empty() && NewBBDominatesDestBB) {
> -        NewBBDominatesDestBB = ID->dominates(DestBB, OtherPreds.back());
> -        OtherPreds.pop_back();
> +    if (ID->get(TIBB)) {  // Only do this if TIBB is reachable.
> +      // TIBB is the new immediate dominator for NewBB.
> +      ID->addNewBlock(NewBB, TIBB);
> +      
> +      // If NewBBDominatesDestBB hasn't been computed yet, do so with ID.
> +      if (!OtherPreds.empty()) {
> +        while (!OtherPreds.empty() && NewBBDominatesDestBB) {
> +          NewBBDominatesDestBB = ID->dominates(DestBB, OtherPreds.back());
> +          OtherPreds.pop_back();
> +        }
> +        OtherPreds.clear();
>        }
> -      OtherPreds.clear();
> +      
> +      // If NewBBDominatesDestBB, then NewBB dominates DestBB, otherwise it
> +      // doesn't dominate anything.
> +      if (NewBBDominatesDestBB)
> +        ID->setImmediateDominator(DestBB, NewBB);
>      }
> -    
> -    // If NewBBDominatesDestBB, then NewBB dominates DestBB, otherwise it
> -    // doesn't dominate anything.
> -    if (NewBBDominatesDestBB)
> -      ID->setImmediateDominator(DestBB, NewBB);
>    }
>  
>    // Update the forest?
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits