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

[Chris Lattner]

On Feb 14, 2011, at 5:47 AM, Duncan Sands wrote:

>>> 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.
> 
> it seems dangerous to erase phi's when explicitly told not to - it creates the
> danger of the caller using freed memory and so on.  Instead, how about having
> removePredecessor always do a RAUW of degenerate phis with undef, but not
> actually erase them if DontDeleteUselessPHIs was passed.

That sounds like it is just a matter of updating the comment on the method, and checking the clients to see who is doing this (fixing it as appropriate).

-Chris