Bug 46055 - [4.6 Regression] -fwhopr failed configure test
Summary: [4.6 Regression] -fwhopr failed configure test
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: bootstrap (show other bugs)
Version: 4.6.0
: P3 normal
Target Milestone: 4.6.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-17 07:51 UTC by H.J. Lu
Modified: 2010-10-20 21:17 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-10-18 09:55:04


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2010-10-17 07:51:49 UTC
On Linux/x86-64, revision 165574 failed to
bootstrap:

http://gcc.gnu.org/ml/gcc-regression/2010-10/msg00186.html

Revision 165562 is OK.
Comment 1 H.J. Lu 2010-10-17 07:54:19 UTC
It may be caused by revision 165569:

http://gcc.gnu.org/ml/gcc-cvs/2010-10/msg00754.html
Comment 2 H.J. Lu 2010-10-17 20:55:24 UTC
I got

[hjl@gnu-32 prev-gcc]$ ./xgcc -B./ -O2  /tmp/foo.i -fuse-linker-plugin  -fwhopr=jobserver
[hjl@gnu-32 prev-gcc]$ cat /tmp/foo.i
char pstat_getstatic ();
char (*f) () = pstat_getstatic;
int
main ()
{
  return f != pstat_getstatic;
}
[hjl@gnu-32 prev-gcc]$ ./xgcc -B./ -O2  /tmp/foo.i -fuse-linker-plugin  -fwhopr=jobserver
[hjl@gnu-32 prev-gcc]$ ./xgcc -B./ -O2  /tmp/foo.i -fuse-linker-plugin 
ld.gold: /tmp/ccdoQXrS.o: in function main:foo.i(.text+0x9): error: undefined reference to 'pstat_getstatic'
ld.gold: /tmp/ccdoQXrS.o: in function f:foo.i(.data+0x0): error: undefined reference to 'pstat_getstatic'
collect2: ld returned 1 exit status
[hjl@gnu-32 prev-gcc]$ 

