Re: RFA: New optimization: Convert local statics into non-statics

["Richard Guenther" <richard dot guenther at gmail dot com>]

On Mon, Jul 14, 2008 at 5:08 PM, Nick Clifton <nickc@redhat.com> wrote:
> Hi Guys,
>
>  I would like to offer the attached patch consideration and I hope,
>  approval to check into gcc.  It implements a new optimization that
>  converts static variables local to a function into their non-static
>  equivalents.  So for example it would convert this:
>
>    int foo (void) { static int a = 1; return a; }
>
>  into this:
>
>    int foo (void) { int a = 1; return a; }
>
>  The conversion is only done when it can be shown that it is safe to
>  do so, which in practice means only when the variable is initialized
>  before it is used.  It is only performed for simple types, so arrays
>  and structures are not affected, and there is a new param provided
>  to stop the optimization from triggering on functions which have
>  lots of local variables already and adding some more would increase
>  the pressure on the stack too much.
>
>  The benefit of the optimization is that it allows gcc the chance to
>  decide that the the converted variable can live entirely in a
>  register.  Plus even if the variable is pushed onto the stack, it is
>  often the case that accessing variables there is faster than
>  accessing variables in global memory.
>
>  The optimization could be performed by hand by editing the source
>  code before compiling the program, but this option is not always
>  available to the user.  (For example this optimization was motivated
>  by the desire to improve the results for a well known benchmark.
>  Since the benchmark code could not be altered, the optimization was
>  added to the compiler).
>
>  I have not added the optimization to those enabled by -O2 (or -O3)
>  because:
>
>    a) I do not know the current policy for deciding when an
>    optimization is enabled by a given -Ox level.

This sounds like something for -O2 or at least -O3.  We should not
add new optimization passes that are not enabled by default on
some level because ...

>    b) The optimization is new and unproven in the wide world of gcc
>    users.  (Although we have been using it internally at Red Hat for
>    a while and it has proved to be quite effective).

... without enabling it it will bit-rot quickly.  Can you say something
about "proved to be quite effective"?  Personally I would have estimated
this to nearly never trigger.  You can use the statistics infrastructure
to get some data on this.

>    c) There are some cases where the optimization results in worse
>    performance for the compiled program.  (Usually when too many
>    local variables end up on the stack).

Which means it is probably good for -O3.  Did you bootstrap with
the pass enables?

>  The patch includes a new gcc testcase, documentation and an update
>  to the changes.html file for gcc 4.4.  Tested without any
>  regressions with an x86 native toolchain.
>
>  Please may I apply this patch ?

Some functions in the new pass need documentation of their parameters,
their return value and their function.

Instead of implementing a linked list for pending transformations
have you considered simply using a VEC?

+               if (dump_file && (dump_flags & TDF_DETAILS))
+                 fprintf (dump_file, "\
+ '%s' not converted because DEF in block %d did not dominate USE in
block %d.\n",
+                          lang_hooks.decl_printable_name (var, 0),

the style is to use

           fprintf (dump_file, "'%s' not converted because DEF "
                     " in block %d did not dominate US...

also don't use FALSE and TRUE, but false and true.

+   return
+     decl != NULL

use parens to force correct indent.

+     /* That has storeage allocated in this

storage

+    2- There exists a definition for V that post-dominates the entry block
+       (V's initialization is not considered a definition).

why?  It's initialization (DECL_INITIAL I assume) can be converted to
an assignment in the entry BB.  I believe this may be done by the
gimplifier already.

I don't see how you deal with the address of the local static escaping
such as in

int *foo(void)
{
  static int x;
  x = 0;
  bar();
  return &x;
}

Also as a general remark I think this functionality is better done in
the existing ipa-reference pass.

Richard.

> Cheers
>  Nick
>
> gcc/ChangeLog
> 2008-07-14  Nick Clifton  <nickc@redhat.com>
>
>        * tree-optimize-local-statics.c: New file.  New optimization.
>        Converts statics local to a function into non-statics.
>        * tree-pass.h: Prototype pass_optimize_local_statics.
>        * passes.c (init_optimization_passes): Add
>        pass_optimize_local_statics.
>        * common.opt (foptimize-local-statics): Add new command line
>        option.
>        * params.def (slo-local-variables-threshold): Define.
>        * Makefile.in (OBJS-common): Add tree-optimize-local-statics.
>        Add rule to compile tree-optimize-local-statics.c.
>        * doc/invoke.texi: Document new command line option and param.
>
> gcc/testsuite/ChangeLog
> 2008-07-14  Nick Clifton  <nickc@redhat.com>
>
>        * gcc.dg/tree-ssa/static-local.c: New file.  Test the local
>        statics optimization.
>
>