On Mon, 9 Jun 2014, Eric Botcazou wrote:

> > I wonder if we want toupdate the frontends to set addressable flag with new
> > sense or we want symtab to simple set addressable on all global symbols or
> > invent a new flag.
> > I would preffer the first case - it seems to make most sense to me.
> 
> I think you need to explain why this change is desirable/necessary for LTO, 
> this would be a good starting point.  As for setting TREE_ADDRESSABLE on 
> every 
> single global symbol in every single front-end, why not, but this seems more 
> complicated than treating global symbols as so by default (in non-LTO mode).

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.

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).

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?

Thanks,
Richard.

2014-06-10  Richard Biener  <rguent...@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 ();
+}

Reply via email to