http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60577

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nathan at acm dot org

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
In 4.6 for some unknown reason we are able to disambiguate

  *.LPBX1[0] and p_3(D)->buf and *D.1606_14

we built the static decls with !TREE_ADDRESSABLE, but starting with
4.7 we unconditionally set TREE_ADDRESSABLE on the decl (which formerly
was only set in case we asked for the address of it via
tree_coverage_counter_addr).

This happened with the big re-org in r181105 but undoing that change
does not fix the regression.  This is because the TREE_ADDRESSABLE
flag is set again in canonicalize_constructor_val:

#0  0x0000000000932775 in canonicalize_constructor_val (
    cval=<addr_expr 0x7ffff6d74900>, from_decl=<tree 0x0>)
    at /space/rguenther/src/svn/trunk/gcc/gimple-fold.c:207
#1  0x0000000000798015 in record_reference (tp=0x7ffff6d73698, 
    walk_subtrees=0x7fffffffd858, data=0x7fffffffdb50)
    at /space/rguenther/src/svn/trunk/gcc/cgraphbuild.c:62
#2  0x0000000000e6859f in walk_tree_1 (tp=0x7ffff6d73698, 
    func=0x797fdd <record_reference(tree*, int*, void*)>, data=0x7fffffffdb50, 
    pset=0x1ea9240, lh=0x0) at /space/rguenther/src/svn/trunk/gcc/tree.c:10921
#3  0x0000000000e6892c in walk_tree_1 (tp=0x7ffff6d736d0, 
    func=0x797fdd <record_reference(tree*, int*, void*)>, data=0x7fffffffdb50, 
    pset=0x1ea9240, lh=0x0) at /space/rguenther/src/svn/trunk/gcc/tree.c:10998
#4  0x0000000000e6892c in walk_tree_1 (tp=0x7ffff6d78808, 
    func=0x797fdd <record_reference(tree*, int*, void*)>, data=0x7fffffffdb50, 
    pset=0x1ea9240, lh=0x0) at /space/rguenther/src/svn/trunk/gcc/tree.c:10998
#5  0x0000000000e6892c in walk_tree_1 (tp=0x7ffff6d788a0, 
    func=0x797fdd <record_reference(tree*, int*, void*)>, data=0x7fffffffdb50, 
    pset=0x1ea9240, lh=0x0) at /space/rguenther/src/svn/trunk/gcc/tree.c:10998
#6  0x0000000000798d82 in record_references_in_initializer (
    decl=<var_decl 0x7ffff6d78850 __gcov_.access_buf>, only_vars=false)
    at /space/rguenther/src/svn/trunk/gcc/cgraphbuild.c:435
#7  0x0000000000eac7fa in varpool_analyze_node (node=0x7ffff6d498f0)
    at /space/rguenther/src/svn/trunk/gcc/varpool.c:398

thus we _do_ take the address of the variable, from the global
constructor of __gcov.access_buf which looks like

{.D.1783=&*.LPBX0, .D.1784=1, .D.1785=4186437761, .D.1786=449688241,
.D.1787={{.D.1780=3, .D.1781=&__gcov0.access_buf}, {.D.1780=1,
.D.1781=&__gcov8.access_buf}}}

thus indeed p->buf may point to __gcov0.access_buf[1] and we can't optimize
those accesses.  This object was simply not present in GCC 4.6 and thus
TREE_ADDRESSABLE was never set and thus we could optimize this.

Which means the revision cited doesn't cause the regression (I highly doubt
so at least), but instead whoever added code to build that record or
whoever added code to record references in initializers (and set
TREE_ADDRESSABLE in that path... - coverage.c fails to do that and builds
a lot of addresses from the counters in places).

The idea of canonicalize_constructor_value is that the result will end up
in actual stmts, thus the addressable flag is important.  In this case
we only record references for the IPA references struct - thus setting
the flag there isn't important (well, that's not entirely true ... alias
disambiguation depends on the order of processing functions then).

Fixing both fixes the regression.

Index: gcc/coverage.c
===================================================================
--- gcc/coverage.c      (revision 208642)
+++ gcc/coverage.c      (working copy)
@@ -466,6 +466,8 @@ tree_coverage_counter_addr (unsigned cou
   gcc_assert (no < fn_n_ctrs[counter] - fn_b_ctrs[counter]);
   no += fn_b_ctrs[counter];

+  TREE_ADDRESSABLE (fn_v_ctrs[counter]) = 1;
+
   /* "no" here is an array index, scaled to bytes later.  */
   return build_fold_addr_expr (build4 (ARRAY_REF, gcov_type_node,
                                       fn_v_ctrs[counter],
@@ -720,7 +722,6 @@ build_var (tree fn_decl, tree type, int
   memcpy (buf + len, fn_name, fn_name_len + 1);
   DECL_NAME (var) = get_identifier (buf);
   TREE_STATIC (var) = 1;
-  TREE_ADDRESSABLE (var) = 1;
   DECL_ALIGN (var) = TYPE_ALIGN (type);

   return var;
Index: gcc/gimple-fold.h
===================================================================
--- gcc/gimple-fold.h   (revision 208642)
+++ gcc/gimple-fold.h   (working copy)
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.
 #ifndef GCC_GIMPLE_FOLD_H
 #define GCC_GIMPLE_FOLD_H

+extern tree canonicalize_constructor_val_1 (tree, tree, bool);
 extern tree canonicalize_constructor_val (tree, tree);
 extern tree get_symbol_constant_value (tree);
 extern void gimplify_and_update_call_from_tree (gimple_stmt_iterator *, tree);
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c   (revision 208642)
+++ gcc/gimple-fold.c   (working copy)
@@ -169,7 +169,8 @@ can_refer_decl_in_current_unit_p (tree d
    FROM_DECL (if non-NULL) specify variable whose constructor contains CVAL. 
*/

 tree
-canonicalize_constructor_val (tree cval, tree from_decl)
+canonicalize_constructor_val_1 (tree cval, tree from_decl,
+                               bool mark_addressable)
 {
   tree orig_cval = cval;
   STRIP_NOPS (cval);
@@ -203,7 +204,8 @@ canonicalize_constructor_val (tree cval,
           || TREE_CODE (base) == FUNCTION_DECL)
          && !can_refer_decl_in_current_unit_p (base, from_decl))
        return NULL_TREE;
-      if (TREE_CODE (base) == VAR_DECL)
+      if (TREE_CODE (base) == VAR_DECL
+         && mark_addressable)
        TREE_ADDRESSABLE (base) = 1;
       else if (TREE_CODE (base) == FUNCTION_DECL)
        {
@@ -225,6 +227,12 @@ canonicalize_constructor_val (tree cval,
   return orig_cval;
 }

+tree
+canonicalize_constructor_val (tree cval, tree from_decl)
+{
+  return canonicalize_constructor_val_1 (cval, from_decl, true);
+}
+
 /* If SYM is a constant variable with known value, return the value.
    NULL_TREE is returned otherwise.  */


but I am not sure this is a valid thing to do... (I can imagine producing
a testcase that breaks with the delayed TREE_ADDRESSABLE setting)

Why do we have to record addresses of these arrays?

Reply via email to