[llvm-commits] [llvm] r132022 - /llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp

[Cameron Zwarich]

On 2011-05-25, at 1:45 AM, Cameron Zwarich wrote:

> On 2011-05-24, at 4:12 PM, Evan Cheng wrote:
> 
>> @@ -910,12 +916,27 @@
>>    // successor if they become single-entry, those PHI nodes may
>>    // be in the Users list.
>> 
>> +    BasicBlock *Switch = SI->getParent();
>> +    BasicBlock *SISucc = SI->getSuccessor(DeadCase);
>> +    BasicBlock *Latch = L->getLoopLatch();
>> +    // If the DeadCase successor dominates all of the predecessors of the
>> +    // loop latch, then the transformation isn't safe since it will delete
>> +    // the predecessor edges to the latch.
>> +    if (Latch) {
>> +      bool DominateAll = true;
>> +      for (pred_iterator PI = pred_begin(Latch), PE = pred_end(Latch);
>> +           DominateAll && PI != PE; ++PI)
>> +        if (!DT->dominates(SISucc, *PI))
>> +          DominateAll = false;
>> +      if (DominateAll)
>> +        continue;
>> +    }
> 
> You only need to check whether SISucc dominates Latch here. Also, you should be change this check in RemoveBlockIfDead to be a dominance check rather than an equality check and avoid skipping the optimization:
> 
>  // If this is the edge to the header block for a loop, remove the loop and
>  // promote all subloops.
>  if (Loop *BBLoop = LI->getLoopFor(BB)) {
>    if (BBLoop->getLoopLatch() == BB)
>      RemoveLoopFromHierarchy(BBLoop);
>  }

Actually, from reading the code it should be removing dead nodes until it gets to the latch, and then removing the containing loop, so obviously there is some other bug here that makes this not work properly. Sorry for the noise.

Cameron