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]

Re: [PATCH] Fix PR54709


On Wed, 26 Sep 2012, Richard Guenther wrote:

> On Wed, 26 Sep 2012, Richard Guenther wrote:
> 
> > 
> > This fixes PR54709, the location change broke DECL_IS_BUILTIN which
> > is bogously used by streamer_handle_as_builtin_p.  I am reverting
> > the previous lto-symtab.c change as it is not necessary for fixing
> > the testcases.  LTO bootstrap fails with this again with
> > 
> > /tmp/cca0yEEE.ltrans17.ltrans.o: In function `is_ctor_or_dtor.16889':
> > /space/rguenther/src/svn/trunk2/libiberty/cp-demangle.c:5506: undefined 
> > reference to `alloca'
> > /space/rguenther/src/svn/trunk2/libiberty/cp-demangle.c:5507: undefined 
> > reference to `alloca'
> > /tmp/cca0yEEE.ltrans17.ltrans.o: In function `d_demangle_callback.16926':
> > /space/rguenther/src/svn/trunk2/libiberty/cp-demangle.c:5227: undefined 
> > reference to `alloca'
> > /space/rguenther/src/svn/trunk2/libiberty/cp-demangle.c:5228: undefined 
> > reference to `alloca'
> > collect2: error: ld returned 1 exit status
> > 
> > but we want a testcase for this instead of having it magically "fixed"
> > without being analyzed.
> > 
> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> Which revealed issues.  Thus, I'm going with the following, simpler.

Which doesn't fix the issue.  The following does, though, and removes
code which is even better.

Eyed by Honza, LTO bootstrap and regtest running on 
x86_64-unknown-linux-gnu.

I will commit this tomorrow if all things go well.

Richard.

2012-09-26  Richard Guenther  <rguenther@suse.de>

	PR lto/54709
	* lto-symtab.c (resolution_guessed_p): Remove.
	(set_resolution_guessed): Likewise.
	(lto_symtab_register_decl): Remove assert.
	(lto_symtab_resolve_symbols): Do not alter symbol resolutions
	and return the prevailing symbol, checking for multiple prevailing
	symbols here.
	(lto_symtab_merge_decls_1): Use the result from
	lto_symtab_resolve_symbols.  Do not alter symbol resolutions.

	* gcc.dg/lto/pr54709_0.c: New testcase.
	* gcc.dg/lto/pr54709_1.c: Likewise.

Index: gcc/lto-symtab.c
===================================================================
*** gcc/lto-symtab.c	(revision 191756)
--- gcc/lto-symtab.c	(working copy)
*************** along with GCC; see the file COPYING3.
*** 32,52 ****
  /* Vector to keep track of external variables we've seen so far.  */
  VEC(tree,gc) *lto_global_var_decls;
  
- /* Return true if the resolution was guessed and not obtained from
-    the file.  */
- static inline bool
- resolution_guessed_p (symtab_node node)
- {
-   return node->symbol.aux != NULL;
- }
- 
- /* Set guessed flag for NODE.  */
- static inline void
- set_resolution_guessed (symtab_node node, bool value)
- {
-   node->symbol.aux = (void *)(size_t)value;
- }
- 
  /* Registers DECL with the LTO symbol table as having resolution RESOLUTION
     and read from FILE_DATA. */
  
--- 32,37 ----
*************** lto_symtab_register_decl (tree decl,
*** 78,84 ****
      {
        node->symbol.resolution = resolution;
        gcc_assert (node->symbol.lto_file_data == file_data);
-       gcc_assert (!resolution_guessed_p (node));
      }
  }
  
