Re: [PATCH] Don't forget to scan TREE_STATIC VAR_DECL DECL_INITIAL for other referenced vars during inlining (PR middle-end/39360)

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

On Fri, Mar 6, 2009 at 9:42 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> Since
> 2009-02-28 ?Jan Hubicka ?<jh@suse.cz>
> ? ? ? ?* tree-inline.c (expand_call_inline): Avoid duplicate declarations of
> ? ? ? ?static vars.
> ? ? ? ?(copy_arguments_for_versioning): If var is declared don't declare it.
> ? ? ? ?(tree_function_versioning): First setup substitutions and then copy
> ? ? ? ?args.
> the following testcase fails. ?The problem is when expand_call_inline
> adds static vars as referenced using referenced_var_check_and_insert,
> when add_referenced_var is called on it, it does nothing, as it is already
> marked as referenced. ?That means
> ? ? ?/* Scan DECL_INITIAL for pointer variables as they may contain
> ? ? ? ? address arithmetic referencing the address of other
> ? ? ? ? variables.
> ? ? ? ? Even non-constant initializers need to be walked, because
> ? ? ? ? IPA passes might prove that their are invariant later on. ?*/
> ? ? ?if (DECL_INITIAL (var)
> ? ? ? ? ?/* Initializers of external variables are not useful to the
> ? ? ? ? ? ? optimizers. ?*/
> ? ? ? ? ?&& !DECL_EXTERNAL (var))
> ? ? ? ?walk_tree (&DECL_INITIAL (var), find_vars_r, NULL, 0);
> is never done on the static var and so variables only referenced through
> such initializers aren't marked as referenced.
>
> The patch below fixes it by calling add_referenced_var instead and extending
> it, so that it returns whether it has been actually newly added as
> referenced or not.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

With alias-improvements branch we could finally get rid of the quadratic
(if you have lots of functions) walking of the DECL_INITIALs and add
these referenced vars on-demand.  I have some old patch for this that
shaves off ~50% compile-time on a testcase (I don't remember the PR).

Thanks,
Richard.

> 2009-03-06 ?Jakub Jelinek ?<jakub@redhat.com>
>
> ? ? ? ?PR middle-end/39360
> ? ? ? ?* tree-flow.h (add_referenced_var): Return bool instead of void.
> ? ? ? ?* tree-dfa.c (add_referenced_var): Return result of
> ? ? ? ?referenced_var_check_and_insert call.
> ? ? ? ?* tree-inline.c (expand_call_inline): Call add_referenced_var instead
> ? ? ? ?of referenced_var_check_and_insert.
>
> ? ? ? ?* gcc.c-torture/compile/pr39360.c: New test.
>
> --- gcc/tree-flow.h.jj ?2009-03-04 20:06:31.000000000 +0100
> +++ gcc/tree-flow.h ? ? 2009-03-06 20:01:19.000000000 +0100
> @@ -768,7 +768,7 @@ extern void dump_referenced_vars (FILE *
> ?extern void dump_variable (FILE *, tree);
> ?extern void debug_variable (tree);
> ?extern tree get_virtual_var (tree);
> -extern void add_referenced_var (tree);
> +extern bool add_referenced_var (tree);
> ?extern void remove_referenced_var (tree);
> ?extern void mark_symbols_for_renaming (gimple);
> ?extern void find_new_referenced_vars (gimple);
> --- gcc/tree-dfa.c.jj ? 2008-10-23 13:21:40.000000000 +0200
> +++ gcc/tree-dfa.c ? ? ?2009-03-06 20:02:35.000000000 +0100
> @@ -1,6 +1,6 @@
> ?/* Data flow functions for trees.
> - ? Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008 Free Software
> - ? Foundation, Inc.
> + ? Copyright (C) 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009
> + ? Free Software Foundation, Inc.
> ? ?Contributed by Diego Novillo <dnovillo@redhat.com>
>
> ?This file is part of GCC.
> @@ -639,7 +639,7 @@ set_default_def (tree var, tree def)
>
> ?/* Add VAR to the list of referenced variables if it isn't already there. ?*/
>
> -void
> +bool
> ?add_referenced_var (tree var)
> ?{
> ? var_ann_t v_ann;
> @@ -655,7 +655,7 @@ add_referenced_var (tree var)
>
> ? ? ? /* Tag's don't have DECL_INITIAL. ?*/
> ? ? ? if (MTAG_P (var))
> - ? ? ? return;
> + ? ? ? return true;
>
> ? ? ? /* Scan DECL_INITIAL for pointer variables as they may contain
> ? ? ? ? address arithmetic referencing the address of other
> @@ -667,7 +667,11 @@ add_referenced_var (tree var)
> ? ? ? ? ? ? optimizers. ?*/
> ? ? ? ? ? && !DECL_EXTERNAL (var))
> ? ? ? ?walk_tree (&DECL_INITIAL (var), find_vars_r, NULL, 0);
> +
> + ? ? ?return true;
> ? ? }
> +
> + ?return false;
> ?}
>
> ?/* Remove VAR from the list. ?*/
> --- gcc/tree-inline.c.jj ? ? ? ?2009-03-04 20:06:31.000000000 +0100
> +++ gcc/tree-inline.c ? 2009-03-06 20:03:14.000000000 +0100
> @@ -1,5 +1,5 @@
> ?/* Tree inlining.
> - ? Copyright 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008
> + ? Copyright 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009
> ? ?Free Software Foundation, Inc.
> ? ?Contributed by Alexandre Oliva <aoliva@redhat.com>
>
> @@ -3383,7 +3383,7 @@ expand_call_inline (basic_block bb, gimp
> ? ? ? var = TREE_VALUE (t_step);
> ? ? ? if (TREE_STATIC (var) && !TREE_ASM_WRITTEN (var))
> ? ? ? ?{
> - ? ? ? ? if (var_ann (var) && referenced_var_check_and_insert (var))
> + ? ? ? ? if (var_ann (var) && add_referenced_var (var))
> ? ? ? ? ? ?cfun->local_decls = tree_cons (NULL_TREE, var,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cfun->local_decls);
> ? ? ? ?}
> --- gcc/testsuite/gcc.c-torture/compile/pr39360.c.jj ? ?2009-03-06 20:06:04.000000000 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr39360.c ? ? ? 2009-03-06 20:05:46.000000000 +0100
> @@ -0,0 +1,16 @@
> +/* PR middle-end/39360 */
> +
> +static int a[] = { 1 };
> +
> +static inline void
> +bar (int **x)
> +{
> + ?static int *c[2] = { 0, a };
> + ?*x = c[1];
> +}
> +
> +int
> +foo (int **x)
> +{
> + ?bar (x);
> +}
>
> ? ? ? ?Jakub
>