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

[Chris Lattner]

On Feb 11, 2011, at 3:24 PM, Frits van Bommel wrote:

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

I agree, that does seem like the right fix.

-Chris