--- 63,68 ----
*************** lto_symtab_resolve_can_prevail_p (symtab
*** 303,309 ****
  /* Resolve the symbol with the candidates in the chain *SLOT and store
     their resolutions.  */
  
! static void
  lto_symtab_resolve_symbols (symtab_node first)
  {
    symtab_node e;
--- 287,293 ----
  /* Resolve the symbol with the candidates in the chain *SLOT and store
     their resolutions.  */
  
! static symtab_node
  lto_symtab_resolve_symbols (symtab_node first)
  {
    symtab_node e;
*************** lto_symtab_resolve_symbols (symtab_node
*** 315,341 ****
  	&& (e->symbol.resolution == LDPR_PREVAILING_DEF_IRONLY
  	    || e->symbol.resolution == LDPR_PREVAILING_DEF_IRONLY_EXP
  	    || e->symbol.resolution == LDPR_PREVAILING_DEF))
!       prevailing = e;
  
    /* If the chain is already resolved there is nothing else to do.  */
    if (prevailing)
!     return;
  
    /* Find the single non-replaceable prevailing symbol and
       diagnose ODR violations.  */
    for (e = first; e; e = e->symbol.next_sharing_asm_name)
      {
        if (!lto_symtab_resolve_can_prevail_p (e))
! 	{
! 	  e->symbol.resolution = LDPR_RESOLVED_IR;
!           set_resolution_guessed (e, true);
! 	  continue;
! 	}
  
!       /* Set a default resolution - the final prevailing one will get
!          adjusted later.  */
!       e->symbol.resolution = LDPR_PREEMPTED_IR;
!       set_resolution_guessed (e, true);
        if (!lto_symtab_resolve_replaceable_p (e))
  	{
  	  if (prevailing)
--- 299,331 ----
  	&& (e->symbol.resolution == LDPR_PREVAILING_DEF_IRONLY
  	    || e->symbol.resolution == LDPR_PREVAILING_DEF_IRONLY_EXP
  	    || e->symbol.resolution == LDPR_PREVAILING_DEF))
!       {
! 	prevailing = e;
! 	break;
!       }
  
    /* If the chain is already resolved there is nothing else to do.  */
    if (prevailing)
!     {
!       /* Assert it's the only one.  */
!       for (e = prevailing->symbol.next_sharing_asm_name; e; e = e->symbol.next_sharing_asm_name)
! 	if (symtab_real_symbol_p (e)
! 	    && (e->symbol.resolution == LDPR_PREVAILING_DEF_IRONLY
! 		|| e->symbol.resolution == LDPR_PREVAILING_DEF_IRONLY_EXP
! 		|| e->symbol.resolution == LDPR_PREVAILING_DEF))
! 	  fatal_error ("multiple prevailing defs for %qE",
! 		       DECL_NAME (prevailing->symbol.decl));
!       return prevailing;
!     }
  
    /* Find the single non-replaceable prevailing symbol and
       diagnose ODR violations.  */
    for (e = first; e; e = e->symbol.next_sharing_asm_name)
      {
        if (!lto_symtab_resolve_can_prevail_p (e))
! 	continue;
  
!       /* If we have a non-replaceable definition it prevails.  */
        if (!lto_symtab_resolve_replaceable_p (e))
  	{
  	  if (prevailing)
*************** lto_symtab_resolve_symbols (symtab_node
*** 349,360 ****
  	}
      }
    if (prevailing)
!     goto found;
  
    /* Do a second round choosing one from the replaceable prevailing decls.  */
    for (e = first; e; e = e->symbol.next_sharing_asm_name)
      {
!       if (e->symbol.resolution != LDPR_PREEMPTED_IR
  	  || !symtab_real_symbol_p (e))
  	continue;
  
--- 339,350 ----
  	}
      }
    if (prevailing)
!     return prevailing;
  
    /* Do a second round choosing one from the replaceable prevailing decls.  */
    for (e = first; e; e = e->symbol.next_sharing_asm_name)
      {
!       if (!lto_symtab_resolve_can_prevail_p (e)
  	  || !symtab_real_symbol_p (e))
  	continue;
  
*************** lto_symtab_resolve_symbols (symtab_node
*** 386,410 ****
  	prevailing = e;
      }
  
!   if (!prevailing)
!     return;
! 
! found:
!   /* If current lto files represent the whole program,
!     it is correct to use LDPR_PREVALING_DEF_IRONLY.
!     If current lto files are part of whole program, internal
!     resolver doesn't know if it is LDPR_PREVAILING_DEF
!     or LDPR_PREVAILING_DEF_IRONLY.  Use IRONLY conforms to
!     using -fwhole-program.  Otherwise, it doesn't
!     matter using either LDPR_PREVAILING_DEF or
!     LDPR_PREVAILING_DEF_IRONLY
!     
!     FIXME: above workaround due to gold plugin makes some
!     variables IRONLY, which are indeed PREVAILING_DEF in
!     resolution file.  These variables still need manual
!     externally_visible attribute.  */
!     prevailing->symbol.resolution = LDPR_PREVAILING_DEF_IRONLY;
!     set_resolution_guessed (prevailing, true);
  }
  
  /* Merge all decls in the symbol table chain to the prevailing decl and
--- 376,382 ----
  	prevailing = e;
      }
  
!   return prevailing;
  }
  
  /* Merge all decls in the symbol table chain to the prevailing decl and
*************** lto_symtab_merge_decls_1 (symtab_node fi
*** 478,504 ****
  
    /* Compute the symbol resolutions.  This is a no-op when using the
       linker plugin and resolution was decided by the linker.  */
!   lto_symtab_resolve_symbols (first);
! 
!   /* Find the prevailing decl.  */
!   for (prevailing = first;
!        prevailing
!        && (!symtab_real_symbol_p (prevailing)
! 	   || (prevailing->symbol.resolution != LDPR_PREVAILING_DEF_IRONLY
! 	       && prevailing->symbol.resolution != LDPR_PREVAILING_DEF_IRONLY_EXP
! 	       && prevailing->symbol.resolution != LDPR_PREVAILING_DEF));
!        prevailing = prevailing->symbol.next_sharing_asm_name)
!     ;
! 
!   /* Assert it's the only one.  */
!   if (prevailing)
!     for (e = prevailing->symbol.next_sharing_asm_name; e; e = e->symbol.next_sharing_asm_name)
!       if (symtab_real_symbol_p (e)
! 	  && (e->symbol.resolution == LDPR_PREVAILING_DEF_IRONLY
! 	      || e->symbol.resolution == LDPR_PREVAILING_DEF_IRONLY_EXP
! 	      || e->symbol.resolution == LDPR_PREVAILING_DEF))
! 	fatal_error ("multiple prevailing defs for %qE",
! 		     DECL_NAME (prevailing->symbol.decl));
  
    /* If there's not a prevailing symbol yet it's an external reference.
       Happens a lot during ltrans.  Choose the first symbol with a
--- 450,456 ----
  
    /* Compute the symbol resolutions.  This is a no-op when using the
       linker plugin and resolution was decided by the linker.  */
!   prevailing = lto_symtab_resolve_symbols (first);
  
    /* If there's not a prevailing symbol yet it's an external reference.
       Happens a lot during ltrans.  Choose the first symbol with a
*************** lto_symtab_merge_decls_1 (symtab_node fi
*** 574,594 ****
        for (e = prevailing; e; e = e->symbol.next_sharing_asm_name)
  	dump_symtab_node (cgraph_dump_file, e);
      }
- 
-   /* Store resolution decision into the callgraph.  
-      In LTRANS don't overwrite information we stored into callgraph at
-      WPA stage.
- 
-      Do not bother to store guessed decisions.  Generic code knows how
-      to handle UNKNOWN relocation well.
- 
-      The problem with storing guessed decision is whether to use
-      PREVAILING_DEF, PREVAILING_DEF_IRONLY, PREVAILING_DEF_IRONLY_EXP.
-      First one would disable some whole program optimizations, while
-      ther second would imply to many whole program assumptions.  */
-   if (resolution_guessed_p (prevailing))
-     prevailing->symbol.resolution = LDPR_UNKNOWN;
-   return;
  }
  
  /* Resolve and merge all symbol table chains to a prevailing decl.  */
--- 526,531 ----
Index: gcc/testsuite/gcc.dg/lto/pr54709_0.c
===================================================================
*** gcc/testsuite/gcc.dg/lto/pr54709_0.c	(revision 0)
--- gcc/testsuite/gcc.dg/lto/pr54709_0.c	(working copy)
***************
*** 0 ****
--- 1,9 ----
+ /* { dg-lto-do link } */
+ /* { dg-require-visibility "hidden" } */
+ /* { dg-extra-ld-options { -shared } } */
+ /* { dg-lto-options { { -fPIC -fvisibility=hidden -flto } } } */
+ 
+ void foo (void *p, void *q, unsigned s)
+ {
+   __builtin_memcpy (p, q, s);
+ }
Index: gcc/testsuite/gcc.dg/lto/pr54709_1.c
===================================================================
*** gcc/testsuite/gcc.dg/lto/pr54709_1.c	(revision 0)
--- gcc/testsuite/gcc.dg/lto/pr54709_1.c	(working copy)
***************
*** 0 ****
--- 1,5 ----
+ void * memcpy (void *, void *, long);
+ void bar (void *p, void *q, unsigned s)
+ {
+   memcpy (p, q, s);
+ }


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