Revision 165569 breaks -fwhopr=jobserver.
Comment 3 H.J. Lu 2010-10-17 21:22:02 UTC
(In reply to comment #2)
> I got
> 
> [hjl@gnu-32 prev-gcc]$ ./xgcc -B./ -O2  /tmp/foo.i -fuse-linker-plugin 
> -fwhopr=jobserver
> [hjl@gnu-32 prev-gcc]$ cat /tmp/foo.i
> char pstat_getstatic ();
> char (*f) () = pstat_getstatic;
> int
> main ()
> {
>   return f != pstat_getstatic;
> }

This test may not be compatible with -fwhopr. Can we
use

--
char pstat_getstatic ();
int
main ()
{
  return pstat_getstatic != (void *) 1;
}
--

instead?
Comment 4 H.J. Lu 2010-10-17 21:32:51 UTC
I am not sure test:

---
char pstat_getstatic ();
char (*f) () = pstat_getstatic;
int
main ()
{
  return f != pstat_getstatic;
}
---

which detects if pstat_getstatic is defined,
is compatible with whopr, which may decide
f is never changed and optimize it to

int
main ()
{
  return 0;
}

We need a better way to detect if a function is
available when whopr is enabled.
Comment 5 H.J. Lu 2010-10-17 22:17:41 UTC
To test if pstat_getstatic exists with a link
test, why not just use

---
int pstat_getstatic ();
int
main ()
{
  return pstat_getstatic ();
}
---
Comment 6 Andi Kleen 2010-10-17 22:28:09 UTC
Could simply mark the variable volatile?
Comment 7 Richard Biener 2010-10-18 09:55:04 UTC
autoconf should disable optimization for all this kind of tests (thus,
append -O0 to CFLAGS).  The issue is caused by -fwhole-program btw, not
by -fwhopr.  If it happens with -fwhopr only then sth is wrong in GCC.
Comment 8 Richard Biener 2010-10-18 09:58:41 UTC
Hmm, does -fuse-linker-plugin have the same side-effects as -fwhole-program?
That will break symbol use by dlopened objects and we have to avoid that.

Honza?
Comment 9 H.J. Lu 2010-10-18 12:23:36 UTC
(In reply to comment #8)
> Hmm, does -fuse-linker-plugin have the same side-effects as -fwhole-program?
> That will break symbol use by dlopened objects and we have to avoid that.
> 

[hjl@gnu-6 pr46055]$ make
/export/build/gnu/gcc/build-x86_64-linux/prev-gcc/xgcc -B/export/build/gnu/gcc/build-x86_64-linux/prev-gcc/ -O2 -fuse-linker-plugin -fwhopr -o foo foo.i
ld.gold: /tmp/ccIRVeGM.ltrans0.ltrans.o: in function main:ccIRVeGM.ltrans0.o(.text+0x3): error: undefined reference to 'pstat_getstatic'
collect2: ld returned 1 exit status
make: *** [foo] Error 1
[hjl@gnu-6 pr46055]$ /export/build/gnu/gcc/build-x86_64-linux/prev-gcc/xgcc -B/export/build/gnu/gcc/build-x86_64-linux/prev-gcc/ -O2 -fwhopr -o foo foo.i
/tmp/cczPtxXa.o: In function `main':
foo.i:(.text+0x3): undefined reference to `pstat_getstatic'
collect2: ld returned 1 exit status
[hjl@gnu-6 pr46055]$ /export/build/gnu/gcc/build-x86_64-linux/prev-gcc/xgcc -B/export/build/gnu/gcc/build-x86_64-linux/prev-gcc/ -O2 -fwhole-program -o foo foo.i
/tmp/ccB81NJo.o: In function `main':
foo.i:(.text+0x3): undefined reference to `pstat_getstatic'
collect2: ld returned 1 exit status
[hjl@gnu-6 pr46055]$ /export/build/gnu/gcc/build-x86_64-linux/prev-gcc/xgcc -B/export/build/gnu/gcc/build-x86_64-linux/prev-gcc/ -O2 -fwhole-program -o foo foo.i -fuse-linker-plugin
ld.gold: /tmp/cc4NeRs5.o: in function main:foo.i(.text+0x3): error: undefined reference to 'pstat_getstatic'
collect2: ld returned 1 exit status
[hjl@gnu-6 pr46055]$ 

It only happens with "-fuse-linker-plugin -fwhopr".
Comment 10 Jan Hubicka 2010-10-18 15:11:52 UTC
Hi,
I found what is causing the problem.  I accidentally comitted also the
following cleanup of visibility code.  The difference is that we now trust
linker's LDPR_PREVAILING_DEF_IRONLY and bring symbols local when they are
declared so even without default visibility.

Linker should not declare symbols that can be bound by dynamic linker
as LDPR_PREVAILING_DEF_IRONLY (according to my earlier conversation about
COMDATs).  I am looking into it and will either revert the patch today or
add missing changelog (and post to ML).
Sorry for that.

Honza

Index: ipa.c
===================================================================
*** ipa.c	(revision 165562)
--- ipa.c	(working copy)
*************** cgraph_externally_visible_p (struct cgra
*** 595,600 ****
--- 608,630 ----
    /* If linker counts on us, we must preserve the function.  */
    if (cgraph_used_from_object_file_p (node))
      return true;
+   if (DECL_PRESERVE_P (node->decl))
+     return true;
+   if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES (node->decl)))
+     return true;
+ 
+   /* See if we have linker information about symbol not being used or
+      if we need to make guess based on the declaration.
+ 
+      Even if the linker clams the symbol is unused, never bring internal
+      symbols that are declared by user as used or externally visible.
+      This is needed for i.e. references from asm statements.   */
+   for (alias = node->same_body; alias; alias = alias->next)
+     if (alias->resolution != LDPR_PREVAILING_DEF_IRONLY)
+       break;
+   if (!alias && node->resolution == LDPR_PREVAILING_DEF_IRONLY)
+     return false;
+ 
    /* When doing link time optimizations, hidden symbols become local.  */
    if (in_lto_p
        && (DECL_VISIBILITY (node->decl) == VISIBILITY_HIDDEN
*************** cgraph_externally_visible_p (struct cgra
*** 626,636 ****
  	      return true;
  	}
      }
!   if (DECL_PRESERVE_P (node->decl))
!     return true;
    if (MAIN_NAME_P (DECL_NAME (node->decl)))
      return true;
!   if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES (node->decl)))
      return true;
    return false;
  }
--- 656,725 ----
  	      return true;
  	}
      }
! 
    if (MAIN_NAME_P (DECL_NAME (node->decl)))
      return true;
