This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH][LTO] (Not) merge incompatible common decls


This patch adds the support to keep track of multiple decls per
symtab where we can merge duplicates into.  Thus for non-compatible
decls we keep them separate but only emit the largest one into
the final assembler file.

This fixes a bunch of issues in SPEC 2000 (one extra completely
fixed benchmark and one where we now only run into type verification
issues) and in SPEC 2006 (the compile is still running here, but
5 benchmarks showed the typical error pattern before).

The gfortran testsuite improves in that we now successfully link
more of the C - Fortran mixed-language cases (but still fail there
due to the extra warning we emit that is unexpected) and that
at -O3 we miscompile the thing (because we do not merge int i; and
struct { int i; })

I believe emulating linker behavior is a very good start to tackle
all the problems (and if you could paste the C and Fortran source
into one CU we would have the miscompile issue in the present state
as well ...).  We can try to fixup common and sensible cases
case by case afterwards.

Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for LTO?

-# of expected passes           51698
-# of unexpected failures       190
+# of expected passes           51734
+# of unexpected failures       196

(6 extra execute fails, a load less WARNINGs due to link failures,
but those are not counted, the execute fails previously were
link fails)

I'm not good at LTO testsuite magic - I failed to scan the assembler
output for the correct sized .comm decl.  If anyone can give hints
there that would be appreciated ;)

Thanks,
Richard.


2009-07-30  Richard Guenther  <rguenther@suse.de>

	PR lto/40903
	* lto-symtab.c (lto_symtab_compatible): Only warn for mismatched
	types.
	(lto_symtab_merge_decl): For decls we cannot merge chain them
	appropriately in the symtab entry.
	(lto_symtab_prevailing_decl): Return a matching entry from the
	symtab chain.
	* lto.c (read_cgraph_and_symbols): After fixing up decls choose
	the largest decl for output and free TREE_CHAIN for further
	use.

	* gcc.dg/lto/20090729_0.c: New testcase.
	* gcc.dg/lto/20090729_1.c: Likewise.

Index: gcc/lto-symtab.c
===================================================================
*** gcc/lto-symtab.c.orig	2009-07-30 12:06:46.000000000 +0200
--- gcc/lto-symtab.c	2009-07-30 12:12:38.000000000 +0200
*************** external_aggregate_decl_p (tree decl)
*** 229,235 ****
  /* Check if OLD_DECL and NEW_DECL are compatible. */
  
  static bool
! lto_symtab_compatible (tree old_decl, tree new_decl)
  {
    tree merged_type = NULL_TREE;
    tree merged_result = NULL_TREE;
--- 229,235 ----
  /* Check if OLD_DECL and NEW_DECL are compatible. */
  
  static bool
! lto_symtab_compatible (tree old_decl, tree new_decl, bool do_warn)
  {
    tree merged_type = NULL_TREE;
    tree merged_result = NULL_TREE;
*************** lto_symtab_compatible (tree old_decl, tr
*** 321,331 ****
  
        if (!merged_type)
  	{
! 	  error_at (DECL_SOURCE_LOCATION (new_decl),
! 		    "type of %qD does not match original declaration",
! 		    new_decl);
! 	  inform (DECL_SOURCE_LOCATION (old_decl),
! 		  "previously declared here");
  	  return false;
  	}
      }
--- 321,334 ----
  
        if (!merged_type)
  	{
! 	  if (do_warn)
! 	    {
! 	      warning_at (DECL_SOURCE_LOCATION (new_decl), 0,
! 			  "type of %qD does not match original declaration",
! 			  new_decl);
! 	      inform (DECL_SOURCE_LOCATION (old_decl),
! 		      "previously declared here");
! 	    }
  	  return false;
  	}
      }
*************** lto_symtab_merge_decl (tree new_decl,
*** 542,547 ****
--- 545,552 ----
  
    gcc_assert (TREE_PUBLIC (new_decl));
  
+   gcc_assert (TREE_CHAIN (new_decl) == NULL_TREE);
+ 
    /* Check that declarations reaching this function do not have
       properties inconsistent with having external linkage.  If any of
       these asertions fail, then the object file reader has failed to
*************** lto_symtab_merge_decl (tree new_decl,
*** 585,593 ****
  	}
      }
  
!   /* The linker may ask us to combine two incompatible symbols. */
!   if (!lto_symtab_compatible (old_decl, new_decl))
!     return;
  
    /* Merge decl state in both directions, we may still end up using
       the new decl.  */
--- 590,609 ----
  	}
      }
  
