[PATCH] Fix PR56168

Richard Biener rguenther@suse.de
Fri Feb 1 13:12:00 GMT 2013


So - back to PR55848 - this testcase shows that we still handle
builtins vs. non-builtins in a wrong way.  If at compile-time
we chose to use a non-builtin variant we have to preserve that
(-fno-builtin) - easy to do at WPA stage by adjusting symbol
merging.  Now, at LTRANS stage somebody clever decided to
skip symtab merging - but forgot to disable decl fixup.  In this
case decl fixup with un-merged builtin and non-builtin symtab
entry choses the builtin if it happens to be first in the list
of asm aliases.

The following patch fixes both issues, preserving runtime
behavior of the weird testcase and avoiding the folding to
cbrt as in the original report.

LTO bootstrap and testing running on x86_64-unknown-linux-gnu
(in stage3 already).  Honza, does this look sane?

Thanks,
Richard.

2013-02-01  Richard Guenther  <rguenther@suse.de>

	PR lto/56168
	* lto-symtab.c (lto_symtab_merge_decls_1): Make non-builtin
	node prevail as last resort.
	(lto_symtab_merge_decls): Remove guard on LTRANS here.
	(lto_symtab_prevailing_decl): Builtins are their own prevailing
	decl.

	lto/
	* lto.c (read_cgraph_and_symbols): Do not call lto_symtab_merge_decls
	or lto_fixup_decls at LTRANS time.

	* gcc.dg/lto/pr56168_0.c: New testcase.
	* gcc.dg/lto/pr56168_1.c: Likewise.

Index: gcc/lto-symtab.c
===================================================================
*** gcc/lto-symtab.c	(revision 195641)
--- gcc/lto-symtab.c	(working copy)
*************** lto_symtab_merge_decls_1 (symtab_node fi
*** 439,450 ****
  		&& COMPLETE_TYPE_P (TREE_TYPE (e->symbol.decl)))
  	      prevailing = e;
  	}
!       /* For variables prefer the builtin if one is available.  */
        else if (TREE_CODE (prevailing->symbol.decl) == FUNCTION_DECL)
  	{
  	  for (e = first; e; e = e->symbol.next_sharing_asm_name)
  	    if (TREE_CODE (e->symbol.decl) == FUNCTION_DECL
! 		&& DECL_BUILT_IN (e->symbol.decl))
  	      {
  		prevailing = e;
  		break;
--- 439,450 ----
  		&& COMPLETE_TYPE_P (TREE_TYPE (e->symbol.decl)))
  	      prevailing = e;
  	}
!       /* For variables prefer the non-builtin if one is available.  */
        else if (TREE_CODE (prevailing->symbol.decl) == FUNCTION_DECL)
  	{
  	  for (e = first; e; e = e->symbol.next_sharing_asm_name)
  	    if (TREE_CODE (e->symbol.decl) == FUNCTION_DECL
! 		&& !DECL_BUILT_IN (e->symbol.decl))
  	      {
  		prevailing = e;
  		break;
*************** lto_symtab_merge_decls (void)
*** 507,518 ****
  {
    symtab_node node;
  
-   /* In ltrans mode we read merged cgraph, we do not really need to care
-      about resolving symbols again, we only need to replace duplicated declarations
-      read from the callgraph and from function sections.  */
-   if (flag_ltrans)
-     return;
- 
    /* Populate assembler name hash.   */
    symtab_initialize_asm_name_hash ();
  
--- 507,512 ----
*************** lto_symtab_prevailing_decl (tree decl)
*** 598,603 ****
--- 592,602 ----
    if (TREE_CODE (decl) == FUNCTION_DECL && DECL_ABSTRACT (decl))
      return decl;
  
+   /* Likewise builtins are their own prevailing decl.  This preserves
+      non-builtin vs. builtin uses from compile-time.  */
+   if (TREE_CODE (decl) == FUNCTION_DECL && DECL_BUILT_IN (decl))
+     return decl;
+ 
    /* Ensure DECL_ASSEMBLER_NAME will not set assembler name.  */
    gcc_assert (DECL_ASSEMBLER_NAME_SET_P (decl));
  
Index: gcc/lto/lto.c
===================================================================
*** gcc/lto/lto.c	(revision 195641)
--- gcc/lto/lto.c	(working copy)
*************** read_cgraph_and_symbols (unsigned nfiles
*** 3033,3048 ****
      fprintf (stderr, "Merging declarations\n");
  
    timevar_push (TV_IPA_LTO_DECL_MERGE);
!   /* Merge global decls.  */
!   lto_symtab_merge_decls ();
  
!   /* If there were errors during symbol merging bail out, we have no
!      good way to recover here.  */
!   if (seen_error ())
!     fatal_error ("errors during merging of translation units");
  
!   /* Fixup all decls.  */
!   lto_fixup_decls (all_file_decl_data);
    htab_delete (tree_with_vars);
    tree_with_vars = NULL;
    ggc_collect ();
--- 3033,3054 ----
      fprintf (stderr, "Merging declarations\n");
  
    timevar_push (TV_IPA_LTO_DECL_MERGE);
!   /* Merge global decls.  In ltrans mode we read merged cgraph, we do not
!      need to care about resolving symbols again, we only need to replace
!      duplicated declarations read from the callgraph and from function
!      sections.  */
!   if (!flag_ltrans)
!     {
!       lto_symtab_merge_decls ();
  
!       /* If there were errors during symbol merging bail out, we have no
! 	 good way to recover here.  */
!       if (seen_error ())
! 	fatal_error ("errors during merging of translation units");
  
!       /* Fixup all decls.  */
!       lto_fixup_decls (all_file_decl_data);
!     }
    htab_delete (tree_with_vars);
    tree_with_vars = NULL;
    ggc_collect ();
Index: gcc/testsuite/gcc.dg/lto/pr56168_0.c
===================================================================
*** gcc/testsuite/gcc.dg/lto/pr56168_0.c	(revision 0)
--- gcc/testsuite/gcc.dg/lto/pr56168_0.c	(working copy)
***************
*** 0 ****
--- 1,15 ----
+ /* { dg-lto-do run } */
+ /* { dg-lto-options { { -flto -O -ffast-math -fno-builtin } } } */
+ 
+ extern double pow(double, double);
+ extern void abort (void);
+ volatile double x = 1.0;
+ int main(int argc, char **argv)
+ {
+   double d1 = x;
+   double d2 = pow(d1, 1.0 / 3.0);
+   double d3 = d1 * d1;
+   if (d3 != 1.0 || d2 != 0.0)
+     abort ();
+   return 0;
+ }
Index: gcc/testsuite/gcc.dg/lto/pr56168_1.c
===================================================================
*** gcc/testsuite/gcc.dg/lto/pr56168_1.c	(revision 0)
--- gcc/testsuite/gcc.dg/lto/pr56168_1.c	(working copy)
***************
*** 0 ****
--- 1,4 ----
+ /* { dg-options "-fno-lto" } */
+ 
+ double __attribute__((noinline,noclone))
+ pow (double x, double y) { return 0.0; }



More information about the Gcc-patches mailing list