! 
!   return false;
! }
! 
! /* Return true when variable VNODE should be considered externally visible.  */
! 
! static bool
! varpool_externally_visible_p (struct varpool_node *vnode, bool aliased)
! {
!   struct varpool_node *alias;
!   if (!DECL_COMDAT (vnode->decl) && !TREE_PUBLIC (vnode->decl))
!     return false;
! 
!   /* Do not even try to be smart about aliased nodes.  Until we properly
!      represent everything by same body alias, these are just evil.  */
!   if (aliased)
!     return true;
! 
!   /* If linker counts on us, we must preserve the function.  */
!   if (varpool_used_from_object_file_p (vnode))
!     return true;
! 
!   if (DECL_PRESERVE_P (vnode->decl))
!     return true;
!   if (lookup_attribute ("externally_visible",
! 			DECL_ATTRIBUTES (vnode->decl)))
!     return true;
! 
!   /* See if we have linker information about symbol not being used or
!      if we need to make guess based on the declaration.
! 
!      Even if the linker clams the symbol is unused, never bring internal
!      symbols that are declared by user as used or externally visible.
!      This is needed for i.e. references from asm statements.   */
!   if (varpool_used_from_object_file_p (vnode))
!     return true;
!   for (alias = vnode->extra_name; alias; alias = alias->next)
!     if (alias->resolution != LDPR_PREVAILING_DEF_IRONLY)
!       break;
!   if (!alias && vnode->resolution == LDPR_PREVAILING_DEF_IRONLY)
!     return false;
! 
!   /* When doing link time optimizations, hidden symbols become local.  */
!   if (in_lto_p
!       && (DECL_VISIBILITY (vnode->decl) == VISIBILITY_HIDDEN
! 	  || DECL_VISIBILITY (vnode->decl) == VISIBILITY_INTERNAL)
!       /* Be sure that node is defined in IR file, not in other object
! 	 file.  In that case we don't set used_from_other_object_file.  */
!       && vnode->finalized)
!     ;
!   else if (!flag_whole_program)
!     return true;
! 
!   /* Do not attempt to privatize COMDATS by default.
!      This would break linking with C++ libraries sharing
!      inline definitions.
! 
!      FIXME: We can do so for readonly vars with no address taken and
!      possibly also for vtables since no direct pointer comparsion is done.
!      It might be interesting to do so to reduce linking overhead.  */
!   if (DECL_COMDAT (vnode->decl) || DECL_WEAK (vnode->decl))
      return true;
    return false;
  }
*************** function_and_variable_visibility (bool w
*** 786,812 ****
        if (!vnode->finalized)
          continue;
        if (vnode->needed
! 	  && (DECL_COMDAT (vnode->decl) || TREE_PUBLIC (vnode->decl))
! 	  && (((!whole_program
! 	        /* We can privatize comdat readonly variables whose address is
! 		   not taken, but doing so is not going to bring us
! 		   optimization oppurtunities until we start reordering
! 		   datastructures.  */
! 		|| DECL_COMDAT (vnode->decl)
! 		|| DECL_WEAK (vnode->decl))
! 	       /* When doing linktime optimizations, all hidden symbols will
! 		  become local.  */
! 	       && (!in_lto_p
! 		   || (DECL_VISIBILITY (vnode->decl) != VISIBILITY_HIDDEN
! 		       && DECL_VISIBILITY (vnode->decl) != VISIBILITY_INTERNAL)
! 		   /* We can get prevailing decision in other object file.
! 		      In this case we do not sed used_from_object_file.  */
! 		   || !vnode->finalized))
! 	      || DECL_PRESERVE_P (vnode->decl)
!               || varpool_used_from_object_file_p (vnode)
! 	      || pointer_set_contains (aliased_vnodes, vnode)
! 	      || lookup_attribute ("externally_visible",
! 				   DECL_ATTRIBUTES (vnode->decl))))
  	vnode->externally_visible = true;
        else
          vnode->externally_visible = false;
--- 875,883 ----
        if (!vnode->finalized)
          continue;
        if (vnode->needed
! 	  && varpool_externally_visible_p
! 	      (vnode, 
! 	       pointer_set_contains (aliased_vnodes, vnode)))
  	vnode->externally_visible = true;
        else
          vnode->externally_visible = false;
Comment 11 Jan Hubicka 2010-10-18 15:47:24 UTC
Hi,
the plugin seems to give bit funny resolutions in side cases, so I will revert
the accident commit now, will commit the cleanup part of change incrementally
and then we can hopefully discuss how the gold should behave and work around
the bugs, if any.

I seem to agree with H.J. that the simple linking test should suffice here.
Can we get libiberty configury fixed?  It is needed anyway for -fwhole-program
as well as for static linking targets.

Honza
Comment 12 H.J. Lu 2010-10-20 21:17:30 UTC
Fixed by

http://gcc.gnu.org/ml/gcc-cvs/2010-10/msg00840.html