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

[Chris Lattner]

already fixed:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070108/042663.html

Thanks,

-Chris

On Mon, 15 Jan 2007, Reid Spencer wrote:

> 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
>

-Chris

-- 
http://nondot.org/sabre/
http://llvm.org/