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

[Frits van Bommel]

On Fri, Feb 11, 2011 at 9:28 AM, Duncan Sands <baldrick at free.fr> wrote:
>>> 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?

It looks to me like the problem is that removePredecessor() respects
the 'DontDeleteUselessPHIs' parameter even when the only predecessor
is removed, and that JumpThreading always passes 'true' for that (even
though AFAICT it doesn't have a good reason to).

The best fix is probably to ignore that parameter if it's removing the
last predecessor; IMHO in this case it should just replace all PHIs
with undefs and then kill them, regardless of that parameter.