[llvm-commits] [llvm] r125319 - in /llvm/trunk: lib/Analysis/ValueTracking.cpp test/Transforms/JumpThreading/degenerate-phi.ll

[Nick Lewycky]

Duncan Sands wrote:
> Hi Nick,
>
>> --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
>> +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Thu Feb 10 17:54:10 2011
>> @@ -590,6 +590,10 @@
>>          }
>>        }
>>
>> +    // Unreachable blocks may have zero-operand PHI nodes.
>> +    if (P->getNumIncomingValues() == 0)
>> +      return;
>
> the LangRef says that zero operand phis are not allowed.  I'm pretty sure that
> there is some code in InstructionSimplify (which is calling ValueTracking here)
> which assumes that phi nodes have at least one operand.  Who knows where else
> this assumption is made?  I think it would be better to have jump-threading zap
> any zero operand phis when it makes them, rather than changing all analyses to
> support them.

It doesn't create them, it calls BasicBlock::removePredecessor.

This code already has to handle the case where you make a query about a 
value and one of the possible inputs (say, through a select) comes from 
an unreachable block which contains a "%x = add %x, %x" instruction.

Nick

>
> Ciao, Duncan.
>
>> +
>>        // Otherwise take the unions of the known bit sets of the operands,
>>        // taking conservative care to avoid excessive recursion.
>>        if (Depth<   MaxDepth - 1&&   !KnownZero&&   !KnownOne) {
>>
>> Added: llvm/trunk/test/Transforms/JumpThreading/degenerate-phi.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/degenerate-phi.ll?rev=125319&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/JumpThreading/degenerate-phi.ll (added)
>> +++ llvm/trunk/test/Transforms/JumpThreading/degenerate-phi.ll Thu Feb 10 17:54:10 2011
>> @@ -0,0 +1,24 @@
>> +; RUN: opt -jump-threading -disable-output %s
>> +; PR9112
>> +
>> +; This is actually a test for value tracking. Jump threading produces
>> +; "%phi = phi i16" when it removes all edges leading to %unreachable.
>> +; The .ll parser won't let us write that directly since it's invalid code.
>> +
>> +define void @func() nounwind {
>> +entry:
>> +  br label %bb
>> +
>> +bb:
>> +  br label %bb
>> +
>> +unreachable:
>> +  %phi = phi i16 [ %add, %unreachable ], [ 0, %next ]
>> +  %add = add i16 0, %phi
>> +  %cmp = icmp slt i16 %phi, 0
>> +  br i1 %cmp, label %unreachable, label %next
>> +
>> +next:
>> +  br label %unreachable
>> +}
>> +
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>