!   /* The linker may ask us to combine two incompatible symbols.
!      Find a decl we can merge with or chain it in the list of decls
!      for that symbol.  */
!   while (old_decl
! 	 && !lto_symtab_compatible (old_decl, new_decl, true))
!     old_decl = TREE_CHAIN (old_decl);
!   if (!old_decl)
!     {
!       old_decl = lto_symtab_get_identifier_decl (name);
!       while (TREE_CHAIN (old_decl) != NULL_TREE)
! 	old_decl = TREE_CHAIN (old_decl);
!       TREE_CHAIN (old_decl) = new_decl;
!       return;
!     }
  
    /* Merge decl state in both directions, we may still end up using
       the new decl.  */
*************** lto_symtab_prevailing_decl (tree decl)
*** 665,673 ****
    /* Ensure DECL_ASSEMBLER_NAME will not set assembler name.  */
    gcc_assert (DECL_ASSEMBLER_NAME_SET_P (decl));
  
    ret = lto_symtab_get_identifier_decl (DECL_ASSEMBLER_NAME (decl));
  
!   return ret;
  }
  
  /* Return the hash table entry of DECL. */
--- 681,703 ----
    /* Ensure DECL_ASSEMBLER_NAME will not set assembler name.  */
    gcc_assert (DECL_ASSEMBLER_NAME_SET_P (decl));
  
+   /* Walk through the list of candidates and return the one we merged to.  */
    ret = lto_symtab_get_identifier_decl (DECL_ASSEMBLER_NAME (decl));
+   if (!ret
+       || TREE_CHAIN (ret) == NULL_TREE)
+     return ret;
+ 
+   /* If there are multiple decls to choose from find the one we merged
+      with and return that.  */
+   while (ret)
+     {
+       if (lto_symtab_compatible (decl, ret, false))
+ 	return ret;
+ 
+       ret = TREE_CHAIN (ret);
+     }
  
!   gcc_unreachable ();
  }
  
  /* Return the hash table entry of DECL. */
Index: gcc/testsuite/gcc.dg/lto/20090729_0.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/lto/20090729_0.c	2009-07-30 12:08:32.000000000 +0200
***************
*** 0 ****
--- 1,4 ----
+ /* { dg-lto-options "-w" } */
+ 
+ double i;
+ int j;
Index: gcc/testsuite/gcc.dg/lto/20090729_1.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/lto/20090729_1.c	2009-07-30 12:08:32.000000000 +0200
***************
*** 0 ****
--- 1,4 ----
+ double j;
+ int i;
+ int main () { return i; }
+ 
Index: gcc/lto/lto.c
===================================================================
*** gcc/lto/lto.c.orig	2009-07-30 12:05:23.000000000 +0200
--- gcc/lto/lto.c	2009-07-30 14:30:24.000000000 +0200
*************** read_cgraph_and_symbols (unsigned nfiles
*** 1588,1593 ****
--- 1588,1638 ----
  
    lto_fixup_decls (all_file_decl_data);
  
+   /* See if we have multiple decls for a symbol and choose the largest
+      one to generate the common.  */
+   for (i = 0; i < VEC_length (tree, lto_global_var_decls); ++i)
+     {
+       tree decl = VEC_index (tree, lto_global_var_decls, i);
+       tree prev_decl = NULL_TREE;
+       tree size;
+ 
+       if (TREE_CODE (decl) != VAR_DECL
+ 	  || !TREE_CHAIN (decl))
+ 	continue;
+ 
+       /* Find the preceeding decl of the largest one.  */
+       size = DECL_SIZE (decl);
+       do
+ 	{
+ 	  if (tree_int_cst_lt (size, DECL_SIZE (TREE_CHAIN (decl))))
+ 	    {
+ 	      size = DECL_SIZE (TREE_CHAIN (decl));
+ 	      prev_decl = decl;
+ 	    }
+ 	  decl = TREE_CHAIN (decl);
+ 	}
+       while (TREE_CHAIN (decl));
+       /* If necessary move the largest decl to the front of the
+ 	 chain.  */
+       if (prev_decl != NULL_TREE)
+ 	{
+ 	  decl = TREE_CHAIN (prev_decl);
+ 	  TREE_CHAIN (prev_decl) = TREE_CHAIN (decl);
+ 	  TREE_CHAIN (decl) = VEC_index (tree, lto_global_var_decls, i);
+ 	  VEC_replace (tree, lto_global_var_decls, i, decl);
+ 	}
+       /* Mark everything apart from the first var as written out and
+          unlink the chain.  */
+       decl = VEC_index (tree, lto_global_var_decls, i);
+       while (TREE_CHAIN (decl))
+ 	{
+ 	  tree next = TREE_CHAIN (decl);
+ 	  TREE_CHAIN (decl) = NULL_TREE;
+ 	  decl = next;
+ 	  TREE_ASM_WRITTEN (decl) = true;
+ 	}
+     }
+ 
    /* FIXME lto. This loop needs to be changed to use the pass manager to
       call the ipa passes directly.  */
    if (!errorcount)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]