Re: [PATCH] Trust TREE_ADDRESSABLE

[Jan Hubicka <hubicka at ucw dot cz>]

> Note that I'm happy to revert the change.
> 
> I am hesitant to any approach that overloads TREE_ADDRESSABLE even more.
> It already is used for two (slightly) different things - first the
> "old" meaning that the address of the symbol is needed, second, that
> the symbol is aliased by pointers.  Those are of course related, but
> as you see they are not 100% equivalent.

An alternative is surely to add a flag to varpool.  But again, having several
flags of similar names and slightly different meanings doesn't make things more
maintaible either.
> 
> As I already added DECL_NONALIASED (for VAR_DECLs) to "fix" that
> coverage counter issue (those are TREE_STATIC but they have their
> address taken - still we know that no pointers alias the accesses),
> we can as well rely on that flag - but then we should set it whenever
> a TU-local decl does not have its address taken (!TREE_ADDRESSABLE).

I see, I did not notice this.  Will this help me with the situation where
address is taken, but it is only passed to external calls that do not capture
it (i.e. memset), so we know it does not appear in the points-to sets?
> 
> So it does impose some redundancy and possibility of things to go
> out-of-sync.
> 
> Btw, the C frontend doesn't call varpool_finalize_decl for externals,
> so setting TREE_ADDRESSABLE there doesn't work unfortunately.  It
> works with doing it in varpool_node_for_decl though.
> 
> Patch doing both attached (we may choose to do this in different
> places for DECL_EXTERNALs vs. TREE_PUBLIC && TREE_STATICs?).
> At LTO input time we directly call symtab_register_node which would
> side-step this thus an IPA pass could drop TREE_ADDRESSABLE from
> those decls.
> 
> Sofar untested.
> 
> Comments?

I think it may be easier to just set the flag as part of the ipa-visiblity
pass.  I.e. at the time we set externally_visile, we can also set
TREE_ADDRESSABLE for variable.  We don't use alias machinery before
that, right?

Honza
> 
> Thanks,
> Richard.
> 
> 2014-06-10  Richard Biener  <rguenther@suse.de>
> 
> 	* tree.h (TREE_ADDRESSABLE): Clarify.
> 	* varpool.c (varpool_node_for_decl): Mark public or external
> 	variables as TREE_ADDRESSABLE.
> 	* cgraphunit.c (varpool_finalize_decl): Likewise.
> 
> 	* gcc.dg/torture/20140610-1.c: New testcase.
> 	* gcc.dg/torture/20140610-2.c: Likewise.
> 
> Index: gcc/tree.h
> ===================================================================
> --- gcc/tree.h	(revision 211398)
> +++ gcc/tree.h	(working copy)
> @@ -571,8 +571,9 @@ extern void omp_clause_range_check_faile
>  
>  /* Define many boolean fields that all tree nodes have.  */
>  
> -/* In VAR_DECL, PARM_DECL and RESULT_DECL nodes, nonzero means address
> -   of this is needed.  So it cannot be in a register.
> +/* In VAR_DECL, PARM_DECL and RESULT_DECL nodes, nonzero means the address
> +   of this is needed.  So it cannot be in a register.  If not set, then
> +   the address of this cannot be used to initialize an aliasing pointer.
>     In a FUNCTION_DECL it has no meaning.
>     In LABEL_DECL nodes, it means a goto for this label has been seen
>     from a place outside all binding contours that restore stack levels.
> Index: gcc/varpool.c
> ===================================================================
> --- gcc/varpool.c	(revision 211398)
> +++ gcc/varpool.c	(working copy)
> @@ -149,6 +149,8 @@ varpool_node_for_decl (tree decl)
>    if (node)
>      return node;
>  
> +  if (TREE_PUBLIC (decl) || DECL_EXTERNAL (decl))
> +    TREE_ADDRESSABLE (decl) = 1;
>    node = varpool_create_empty_node ();
>    node->decl = decl;
>    symtab_register_node (node);
> Index: gcc/cgraphunit.c
> ===================================================================
> --- gcc/cgraphunit.c	(revision 211398)
> +++ gcc/cgraphunit.c	(working copy)
> @@ -818,6 +818,11 @@ varpool_finalize_decl (tree decl)
>  
>    gcc_assert (TREE_STATIC (decl) || DECL_EXTERNAL (decl));
>  
> +  /* Mark all symbols visible to other TUs as possibly having their
> +     address taken.  */
> +  if (TREE_PUBLIC (decl) || DECL_EXTERNAL (decl))
> +    TREE_ADDRESSABLE (decl) = 1;
> +
>    if (node->definition)
>      return;
>    notice_global_symbol (decl);
> Index: gcc/testsuite/gcc.dg/torture/20140610-1.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/torture/20140610-1.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/torture/20140610-1.c	(working copy)
> @@ -0,0 +1,15 @@
> +/* { dg-do run } */
> +/* { dg-additional-sources "20140610-2.c" } */
> +
> +extern int a;
> +extern int *p;
> +
> +void test (void);
> +
> +int main ()
> +{
> +  *p = 0;
> +  a = 1;
> +  test ();
> +  return 0;
> +}
> Index: gcc/testsuite/gcc.dg/torture/20140610-2.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/torture/20140610-2.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/torture/20140610-2.c	(working copy)
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +
> +extern void abort (void);
> +
> +int a;
> +int *p = &a;
> +
> +void test (void)
> +{
> +  if (a != 1)
> +    abort ();
> +}