Re: [PATCH] Teach SCCVN/PRE about type-punning through unions

["Daniel Berlin" <dberlin at dberlin dot org>]

On Mon, Mar 3, 2008 at 7:25 AM, Richard Guenther <rguenther@suse.de> wrote:
>
>  This patch is an alternate implementation to
>  http://gcc.gnu.org/ml/gcc-patches/2008-02/msg01422.html
>  to optimize type-punning operations through unions to use scalar
>  operations (aka VIEW_CONVERT_EXPR).
>
>  It does so by doing (pseudo-)insertion of new expressions with an
>  associated SSA_NAME (to hold its value number) during the final
>  SCC iteration.  At PRE/FRE elimination time these expressions are
>  inserted in place of the SSA_NAME (that is, no real insertion takes
>  place, but we replace with an expression instead of an SSA_NAME),
>  properly re-constructed with using expression leaders for its
>  operands (where available).
>
>  Variants of this approach would be to add the fake SSA_NAMEs
>  during regular SCC iteration, but then also to the SCC itself
>  (which was too tricky for me, thus only doing that at the final
>  iteration).
>
>  On the PRE/FRE side a new insertion phase after SCCVN could insert
>  the expressions explicitly (like we insert fake stores);  I don't
>  know if this would fix anything or make anything easier at the
>  point of insertion though (at least we would only insert a replacement
>  expression once).  The problem with availability we face is that for
>
>  union { int i; float f; } u;
>
>   # VUSE <u_1>
>   tmp_2 = u.i;
>   # VUSE <u_1>
>   tmp_1 = u.f;
>
>  SCCVN (randomly?) processes the SCCs for tmp_1 and tmp_2 and thus
>  can end up value-numbering tmp_2 as VIEW_CONVERT_EXPR <tmp_1> which
>  obviously would violate SSA form.

It should be sorting them into RPO so that we process tmp_2 before tmp_1.

Is this not happening?
If so, we should fix it :)

>  Danny, do you have any idea to the above problem?  (Other than
>  unconditionally inserting expressions for VIEW_CONVERT_EXPR to
>  any other union member even before running SCCVN)
>
>  But, the following was bootstrapped and tested on x86_64-unknown-linux-gnu
>  and fixes PRs 34043 and 33989 (and maybe others), like the other
>  patch referenced above.
>
>  Shall we just pick one or are there suggestions for improvements
>  for either approaches?

Would fixing the SCC sort order fix your problem with tmp_1 vs tmp_2?
If so, we should just go with this approach.

If I work up the time, I will give rewriting PRE a chance so that we
collect expressions something like SSAPRE does, and calculate
ANTIC/AVAIL sparsely.