[patch] split FRAME variables back into pieces

[Richard Guenther]

On Wed, Sep 19, 2012 at 12:58 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this transformation has been in our tree for a couple of years and was
> originally developed for SPARKSkein (http://www.skein-hash.info/node/48),
> which is the implementation in SPARK (subset of Ada) of the Skein algorithm.
>
> When nested functions access local variables of their parent, the compiler
> creates a special FRAME local variable in the parent, which represents the
> non-local frame, and puts into it all the variables accessed non-locally.
>
> If these nested functions are later inlined into their parent, these FRAME
> variables generally remain unmodified and this has various drawbacks:
>  1) the frame of the parent is unnecessarily large,
>  2) scalarization of aggregates put into the FRAME variables is hindered,
>  3) debug info for scalars put into the FRAME variables is poor since VTA only
> works on GIMPLE registers.
>
> The attached patch makes it so that the compiler splits FRAME variables back
> into pieces when all the nested functions have been inlined.  The natural
> place to implement the transformation would probably be the SRA pass, but this
> would require a special path to work around all the heuristics and the pass is
> already complicated enough (sorry Martin ;-)  The transformation is therefore
> implemented as a sub-pass of execute_update_addresses_taken for technical
> reasons exposed in the patch.
>
> Tested on x86-64/Linux, OK for the mainline?

I really don't like this to be done outside of SRA (and it is written in a
non-MEM_REF way).  For the testcase in question we scalarize back
'i' in SRA (other scalars are optimized away already, but as SRA runs
before DSE it still gets to see stores to FRAME.i).  Now I wonder
why we generate reasonable debug info even without inlining,
thus there has to be a association to the original decls with
the frame FIELD_DECLs.  That is, lookup_decl_for_field should not be necessary
and what we use for debug info generation should be used by SRA
to assign a name to scalarized fields.

That alone would not solve your issue because of the 'arr' field in
the structure which cannot be scalarized (moved to a stand-alone
decl) by SRA.  That's one missed feature of SRA though, and generally
useful.

So no, I don't think this patch is the right approach.

Thanks,
Richard.



>
> 2012-09-19  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * tree.h (DECL_NONLOCAL_FRAME): New macro.
>         * gimple.c (gimple_ior_addresses_taken_1): Handle non-local frame
>         structures specially.
>         * tree-nested.c (get_frame_type): Set DECL_NONLOCAL_FRAME.
>         * tree-ssa.c (lookup_decl_for_field): New static function.
>         (split_nonlocal_frames_op): Likewise.
>         (execute_update_addresses_taken): Break up non-local frame structures
>         into variables when possible.
>         * tree-streamer-in.c (unpack_ts_decl_common_value_fields): Stream in
>         DECL_NONLOCAL_FRAME flag.
>         * tree-streamer-out.c (pack_ts_decl_common_value_fields): Stream out
>         DECL_NONLOCAL_FRAME flag.
>
>
> 2012-09-19  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gcc.dg/nested-func-9.c: New test.
>
>
> --
> Eric Botcazou