Re: [PR middle-end/78566] Fix uninit regressions caused by previous -Wmaybe-uninit change

[Aldy Hernandez <aldyh at redhat dot com>]

On 12/05/2016 11:07 PM, Jeff Law wrote:
> On 11/29/2016 09:33 AM, Aldy Hernandez wrote:
> > This fixes the gcc.dg/uninit-pred-6* failures I seem to have caused on
> > some non x86 platforms. Sorry for the delay.
> >
> > The problem is that my fix for PR61409 had the logic backwards.  I was
> > proving that all the uses of a PHI are invalidated by any one undefined
> > PHI path, whereas what we want is to prove that EVERY uninitialized path
> > is invalidated by some facor in the PHI use.
> >
> > The attached patch fixes this without causing any regressions on x86-64
> > Linux.  I also verified that at least on [arm-none-linux-gnueabihf
> > --with-cpu=cortex-a5 --with-fpu=vfpv3-d16-fp16], there are no
> > gcc.dg/*uninit* regressions.
> >
> > There is still one regression at large involving a double free in
> > PR78548 which I will look at next/independently.
> >
> > OK for trunk?
> > Aldy
> >
> > curr
> >
> >
> > commit 469f4c38a48bc284c268b40f5d5511f015844ea2
> > Author: Aldy Hernandez <aldyh@redhat.com>
> > Date:   Tue Nov 29 05:59:53 2016 -0500
> >
> >             PR middle-end/78566
> >             * tree-ssa-uninit.c (can_one_predicate_be_invalidated_p):
> > Change
> >             argument type to a pred_chain.
> >             (can_chain_union_be_invalidated_p): Use pred_chain instead
> > of a
> >             worklist.
> >             (flatten_out_predicate_chains): Remove.
> >             (uninit_uses_cannot_happen): Rename from
> >             uninit_ops_invalidate_phi_use.
> >             Change logic so that we are checking that the PHI use will
> >             invalidate _ALL_ possibly uninitialized operands.
> >             (is_use_properly_guarded): Rename call to
> >             uninit_ops_invalidate_phi_use into uninit_uses_cannot_happen.
> This walk through is mostly for the historical record as I suspect we'll
> be in here again in the future...  And it helps me organize my own
> thoughts as I walk through the code.  I can't keep it all in my head :-)

Woah, I should've hired you to write the comments, cause I had a 
horrible time documenting the code (as you can tell).

> Anyway, so you have a set of PHI args that are potentially undefined.
> Those args (of course) are associated with control paths through the CFG.
>
> You also have a set of uses of the PHI which are themselves associated
> with control paths through the CFG.
>
> If none of the uninitialized PHI arguments can flow to the PHI uses,
> then no warning needs to be emitted as the path in question can not
> occur at runtime (and one might argue we want to mark that path for
> potential isolation/optimization or for other "may-be" analysis).
>
> find_uninit_use iterates over the uses of the PHI result and calls
> is_use_properly_guarded for each.
>
> is_use_properly_guarded will call uninit_uses_cannot_happen to see if
> the given use can or can not be reached by paths including the
> uninitialized PHI arguments.
>
> uninit_uses_cannot_happen builds the predicate for each uninitialized
> PHI argument then uses can_chain_union_be_invalidated to see if the
> argument's predicate ensures the use can not be reached.  If that is
> true for all the PHI args, then we return true, false otherwise.
>
> can_chain_union_be_invalidated invalidates things one predicate at a
> time using can_one_predicate_be_invalidated.  If all can be invalidated,
> then it returns true, false otherwise.

Yes to all the above.

> OK.

I will close this PR and 78548 whose approved patch depends on this one.

Thank you.