Re: [PATCH] Fix PR42717

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Jan 20, 2010 at 4:26 AM, Richard Guenther <rguenther@suse.de> wrote:
> On Wed, 20 Jan 2010, Jan Hubicka wrote:
>
>> > On Sat, 16 Jan 2010, Richard Guenther wrote:
>> >
>> > >
>> > > This fixes PR42717 where we run into issues with the PHI updating
>> > > of CDDCE dead region (aka dead control statement) removal.
>> > >
>> > > The core issue with the PR in question is that we pick the wrong
>> > > edge to copy PHI args from as the existing dominance check is
>> > > not strict enough to identify an edge from the dead region.
>> > > Fixing this opens up a can of worms.
>> > >
>> > > First we do not, even less so
>> > > after lxo came along, remove dead control statements in dominator
>> > > order. ?This means we do not remove the most dominating dead
>> > > control statement first but may start in the middle of a dead
>> > > region. ?That is at least inefficient but also leads to problems
>> > > with the proposed fix as we do not have dominator information
>> > > updated as we redirect edges (and it's non-trivial to do so).
>> > >
>> > > Second, the optimization of threading to the next _live_
>> > > post-dominator doesn't work as we come along totally unrelated
>> > > PHIs and the choice which edge to copy PHI args from is more
>> > > or less not solvable in general.
>> > >
>> > > Third with the patch we need to avoid visiting blocks in regions
>> > > we cut off again. ?The simple approach would be to delete them,
>> > > but that comes in the way of debug statements. ?Ugh. ?So I
>> > > have to re-compute block reachability after each region "removal".
>> >
>> > This is an easier variant - fixing 2) is the only thing required
>> > as in that case the PHI arg updating gets trivial.
>> >
>> > Bootstrapped and tested on x86_64-unknown-linux-gnu. ?Honza, what
>> > was the reason for threading to the next _live_ postdom?
>>
>> That was what textbook says ;)
>> Originally code was simply not updating CFG in non-trivial cases. ?This
>> does not work, since we remove the conditional and then we eneded up
>> chosing random edge out of the block often turning finite loops into
>> infinite.
>> I guess if you replace every dead conditional by edge to post dominator,
>> then the dead regions will turn into acyclic blocks so later jump
>> threading will cure rest of updates?
>
> I think cfg-cleanup will take care of whatever is left to do.
>
>> As for orignial algorithm, I believe problem there was that we looked
>> for BB dominating the edge assumign that it is the block having control
>> dependency. ?It is possible to construct valid CFG where there are
>> multiple blocks dominating the edge, but only one of them is CD block.
>> It is always the lowest one in the chain. ?I will try to thinking about
>> this bit more, but I don't see problem with your code and I guess it is
>> overall easier.
>
> Indeed.
>
> Thus, committed.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43367


-- 
H.J.