Re: [PATCH] ICF: move readonly decision for variables to the right place

["Martin Liška" <mliska@suse.cz>]

On 03/02/2015 08:53 AM, Jan Hubicka wrote:
> Hi,
> this is a variant of patch I commited.
> It takes care to load the constructor to memory in sem_variable::equals
> and donot touch it earlier.  I also made sem_variable::parse to skip volatile
> and reigster variables.
>
> Bootstrapped/regtested x86_64-linux, lto-bootstrapped with -fmerge-all-constants
> comitted.
>
> Honza

Thank you Honza for patch improvement and new test cases you added.

Martin

>
> 2015-02-28  Martin Liska  <mliska@suse.cz>
> 	    Jan Hubicka   <hubicka@ucw.cz>
>
> 	* ipa-icf.c (sem_variable::equals): Improve debug output;
> 	get variable constructor.
> 	(sem_variable::parse): Do not filter out too early; give up on
> 	volatile and register vars.
> 	(sem_item_optimizer::filter_removed_items): Filter out nonreadonly
> 	variables.
> 	* ipa-icf.h (sem_variable::init): Do not set ctor.
> 	(sem_variable::ctor): Remove.
>
> gcc/testsuite/ChangeLog:
>
> 2015-02-28  Martin Liska  <mliska@suse.cz>
> 	    Jan Hubicka   <hubicka@ucw.cz>
>
> 	* gcc.dg/ipa/ipa-icf-35.c: New test.
> 	* gcc.dg/ipa/ipa-icf-36.c: New test.
> 	* gcc.dg/ipa/ipa-icf-37.c: New test.
> Index: ipa-icf.c
> ===================================================================
> --- ipa-icf.c	(revision 221096)
> +++ ipa-icf.c	(working copy)
> @@ -1448,18 +1452,29 @@ sem_variable::equals_wpa (sem_item *item
>
>   /* Returns true if the item equals to ITEM given as argument.  */
>
> +/* Returns true if the item equals to ITEM given as argument.  */
> +
>   bool
>   sem_variable::equals (sem_item *item,
> -		      hash_map <symtab_node *, sem_item *> & ARG_UNUSED (ignored_nodes))
> +		      hash_map <symtab_node *, sem_item *> &)
>   {
>     gcc_assert (item->type == VAR);
> +  bool ret;
>
> -  sem_variable *v = static_cast<sem_variable *>(item);
> -
> -  if (!ctor || !v->ctor)
> -    return return_false_with_msg ("ctor is missing for semantic variable");
> +  if (DECL_INITIAL (decl) == error_mark_node && in_lto_p)
> +    dyn_cast <varpool_node *>(node)->get_constructor ();
> +  if (DECL_INITIAL (item->decl) == error_mark_node && in_lto_p)
> +    dyn_cast <varpool_node *>(item->node)->get_constructor ();
> +
> +  ret = sem_variable::equals (DECL_INITIAL (decl),
> +			      DECL_INITIAL (item->node->decl));
> +  if (dump_file && (dump_flags & TDF_DETAILS))
> +    fprintf (dump_file,
> +	     "Equals called for vars:%s:%s (%u:%u) (%s:%s) with result: %s\n\n",
> +	     name(), item->name (), node->order, item->node->order, asm_name (),
> +	     item->asm_name (), ret ? "true" : "false");
>
> -  return sem_variable::equals (ctor, v->ctor);
> +  return ret;
>   }
>
>   /* Compares trees T1 and T2 for semantic equality.  */
> @@ -1653,24 +1668,7 @@ sem_variable::equals (tree t1, tree t2)
>   sem_variable *
>   sem_variable::parse (varpool_node *node, bitmap_obstack *stack)
>   {
> -  tree decl = node->decl;
> -
> -  if (node->alias)
> -    return NULL;
> -
> -  bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl);
> -  if (!readonly)
> -    return NULL;
> -
> -  bool can_handle = DECL_VIRTUAL_P (decl)
> -		    || flag_merge_constants >= 2
> -		    || (!TREE_ADDRESSABLE (decl) && !node->externally_visible);
> -
> -  if (!can_handle || DECL_EXTERNAL (decl))
> -    return NULL;
> -
> -  tree ctor = ctor_for_folding (decl);
> -  if (!ctor)
> +  if (TREE_THIS_VOLATILE (node->decl) || DECL_HARD_REGISTER (node->decl))
>       return NULL;
>
>     sem_variable *v = new sem_variable (node, 0, stack);
> @@ -1686,8 +1684,8 @@ hashval_t
>   sem_variable::get_hash (void)
>   {
>     if (hash)
> -    return hash;
>
> +    return hash;
>     /* All WPA streamed in symbols should have their hashes computed at compile
>        time.  At this point, the constructor may not be in memory at all.
>        DECL_INITIAL (decl) would be error_mark_node in that case.  */
> @@ -2155,7 +2153,14 @@ sem_item_optimizer::filter_removed_items
>   	  if (!flag_ipa_icf_variables)
>   	    remove_item (item);
>   	  else
> -	    filtered.safe_push (item);
> +	    {
> +	      /* Filter out non-readonly variables.  */
> +	      tree decl = item->decl;
> +	      if (TREE_READONLY (decl))
> +		filtered.safe_push (item);
> +	      else
> +		remove_item (item);
> +	    }
>           }
>       }
>
> Index: ipa-icf.h
> ===================================================================
> --- ipa-icf.h	(revision 221096)
> +++ ipa-icf.h	(working copy)
> @@ -393,7 +393,6 @@ public:
>     inline virtual void init (void)
>     {
>       decl = get_node ()->decl;
> -    ctor = ctor_for_folding (decl);
>     }
>
>     virtual hashval_t get_hash (void);
> @@ -415,9 +414,6 @@ public:
>     /* Parser function that visits a varpool NODE.  */
>     static sem_variable *parse (varpool_node *node, bitmap_obstack *stack);
>
> -  /* Variable constructor.  */
> -  tree ctor;
> -
>   private:
>     /* Iterates though a constructor and identifies tree references
>        we are interested in semantic function equality.  */
> Index: testsuite/gcc.dg/ipa/ipa-icf-35.c
> ===================================================================
> --- testsuite/gcc.dg/ipa/ipa-icf-35.c	(revision 0)
> +++ testsuite/gcc.dg/ipa/ipa-icf-35.c	(revision 0)
> @@ -0,0 +1,31 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-ipa-icf"  } */
> +
> +void f1()
> +{
> +}
> +
> +void f2()
> +{
> +}
> +
> +static void (*a)(void)=&f1;
> +static void (*b)(void)=&f1;
> +static void (*c)(void)=&f2;
> +static void (*d)(void)=&f2;
> +
> +int main()
> +{
> +  a();
> +  b();
> +  c();
> +  d();
> +
> +  return 0;
> +}
> +
> +/* { dg-final { scan-ipa-dump "Equal symbols: 3" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:f2->f1" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:d->c" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:b->a" "icf"  } } */
> +/* { dg-final { cleanup-ipa-dump "icf" } } */
> Index: testsuite/gcc.dg/ipa/ipa-icf-36.c
> ===================================================================
> --- testsuite/gcc.dg/ipa/ipa-icf-36.c	(revision 0)
> +++ testsuite/gcc.dg/ipa/ipa-icf-36.c	(revision 0)
> @@ -0,0 +1,36 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-ipa-icf -fmerge-all-constants"  } */
> +static int a;
> +static int b;
> +static const int c = 2;
> +static const int d = 2;
> +static char * e = "test";
> +static char * f = "test";
> +static int g[3]={1,2,3};
> +static int h[3]={1,2,3};
> +static const int *i=&c;
> +static const int *j=&c;
> +static const int *k=&d;
> +int t(int tt)
> +{
> +  switch (tt)
> +  {
> +    case 1: return a;
> +    case 2: return b;
> +    case 3: return c;
> +    case 4: return d;
> +    case 5: return e[1];
> +    case 6: return f[1];
> +    case 7: return g[1];
> +    case 8: return h[1];
> +    case 9: return i[0];
> +    case 10: return j[0];
> +    case 11: return k[0];
> +  }
> +}
> +/* { dg-final { scan-ipa-dump "Equal symbols: 6" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:b->a" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:d->c" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:f->e" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:h->g" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:k->i" "icf"  } } */
> Index: testsuite/gcc.dg/ipa/ipa-icf-37.c
> ===================================================================
> --- testsuite/gcc.dg/ipa/ipa-icf-37.c	(revision 0)
> +++ testsuite/gcc.dg/ipa/ipa-icf-37.c	(revision 0)
> @@ -0,0 +1,36 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-ipa-icf"  } */
> +static int a;
> +static int b;
> +static const int c = 2;
> +static const int d = 2;
> +static char * e = "test";
> +static char * f = "test";
> +static int g[3]={1,2,3};
> +static int h[3]={1,2,3};
> +static const int *i=&c;
> +static const int *j=&c;
> +static const int *k=&d;
> +int t(int tt)
> +{
> +  switch (tt)
> +  {
> +    case 1: return a;
> +    case 2: return b;
> +    case 3: return c;
> +    case 4: return d;
> +    case 5: return e[1];
> +    case 6: return f[1];
> +    case 7: return g[1];
> +    case 8: return h[1];
> +    case 9: return i[0];
> +    case 10: return j[0];
> +    case 11: return k[0];
> +  }
> +}
> +/* { dg-final { scan-ipa-dump "Equal symbols: 5" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:b->a" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:d->c" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:f->e" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:h->g" "icf"  } } */
> +/* { dg-final { scan-ipa-dump "Semantic equality hit:j->i" "icf"  } } */
>