Re: [patch] Fix type merging deficiency during WPA

[Eric Botcazou <ebotcazou at adacore dot com>]

> So sth like
> 
> Index: gcc/lto-streamer-out.c
> ===================================================================
> --- gcc/lto-streamer-out.c      (revision 238039)
> +++ gcc/lto-streamer-out.c      (working copy)
> @@ -996,7 +996,7 @@ hash_tree (struct streamer_tree_cache_d
>    else
>      hstate.add_flag (TREE_NO_WARNING (t));
>    hstate.add_flag (TREE_NOTHROW (t));
> -  hstate.add_flag (TREE_STATIC (t));
> +  //hstate.add_flag (TREE_STATIC (t));
>    hstate.add_flag (TREE_PROTECTED (t));
>    hstate.add_flag (TREE_DEPRECATED (t));
>    if (code != TREE_BINFO)
> @@ -1050,7 +1050,7 @@ hash_tree (struct streamer_tree_cache_d
>        hstate.add_flag (DECL_ARTIFICIAL (t));
>        hstate.add_flag (DECL_USER_ALIGN (t));
>        hstate.add_flag (DECL_PRESERVE_P (t));
> -      hstate.add_flag (DECL_EXTERNAL (t));
> +      //hstate.add_flag (DECL_EXTERNAL (t));
>        hstate.add_flag (DECL_GIMPLE_REG_P (t));
>        hstate.commit_flag ();
>        hstate.add_int (DECL_ALIGN (t));
> Index: gcc/lto/lto.c
> ===================================================================
> --- gcc/lto/lto.c       (revision 238039)
> +++ gcc/lto/lto.c       (working copy)
> @@ -1263,7 +1263,8 @@ compare_tree_sccs_1 (tree t1, tree t2, t
>      tree t1_ = (E1), t2_ = (E2); \
>      if (t1_ != t2_ \
>         && (!t1_ || !t2_ \
> -           || !TREE_VISITED (t2_) \
> +           || (!TREE_VISITED (t2_) \
> +               && !same_decls (t1_, t2_)) \
> 
>             || (!TREE_ASM_WRITTEN (t2_) \
> 
>                 && !compare_tree_sccs_1 (t1_, t2_, map)))) \
>        return false; \
> 
> plus the magic new function same_decls which would check if the trees are
> var/function decls with the same assembler name. The function can't use
> the cgraph as that is not yet read in unfortunately - I still think we
> should do that so we can see the prevailing nodes early.

OK, thanks for the hint, the attached patch where the call to same_decls is 
placed differently works on the testcase (modulo the gigi change I posted 
earlier today) but not on the original code...  It looks like the hash value 
computed in lto-streamer-out.c is stricter than the hash value computed for 
canonical types, I'm trying to pinpoint the difference.


	* lto-streamer-out.c (hash_tree): Do not add the TREE_STATIC and
	DECL_EXTERNAL flags for variable or function declarations.
lto/
	* lto.c (same_global_decl_p): New predicate.
	(compare_tree_sccs_1): Use it in the comparison of pointer fields.
	(walk_simple_constant_arithmetic): New function.
        (LTO_SET_PREVAIL): Use it to fix up DECL nodes in simple expressions.

-- 
Eric Botcazou

Index: lto-streamer-out.c
===================================================================
--- lto-streamer-out.c	(revision 238156)
+++ lto-streamer-out.c	(working copy)
@@ -996,7 +996,9 @@ hash_tree (struct streamer_tree_cache_d
   else
     hstate.add_flag (TREE_NO_WARNING (t));
   hstate.add_flag (TREE_NOTHROW (t));
-  hstate.add_flag (TREE_STATIC (t));
+  /* We want to unify DECL nodes in pointer fields of global types.  */
+  if (!(VAR_OR_FUNCTION_DECL_P (t)))
+    hstate.add_flag (TREE_STATIC (t));
   hstate.add_flag (TREE_PROTECTED (t));
   hstate.add_flag (TREE_DEPRECATED (t));
   if (code != TREE_BINFO)
@@ -1050,7 +1052,9 @@ hash_tree (struct streamer_tree_cache_d
       hstate.add_flag (DECL_ARTIFICIAL (t));
       hstate.add_flag (DECL_USER_ALIGN (t));
       hstate.add_flag (DECL_PRESERVE_P (t));
-      hstate.add_flag (DECL_EXTERNAL (t));
+      /* We want to unify DECL nodes in pointer fields of global types.  */
+      if (!(VAR_OR_FUNCTION_DECL_P (t)))
+	hstate.add_flag (DECL_EXTERNAL (t));
       hstate.add_flag (DECL_GIMPLE_REG_P (t));
       hstate.commit_flag ();
       hstate.add_int (DECL_ALIGN (t));
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 238156)
+++ lto/lto.c	(working copy)
@@ -970,6 +970,27 @@ static unsigned long num_sccs_merged;
 static unsigned long num_scc_compares;
 static unsigned long num_scc_compare_collisions;
 
+/* Return true if T1 and T2 represent the same global declaration.
+
+   We need to unify declaration nodes in pointer fields of global types
+   so as to unify the types themselves before the declarations are merged.  */
+
+static inline bool
+same_global_decl_p (tree t1, tree t2)
+{
+  if (TREE_CODE (t1) != TREE_CODE (t2) || !VAR_OR_FUNCTION_DECL_P (t1))
+    return false;
+
+  if (!(TREE_PUBLIC (t1) || DECL_EXTERNAL (t1)))
+    return false;
+
+  if (!(TREE_PUBLIC (t2) || DECL_EXTERNAL (t2)))
+    return false;
+
+  return symbol_table::assembler_names_equal_p
+			 (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (t1)),
+			  IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (t2)));
+}
 
 /* Compare the two entries T1 and T2 of two SCCs that are possibly equal,
    recursing through in-SCC tree edges.  Returns true if the SCCs entered
@@ -1262,6 +1283,7 @@ compare_tree_sccs_1 (tree t1, tree t2, t
   do { \
     tree t1_ = (E1), t2_ = (E2); \
     if (t1_ != t2_ \
+	&& !(t1_ && t2_ && same_global_decl_p (t1_, t2_)) \
 	&& (!t1_ || !t2_ \
 	    || !TREE_VISITED (t2_) \
 	    || (!TREE_ASM_WRITTEN (t2_) \
@@ -2506,15 +2528,55 @@ lto_wpa_write_files (void)
   timevar_pop (TV_WHOPR_WPA_IO);
 }
 
+/* Look inside *EXPR into simple arithmetic operations involving constants.
+   Return the address of the outermost non-arithmetic or non-constant node.
+   This should be equivalent to tree.c:skip_simple_constant_arithmetic.  */
+
+static tree *
+walk_simple_constant_arithmetic (tree *expr)
+{
+  while (TREE_CODE (*expr) == NON_LVALUE_EXPR)
+    expr = &TREE_OPERAND (*expr, 0);
+
+  while (true)
+    {
+      if (UNARY_CLASS_P (*expr))
+	expr = &TREE_OPERAND (*expr, 0);
+      else if (BINARY_CLASS_P (*expr))
+	{
+	  if (TREE_CONSTANT (TREE_OPERAND (*expr, 1)))
+	    expr = &TREE_OPERAND (*expr, 0);
+	  else if (TREE_CONSTANT (TREE_OPERAND (*expr, 0)))
+	    expr = &TREE_OPERAND (*expr, 1);
+	  else
+	    break;
+	}
+      else
+	break;
+    }
+
+  return expr;
+}
 
-/* If TT is a variable or function decl replace it with its
-   prevailing variant.  */
+/* If TT is a variable or function decl or a simple expression containing one,
+   replace the occurrence with its prevailing variant.  This should be able to
+   deal with all the expressions attached to _DECL and _TYPE nodes which were
+   streamed into the GIMPLE IR.  */
 #define LTO_SET_PREVAIL(tt) \
   do {\
+    tree *loc; \
     if ((tt) && VAR_OR_FUNCTION_DECL_P (tt) \
 	&& (TREE_PUBLIC (tt) || DECL_EXTERNAL (tt))) \
       { \
-        tt = lto_symtab_prevailing_decl (tt); \
+	tt = lto_symtab_prevailing_decl (tt); \
+	fixed = true; \
+      } \
+    else if ((tt) && EXPR_P (tt) \
+	     && (loc = walk_simple_constant_arithmetic (&tt)) \
+	     && VAR_OR_FUNCTION_DECL_P (*loc) \
+	     && (TREE_PUBLIC (*loc) || DECL_EXTERNAL (*loc))) \
+      { \
+	*loc = lto_symtab_prevailing_decl (*loc); \
 	fixed = true; \
       } \
   } while (0)