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

[Duncan Sands]

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.

clearly it must clean them up somewhere (since phi nodes with no operands are
invalid), so can't it just clean them up earlier?

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

The big difference is that unreachable blocks are not invalid according to the
LangRef.

Ciao, Duncan.

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