Re: Another AIX Bootstrap failure

Re: Another AIX Bootstrap failure

[Dominique Dhumieres]

The tests gcc.dg/globalalias-2.c and gcc.dg/localalias-2.c fail on darwin with

/opt/gcc/work/gcc/testsuite/gcc.dg/globalalias-2.c:20:2: warning: alias definitions not supported in Mach-O; ignored

I think they should be protected by

/* { dg-require-alias "" } */

Dominique

Re: Another AIX Bootstrap failure

[Jan Hubicka]

> The tests gcc.dg/globalalias-2.c and gcc.dg/localalias-2.c fail on darwin with
> 
> /opt/gcc/work/gcc/testsuite/gcc.dg/globalalias-2.c:20:2: warning: alias definitions not supported in Mach-O; ignored
> 
> I think they should be protected by
> 
> /* { dg-require-alias "" } */

I see, the anoying property of dg.exp where we compile the additional sources separately, too.  Will fix that.
Is it really the case that Mach-O have no way of creating alias, even by putting alternative symbol into the
source as we intend to do for AIX?

Honza
> 
> Dominique

Re: Another AIX Bootstrap failure

[Iain Sandoe]

Hi Honza,

On 23 Jun 2014, at 18:36, Jan Hubicka wrote:

>> The tests gcc.dg/globalalias-2.c and gcc.dg/localalias-2.c fail on darwin with
>> 
>> /opt/gcc/work/gcc/testsuite/gcc.dg/globalalias-2.c:20:2: warning: alias definitions not supported in Mach-O; ignored
>> 
>> I think they should be protected by
>> 
>> /* { dg-require-alias "" } */
> 
> I see, the anoying property of dg.exp where we compile the additional sources separately, too.  Will fix that.
> Is it really the case that Mach-O have no way of creating alias, even by putting alternative symbol into the
> source as we intend to do for AIX?

The status is this: 

There is a symbol flag in Mach-O (N_INDR) that would permit marking one symbol as an alias of another.

At present, this is ignored by ld64 (and not emitted by cctools-as)
[although, ironically, it used to be acted on in the ancient ld_classic of ~2004 vintage]

Fortunately, it seems that the darwin/OSX community within llvm is interested in having support for this functionality which is Good News, since without ld64 suppport, it would make maintaining it for future systems very difficult.

The statement is "it's on the TODO, but not a high priority".

----

For any darwin < 13 (or maybe even 14) it would mean that we would have to roll our own implementation.

Actually, I am quite close to having both GAS ports and a more generally buildable ld64 version - so rolling our own is really a possibility.  

However, as I'm sure you know, Darwin stuff on GCC/GAS is on a volunteer basis, so it's difficult to say in which decade the support might appear ;)

cheers
Iain

Re: Another AIX Bootstrap failure

[Jan Hubicka]

> The new testcases also declare main() as "void", but "return 0".

I fixed that with previous commit too (what happened is that I managed to
copy wrong version of the files while testing that they fail on AIX.
They indeed did fail, but for wrong versoin)

Honza

Re: Another AIX Bootstrap failure

[David Edelsohn]

The new testcases also declare main() as "void", but "return 0".

- David

On Fri, Jun 20, 2014 at 10:43 PM, Jan Hubicka <hubicka@ucw.cz> wrote:

> Index: testsuite/gcc.dg/localalias.c
> ===================================================================
> --- testsuite/gcc.dg/localalias.c       (revision 0)
> +++ testsuite/gcc.dg/localalias.c       (revision 0)
> @@ -0,0 +1,42 @@
> +/* This test checks that local aliases behave sanely.  This is necessary for code correctness
> +   of aliases introduced by ipa-visibility pass.
> +
> +   If this test fails either aliases needs to be disabled on given target on aliases with
> +   proper semantic needs to be implemented.  This is problem with e.g. AIX .set pseudo-op
> +   that implementes alias syntactically (by substituting in assembler) rather as alternative
> +   symbol defined on a target's location.  */
> +
> +/* { dg-do run }
> +   { dg-options "-Wstrict-aliasing=2 -fstrict-aliasing" }
> +   { dg-require-alias "" }
> +   { dg-xfail-if "" { powerpc-ibm-aix* } { "*" } { "" } }
> +   { dg-additional-sources "localalias-2.c" } */
> +extern void abort (void);
> +extern int test2count;
> +int testcount;
> +__attribute__ ((weak,noinline))
> +void test(void)
> +{
> +  testcount++;
> +}
> +__attribute ((alias("test")))
> +static void test2(void);
> +
> +void main()
^^^^^^^^^^^^^^^^^^^^^
> +{
> +  test2();
> +  /* This call must bind locally.  */
> +  if (!testcount)
> +    abort ();
> +  test();
> +  /* Depending on linker choice, this one may bind locally
> +     or to the other unit.  */
> +  if (!testcount && !test2count)
> +    abort();
> +  tt();
> +
> +  if ((testcount != 1 || test2count != 3)
> +      && (testcount != 3 || test2count != 1))
> +    abort ();
> +  reutrn 0;
^^^^^^^^^^^^^^^^^^^^^^
> +}
> Index: testsuite/gcc.dg/globalalias.c
> ===================================================================
> --- testsuite/gcc.dg/globalalias.c      (revision 0)
> +++ testsuite/gcc.dg/globalalias.c      (revision 0)
> @@ -0,0 +1,42 @@
> +/* This test checks that local aliases behave sanely.  This is necessary for code correctness
> +   of aliases introduced by ipa-visibility pass.
> +
> +   This test expose weird behaviour of AIX's .set pseudo-op where the global symbol is created,
> +   but all uses of the alias are syntactically replaced by uses of the target.  This means that
> +   both counters are increased to 2.  */
> +
> +/* { dg-do run }
> +   { dg-options "-O2" }
> +   { dg-require-alias "" }
> +   { dg-xfail-if "" { powerpc-ibm-aix* } { "*" } { "" } }
> +   { dg-additional-sources "globalalias-2.c" } */
> +extern int test2count;
> +extern void abort (void);
> +int testcount;
> +static
> +void test(void)
> +{
> +  testcount++;
> +}
> +__attribute__ ((weak,noinline))
> +__attribute ((alias("test")))
> +void test2(void);
> +
> +void main()
^^^^^^^^^^^^^^^^^^^^^^^
> +{
> +  test();
> +  /* This call must bind locally.  */
> +  if (!testcount)
> +    abort ();
> +  test2();
> +  /* Depending on linker choice, this one may bind locally
> +     or to the other unit.  */
> +  if (!testcount && !test2count)
> +    abort();
> +  tt();
> +
> +  if ((testcount != 1 || test2count != 3)
> +      && (testcount != 3 || test2count != 1))
> +    abort ();
> +  return 0;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +}
> Index: testsuite/gcc.dg/globalalias-2.c
> ===================================================================
> --- testsuite/gcc.dg/globalalias-2.c    (revision 0)
> +++ testsuite/gcc.dg/globalalias-2.c    (revision 0)
> @@ -0,0 +1,20 @@
> +int test2count;
> +extern void abort (void);
> +static
> +void test(void)
> +{
> +  test2count++;
> +}
> +__attribute__ ((weak,noinline))
> +__attribute ((alias("test")))
> +void test2(void);
> +
> +void tt()
> +{
> +  int prev = test2count;
> +  /* This call must bind locally.  */
> +  test();
> +  if (test2count == prev)
> +    abort();
> +  test2();
> + }
> Index: testsuite/gcc.dg/localalias-2.c
> ===================================================================
> --- testsuite/gcc.dg/localalias-2.c     (revision 0)
> +++ testsuite/gcc.dg/localalias-2.c     (revision 0)
> @@ -0,0 +1,19 @@
> +extern void abort (void);
> +int test2count;
> +__attribute__ ((weak,noinline))
> +void test(void)
> +{
> +  test2count++;
> +}
> +__attribute ((alias("test")))
> +static void test2(void);
> +
> +void tt()
> +{
> +  int prev = test2count;
> +  /* This call must bind locally.  */
> +  test2();
> +  if (test2count == prev)
> +    abort();
> +  test();
> + }
> Index: ipa-visibility.c
> ===================================================================
> --- ipa-visibility.c    (revision 211858)
> +++ ipa-visibility.c    (working copy)
> @@ -566,7 +566,11 @@ function_and_variable_visibility (bool w
>          cheaper and enable more optimization.
>
>          TODO: We can also update virtual tables.  */
> -      if (node->callers && can_replace_by_local_alias (node))
> +      if (node->callers
> +          /* FIXME: currently this optimization breaks on AIX.  Disable it for targets
> +             without comdat support for now.  */
> +         && SUPPORTS_ONE_ONLY
> +         && can_replace_by_local_alias (node))
>         {
>           struct cgraph_node *alias = cgraph (symtab_nonoverwritable_alias (node));
>

Re: Another AIX Bootstrap failure

[Jan Hubicka]

> > +  /* Depending on linker choice, this one may bind locally
> > +     or to the other unit.  */
> > +  if (!testcount && !test2count)
> > +    abort();
> > +  tt();
> > +
> > +  if ((testcount != 1 || test2count != 3)
> > +      && (testcount != 3 || test2count != 1))
> > +    abort ();
> > +  reutrn 0;
> ^^^^^^^^^^^^^^^^^ typo
> > +}
> 
> return 0;
> 
> You probably should run the testcases before committing them.

Uhm, sorry. I must have messed up testing.
I commited the fix.

Honza

Re: Another AIX Bootstrap failure

[David Edelsohn]

> Index: testsuite/gcc.dg/localalias.c
> ===================================================================
> --- testsuite/gcc.dg/localalias.c       (revision 0)
> +++ testsuite/gcc.dg/localalias.c       (revision 0)
> @@ -0,0 +1,42 @@
> +/* This test checks that local aliases behave sanely.  This is necessary for code correctness
> +   of aliases introduced by ipa-visibility pass.
> +
> +   If this test fails either aliases needs to be disabled on given target on aliases with
> +   proper semantic needs to be implemented.  This is problem with e.g. AIX .set pseudo-op
> +   that implementes alias syntactically (by substituting in assembler) rather as alternative
> +   symbol defined on a target's location.  */
> +
> +/* { dg-do run }
> +   { dg-options "-Wstrict-aliasing=2 -fstrict-aliasing" }
> +   { dg-require-alias "" }
> +   { dg-xfail-if "" { powerpc-ibm-aix* } { "*" } { "" } }
> +   { dg-additional-sources "localalias-2.c" } */
> +extern void abort (void);
> +extern int test2count;
> +int testcount;
> +__attribute__ ((weak,noinline))
> +void test(void)
> +{
> +  testcount++;
> +}
> +__attribute ((alias("test")))
> +static void test2(void);
> +
> +void main()
> +{
> +  test2();
> +  /* This call must bind locally.  */
> +  if (!testcount)
> +    abort ();
> +  test();
> +  /* Depending on linker choice, this one may bind locally
> +     or to the other unit.  */
> +  if (!testcount && !test2count)
> +    abort();
> +  tt();
> +
> +  if ((testcount != 1 || test2count != 3)
> +      && (testcount != 3 || test2count != 1))
> +    abort ();
> +  reutrn 0;
^^^^^^^^^^^^^^^^^ typo
> +}

return 0;

You probably should run the testcases before committing them.

Thanks, David

Re: Another AIX Bootstrap failure

[Jan Hubicka]

Hello,
after some lengthly investigation it turned out that aliases on AIX doesn't
behave in the way we expect.  In particular creating a static alias of a global
symbol has no effect. This is somewhat special behaviour of AIX's .set
pseudo-op I think I can get this fixed by simply emitting alternative symbols
into every definition instead of relying on semantic of assembler's .set.

This patch disables aliases for !SUPPORTS_ONE_ONLY targets (I hope this
to be turned back soon after we fix AIX output macros) and adds testcase
for weird behavoiur of aliases so we can possibly catch other targets
that do not behave as expected.

Bootstrapped/regtested x86_64-linux.

	* gcc.dg/localalias.c: New testcase.
	* gcc.dg/localalias-2.c: New testcase.
	* gcc.dg/globalalias.c: New testcase.
	* gcc.dg/globalalias-2.c: New testcase.
	* ipa-visibility.c (function_and_variable_visibility): Disable
	temporarily local aliases for some targets.

Index: testsuite/gcc.dg/localalias.c
===================================================================
--- testsuite/gcc.dg/localalias.c	(revision 0)
+++ testsuite/gcc.dg/localalias.c	(revision 0)
@@ -0,0 +1,42 @@
+/* This test checks that local aliases behave sanely.  This is necessary for code correctness
+   of aliases introduced by ipa-visibility pass.
+
+   If this test fails either aliases needs to be disabled on given target on aliases with
+   proper semantic needs to be implemented.  This is problem with e.g. AIX .set pseudo-op
+   that implementes alias syntactically (by substituting in assembler) rather as alternative
+   symbol defined on a target's location.  */
+
+/* { dg-do run }
+   { dg-options "-Wstrict-aliasing=2 -fstrict-aliasing" } 
+   { dg-require-alias "" }
+   { dg-xfail-if "" { powerpc-ibm-aix* } { "*" } { "" } } 
+   { dg-additional-sources "localalias-2.c" } */
+extern void abort (void);
+extern int test2count;
+int testcount;
+__attribute__ ((weak,noinline))
+void test(void)
+{
+  testcount++;
+}
+__attribute ((alias("test")))
+static void test2(void);
+
+void main()
+{
+  test2();
+  /* This call must bind locally.  */
+  if (!testcount)
+    abort ();
+  test();
+  /* Depending on linker choice, this one may bind locally
+     or to the other unit.  */
+  if (!testcount && !test2count)
+    abort();
+  tt();
+
+  if ((testcount != 1 || test2count != 3)
+      && (testcount != 3 || test2count != 1))
+    abort ();
+  reutrn 0;
+}
Index: testsuite/gcc.dg/globalalias.c
===================================================================
--- testsuite/gcc.dg/globalalias.c	(revision 0)
+++ testsuite/gcc.dg/globalalias.c	(revision 0)
@@ -0,0 +1,42 @@
+/* This test checks that local aliases behave sanely.  This is necessary for code correctness
+   of aliases introduced by ipa-visibility pass.
+
+   This test expose weird behaviour of AIX's .set pseudo-op where the global symbol is created,
+   but all uses of the alias are syntactically replaced by uses of the target.  This means that
+   both counters are increased to 2.  */
+
+/* { dg-do run }
+   { dg-options "-O2" } 
+   { dg-require-alias "" }
+   { dg-xfail-if "" { powerpc-ibm-aix* } { "*" } { "" } } 
+   { dg-additional-sources "globalalias-2.c" } */
+extern int test2count;
+extern void abort (void);
+int testcount;
+static
+void test(void)
+{
+  testcount++;
+}
+__attribute__ ((weak,noinline))
+__attribute ((alias("test")))
+void test2(void);
+
+void main()
+{
+  test();
+  /* This call must bind locally.  */
+  if (!testcount)
+    abort ();
+  test2();
+  /* Depending on linker choice, this one may bind locally
+     or to the other unit.  */
+  if (!testcount && !test2count)
+    abort();
+  tt();
+
+  if ((testcount != 1 || test2count != 3)
+      && (testcount != 3 || test2count != 1))
+    abort ();
+  return 0;
+}
Index: testsuite/gcc.dg/globalalias-2.c
===================================================================
--- testsuite/gcc.dg/globalalias-2.c	(revision 0)
+++ testsuite/gcc.dg/globalalias-2.c	(revision 0)
@@ -0,0 +1,20 @@
+int test2count;
+extern void abort (void);
+static
+void test(void)
+{
+  test2count++;
+}
+__attribute__ ((weak,noinline))
+__attribute ((alias("test")))
+void test2(void);
+
+void tt() 
+{
+  int prev = test2count;
+  /* This call must bind locally.  */
+  test();
+  if (test2count == prev)
+    abort();
+  test2();
+ }
Index: testsuite/gcc.dg/localalias-2.c
===================================================================
--- testsuite/gcc.dg/localalias-2.c	(revision 0)
+++ testsuite/gcc.dg/localalias-2.c	(revision 0)
@@ -0,0 +1,19 @@
+extern void abort (void);
+int test2count;
+__attribute__ ((weak,noinline))
+void test(void)
+{
+  test2count++;
+}
+__attribute ((alias("test")))
+static void test2(void);
+
+void tt() 
+{
+  int prev = test2count;
+  /* This call must bind locally.  */
+  test2();
+  if (test2count == prev)
+    abort();
+  test();
+ }
Index: ipa-visibility.c
===================================================================
--- ipa-visibility.c	(revision 211858)
+++ ipa-visibility.c	(working copy)
@@ -566,7 +566,11 @@ function_and_variable_visibility (bool w
 	 cheaper and enable more optimization.
 
 	 TODO: We can also update virtual tables.  */
-      if (node->callers && can_replace_by_local_alias (node))
+      if (node->callers 
+          /* FIXME: currently this optimization breaks on AIX.  Disable it for targets
+             without comdat support for now.  */
+	  && SUPPORTS_ONE_ONLY
+	  && can_replace_by_local_alias (node))
 	{
 	  struct cgraph_node *alias = cgraph (symtab_nonoverwritable_alias (node));
 

Re: Another AIX Bootstrap failure

[David Edelsohn]

On Tue, Jun 17, 2014 at 12:50 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > To avoid using PLT and GOT when the unit refers to the symbol and we know
>> > that interposition does not matter.
>>
>> I am not certain if the linker is creating the PLT stub code because
>> it wants to allow interpolation or because it cannot see a definition
>> of the function and wants to allow for some other shared library to
>> provide the definition at runtime.
>
> OK, but the definition appears in the same file..
>>
>> > Why branch to a non-global (static) symbol
>> >   b ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
>> > leads to PLT stub here and why branching to such symbols seems to work otherwise?
>>
>> Branching to non-global (static) symbol, even an alias, is working
>> here. The weak function seems to be the problem.

The weak function is the problem, but I don't know why.  And I don't
understand how this is different than past uses of weak functions.  Or
is that new?

This is very confusing because the library, libstdc++, is being linked
statically. It provides a weak definition of the function. There
should be no glink code (PLT stub).  If the function is declared
.lglobl, it is called directly and no PLT stub is created.  I need to
call in the help of the AIX linker expert to figure out why it is
inserting PLT stub code, especially when linking statically.

Thanks, David

Re: Another AIX Bootstrap failure

[Jan Hubicka]

> > To avoid using PLT and GOT when the unit refers to the symbol and we know
> > that interposition does not matter.
> 
> I am not certain if the linker is creating the PLT stub code because
> it wants to allow interpolation or because it cannot see a definition
> of the function and wants to allow for some other shared library to
> provide the definition at runtime.

OK, but the definition appears in the same file..
> 
> > Why branch to a non-global (static) symbol
> >   b ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
> > leads to PLT stub here and why branching to such symbols seems to work otherwise?
> 
> Branching to non-global (static) symbol, even an alias, is working
> here. The weak function seems to be the problem.
> 
> > The failing branch is
> >>         b ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
> > so the call to static construction seems to have happened correctly but we can
> > not get right the call from the constructor to static function (that is an alias
> > of a global symbol)
> 
> The linker appears to not want to resolve the weak function. If I
> change ._ZN14__gnu_parallel9_SettingsC1Ev to lglobl, it works. If I
> change the static constructor to call the weak function directly,
> avoiding the alias, it shows the same failure mode.
> 
> I don't know what code generation looked like before.  Was GCC
> generating calls to weak functions within the same file?

Yes, this is how you implement COMDAT functions, right?  I looked at rs6000 call
expansion and it does not seem to care about visibility properties (just about
direct wrt indirect call).

One problem I can think of is a scenario where linked unify calls comdat functoins
in between units somehow forgetting about the aliases, but this function seems to
not be shared.
Index: symtab.c
===================================================================
--- symtab.c    (revision 211693)
+++ symtab.c    (working copy)
@@ -1327,10 +1327,8 @@
                               (void *)&new_node, true);
   if (new_node)
     return new_node;
-#ifndef ASM_OUTPUT_DEF
   /* If aliases aren't supported by the assembler, fail.  */
   return NULL;
-#endif
 
   /* Otherwise create a new one.  */
   new_decl = copy_node (node->decl);

disable generation of the local aliases completely.  I do not see much of difference
in the actual codegen with this...
I will check older GCC

Honza
> 
> Thanks, David

Re: Another AIX Bootstrap failure

[David Edelsohn]

On Mon, Jun 16, 2014 at 11:44 PM, Jan Hubicka <hubicka@ucw.cz> wrote:

>> The linker is not seeing the local definition of
>> ._ZN14__gnu_parallel9_SettingsC1Ev.  libstdc++ is built with
>> Linux-like semantics, so it allows symbols to be overridden. AIX calls
>> everything through the PLT. But the real definition of the function is
>
> Even static functions?
>
>> not being seen.
>>
>> I'm not exactly sure why inlining changing this and what these extra
>> levels of indirections are trying to accomplish. The visibility of the
>
> To avoid using PLT and GOT when the unit refers to the symbol and we know
> that interposition does not matter.

I am not certain if the linker is creating the PLT stub code because
it wants to allow interpolation or because it cannot see a definition
of the function and wants to allow for some other shared library to
provide the definition at runtime.

> Why branch to a non-global (static) symbol
>   b ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
> leads to PLT stub here and why branching to such symbols seems to work otherwise?

Branching to non-global (static) symbol, even an alias, is working
here. The weak function seems to be the problem.

> The failing branch is
>>         b ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
> so the call to static construction seems to have happened correctly but we can
> not get right the call from the constructor to static function (that is an alias
> of a global symbol)

The linker appears to not want to resolve the weak function. If I
change ._ZN14__gnu_parallel9_SettingsC1Ev to lglobl, it works. If I
change the static constructor to call the weak function directly,
avoiding the alias, it shows the same failure mode.

I don't know what code generation looked like before.  Was GCC
generating calls to weak functions within the same file?

Thanks, David

Re: Another AIX Bootstrap failure

[Jan Hubicka]

David,
thanks for the analysis!
> Without the ipa-inline-analysis.c change, g++ creates a static
> constructor with global visibility
> 
>         .globl ._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0
> 
> ._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0:
> 
> With the patch, g++ creates weak method
> 
>         .weak   ._ZN14__gnu_parallel9_SettingsC1Ev
> ._ZN14__gnu_parallel9_SettingsC1Ev:
> 
> with non-global alias
> 
>         .lglobl ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
>         .set
> ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0,._ZN14__gnu_parallel9_SettingsC1Ev
> 
> and the static constructor branches to the alias
> 
>         .globl ._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0
> ._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0:
>         lwz 3,LC..8(2)
>         b ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
> 
> The code where it's hanging is the AIX "glink" code, essentially a PLT
> stub, trying to call the method ._ZN14__gnu_parallel9_SettingsC1Ev

I see, one can get this with -fno-inline more easily and probably it affects
4.9, too.
> 
> The linker is not seeing the local definition of
> ._ZN14__gnu_parallel9_SettingsC1Ev.  libstdc++ is built with
> Linux-like semantics, so it allows symbols to be overridden. AIX calls
> everything through the PLT. But the real definition of the function is

Even static functions?

> not being seen.
> 
> I'm not exactly sure why inlining changing this and what these extra
> levels of indirections are trying to accomplish. The visibility of the

To avoid using PLT and GOT when the unit refers to the symbol and we know
that interposition does not matter.

Why branch to a non-global (static) symbol
  b ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
leads to PLT stub here and why branching to such symbols seems to work otherwise? 

> symbols as declared in the XCOFF assembly files appears to be
> preventing the AIX linker and loader from resolving the static
> constructor functions.

The failing branch is
>         b ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
so the call to static construction seems to have happened correctly but we can
not get right the call from the constructor to static function (that is an alias
of a global symbol)

I can get simliar code with -fno-inline.  Here we apparently handle first call
correctly but fail in the second call. First is to static function other
is to static alias of global function. Both should result in same code but doesn't

One is:
        .lglobl ._GLOBAL__sub_I_parallel_settings.cc
        .csect _GLOBAL__sub_I_parallel_settings.cc[DS]
_GLOBAL__sub_I_parallel_settings.cc:
        .long ._GLOBAL__sub_I_parallel_settings.cc, TOC[tc0], 0
        .csect ..text.startup[PR],2
._GLOBAL__sub_I_parallel_settings.cc:

Other is
        .csect ._ZN14__gnu_parallel9_SettingsC1Ev[PR],2
        .align 2
        .align 4
        .weak   _ZN14__gnu_parallel9_SettingsC1Ev[DS]
        .weak   ._ZN14__gnu_parallel9_SettingsC1Ev
        .csect _ZN14__gnu_parallel9_SettingsC1Ev[DS]
_ZN14__gnu_parallel9_SettingsC1Ev:
        .long ._ZN14__gnu_parallel9_SettingsC1Ev, TOC[tc0], 0
        .csect ._ZN14__gnu_parallel9_SettingsC1Ev[PR],2
._ZN14__gnu_parallel9_SettingsC1Ev:
        .bi     "/home/jh/trunk/build/powerpc-ibm-aix7.1.0.0/libstdc++-v3/include/parallel/settings.h"
        .stabx  "_ZN14__gnu_parallel9_SettingsC1Ev:F-11",._ZN14__gnu_parallel9_SettingsC1Ev,142,0
        .function ._ZN14__gnu_parallel9_SettingsC1Ev,._ZN14__gnu_parallel9_SettingsC1Ev,16,044,FE.._ZN14__gnu_parallel9_SettingsC1Ev-._ZN14__gnu_parallel9_SettingsC1Ev
....
FE.._ZN14__gnu_parallel9_SettingsC1Ev:
LFE..146:
        .lglobl ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
        .lglobl _ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
        .set    ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0,._ZN14__gnu_parallel9_SettingsC1Ev
        .set _ZN14__gnu_parallel9_SettingsC1Ev.localalias.0,_ZN14__gnu_parallel9_SettingsC1Ev

Let me see if I can derive something more self contained.

Honza
> 
> Thanks David

Re: Another AIX Bootstrap failure

[David Edelsohn]

[-- Attachment #1: Type: text/plain, Size: 3736 bytes --]

Without the ipa-inline-analysis.c change, g++ creates a static
constructor with global visibility

        .globl ._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0

._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0:

With the patch, g++ creates weak method

        .weak   ._ZN14__gnu_parallel9_SettingsC1Ev
._ZN14__gnu_parallel9_SettingsC1Ev:

with non-global alias

        .lglobl ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
        .set
._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0,._ZN14__gnu_parallel9_SettingsC1Ev

and the static constructor branches to the alias

        .globl ._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0
._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0:
        lwz 3,LC..8(2)
        b ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0

The code where it's hanging is the AIX "glink" code, essentially a PLT
stub, trying to call the method ._ZN14__gnu_parallel9_SettingsC1Ev

The linker is not seeing the local definition of
._ZN14__gnu_parallel9_SettingsC1Ev.  libstdc++ is built with
Linux-like semantics, so it allows symbols to be overridden. AIX calls
everything through the PLT. But the real definition of the function is
not being seen.

I'm not exactly sure why inlining changing this and what these extra
levels of indirections are trying to accomplish. The visibility of the
symbols as declared in the XCOFF assembly files appears to be
preventing the AIX linker and loader from resolving the static
constructor functions.

Thanks David


On Mon, Jun 16, 2014 at 6:06 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Mon, Jun 16, 2014 at 12:35 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>
>> > @@ -512,9 +516,9 @@ function_and_variable_visibility (bool w
>> >                      next = next->same_comdat_group)
>> >                 {
>> >                   next->set_comdat_group (NULL);
>> > -                 if (!next->alias)
>> > -                   next->set_section (NULL);
>> >                   symtab_make_decl_local (next->decl);
>> > +                 if (!node->alias)
>> > +                   node->reset_section ();
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> >                   next->unique_name = ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
>> >                                         || next->unique_name
>> >                                         || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
>>
>> Honza, did you really intend to change the above from next->alias and
>> next->set_section () to node->alias and node->reset_section () ?  That
>> doesn't look right.
>
> You are right, this is a pasto in anoying code duplication here.  I fixed that
> in my local copy of the patch.  I have a followup patch in the ipa-visibility
> TLC to merge the code duplication here, I am just holding it until we debug the
> issues.
>
> This bug will not affect AIX, becuase there are no comdat groups.  It only
> wastes a bit of code size on ELF systems because of extra alignment coming
> from the named section.
>
> The hang happens in
> (gdb) bt
> #0  0x100be244 in __gnu_parallel::_Settings::_Settings() ()
> #1  0x10008d54 in _GLOBAL__FI_libstdc___so ()
> #2  0x10008e88 in _GLOBAL__AIXI_libstdc___so ()
> #3  0x100be954 in _GLOBAL__FI_genconstants ()
> #4  0xd017fa54 in mod_init1 () from /usr/lib/libc.a(shr.o)
> #5  0xd017f774 in __modinit () from /usr/lib/libc.a(shr.o)
> #6  0x100001a0 in __start ()
>
> I suppose it may be crtbegin/crtend miscompile.  Any insight would be welcome,
> otherwise I will try to progress on debugging tonight or tomorrow.
>
> Honza

[-- Attachment #2: parallel_settings.bad.s --]
[-- Type: application/octet-stream, Size: 5923 bytes --]

	.file	"/home/dje/src/src/libstdc++-v3/src/c++98/parallel_settings.cc"
	.csect .text[PR]
	.toc
	.csect .text[PR]
	.toc
LC..2:
	.tc LANCHOR..0[TC],LANCHOR..0
	.csect ._ZN14__gnu_parallel9_SettingsC1Ev[PR],2
	.align 2
	.align 4
	.weak	_ZN14__gnu_parallel9_SettingsC1Ev[DS]
	.weak	._ZN14__gnu_parallel9_SettingsC1Ev
	.csect _ZN14__gnu_parallel9_SettingsC1Ev[DS]
_ZN14__gnu_parallel9_SettingsC1Ev:
	.long ._ZN14__gnu_parallel9_SettingsC1Ev, TOC[tc0], 0
	.csect ._ZN14__gnu_parallel9_SettingsC1Ev[PR],2
._ZN14__gnu_parallel9_SettingsC1Ev:
LFB..146:
	lwz 8,LC..2(2)
	li 5,0
	li 10,0
	li 11,1000
	li 9,1
	stw 13,-76(1)
LCFI..0:
	li 6,1000
	stw 5,0(3)
	stw 5,4(3)
	stw 5,12(3)
	stw 9,8(3)
	lfs 0,0(8)
	stw 9,16(3)
	li 7,10
	stw 9,20(3)
	stw 9,24(3)
	stw 9,28(3)
	stw 6,40(3)
	stw 10,32(3)
	stw 11,36(3)
	stw 10,48(3)
	stw 11,52(3)
	stw 10,56(3)
	stw 11,60(3)
	stfd 0,64(3)
	stw 10,96(3)
	li 12,0
	stw 11,100(3)
	stw 10,104(3)
	stw 11,108(3)
	stw 10,112(3)
	stw 12,72(3)
	stw 12,88(3)
	stw 11,116(3)
	stw 10,120(3)
	stw 11,124(3)
	li 13,256
	li 4,0
	li 5,8192
	stw 13,76(3)
	stw 13,92(3)
	li 9,2
	stw 4,80(3)
	stw 5,84(3)
	stw 7,128(3)
	lfd 0,8(8)
	lfs 12,16(8)
	stw 9,152(3)
	li 9,100
	stw 7,156(3)
	stw 6,204(3)
	stw 6,208(3)
	stw 10,136(3)
	stw 11,140(3)
	stw 10,144(3)
	stw 11,148(3)
	stw 10,160(3)
	stfd 0,176(3)
	stw 11,164(3)
	lfs 0,20(8)
	stw 10,168(3)
	li 8,0
	stw 11,172(3)
	stw 10,184(3)
	li 6,0
	stw 11,188(3)
	stw 10,192(3)
	stw 11,196(3)
	stfs 12,200(3)
	stw 10,216(3)
	stw 11,220(3)
	stw 10,224(3)
	stw 11,228(3)
	stw 10,232(3)
	stw 11,236(3)
	stw 10,240(3)
	stw 11,244(3)
	stw 10,248(3)
	stw 11,252(3)
	stw 10,256(3)
	stw 11,260(3)
	stw 7,264(3)
	stw 9,276(3)
	stw 9,300(3)
	stw 8,272(3)
	stw 8,296(3)
	lwz 13,-76(1)
	li 9,16384
	stw 10,280(3)
	stw 11,284(3)
	stw 6,288(3)
	stw 10,336(3)
	stw 9,308(3)
	stw 11,340(3)
	stfs 0,344(3)
	lis 9,0x4
	li 8,0
	li 7,100
	stw 9,316(3)
	li 9,128
	stw 8,304(3)
	li 8,0
	stw 7,268(3)
	stw 9,320(3)
	li 9,64
	stw 8,312(3)
	li 7,10000
	stw 9,324(3)
	li 8,0
	li 9,0
	stw 7,292(3)
	stw 8,328(3)
	stw 9,332(3)
LCFI..1:
	blr
LT.._ZN14__gnu_parallel9_SettingsC1Ev:
	.long 0
	.byte 0,9,32,64,0,19,1,0
	.long 0
	.long LT.._ZN14__gnu_parallel9_SettingsC1Ev-._ZN14__gnu_parallel9_SettingsC1Ev
	.short 33
	.byte "_ZN14__gnu_parallel9_SettingsC1Ev"
	.align 2
LFE..146:
	.lglobl	._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
	.lglobl	_ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
	.set	._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0,._ZN14__gnu_parallel9_SettingsC1Ev
	.set _ZN14__gnu_parallel9_SettingsC1Ev.localalias.0,_ZN14__gnu_parallel9_SettingsC1Ev
	.toc
LC..6:
	.tc _ZN12_GLOBAL__N_11sE[TC],_ZN12_GLOBAL__N_11sE
	.csect ._ZN14__gnu_parallel9_Settings3getEv[PR],2
	.align 2
	.align 4
	.globl _ZN14__gnu_parallel9_Settings3getEv
	.globl ._ZN14__gnu_parallel9_Settings3getEv
	.csect _ZN14__gnu_parallel9_Settings3getEv[DS]
_ZN14__gnu_parallel9_Settings3getEv:
	.long ._ZN14__gnu_parallel9_Settings3getEv, TOC[tc0], 0
	.csect ._ZN14__gnu_parallel9_Settings3getEv[PR],2
._ZN14__gnu_parallel9_Settings3getEv:
LFB..147:
	lwz 3,LC..6(2)
	blr
LT.._ZN14__gnu_parallel9_Settings3getEv:
	.long 0
	.byte 0,9,32,64,0,0,0,0
	.long LT.._ZN14__gnu_parallel9_Settings3getEv-._ZN14__gnu_parallel9_Settings3getEv
	.short 35
	.byte "_ZN14__gnu_parallel9_Settings3getEv"
	.align 2
LFE..147:
	.toc
	.set LC..7,LC..6
	.csect ._ZN14__gnu_parallel9_Settings3setERS0_[PR],2
	.align 2
	.align 4
	.globl _ZN14__gnu_parallel9_Settings3setERS0_
	.globl ._ZN14__gnu_parallel9_Settings3setERS0_
	.csect _ZN14__gnu_parallel9_Settings3setERS0_[DS]
_ZN14__gnu_parallel9_Settings3setERS0_:
	.long ._ZN14__gnu_parallel9_Settings3setERS0_, TOC[tc0], 0
	.csect ._ZN14__gnu_parallel9_Settings3setERS0_[PR],2
._ZN14__gnu_parallel9_Settings3setERS0_:
LFB..148:
	mflr 0
	mr 4,3
	lwz 3,LC..7(2)
	li 5,348
	stw 0,8(1)
	stwu 1,-64(1)
LCFI..2:
	bl .memcpy
	nop
	addi 1,1,64
LCFI..3:
	lwz 0,8(1)
	mtlr 0
LCFI..4:
	blr
LT.._ZN14__gnu_parallel9_Settings3setERS0_:
	.long 0
	.byte 0,9,32,65,128,0,1,0
	.long 0
	.long LT.._ZN14__gnu_parallel9_Settings3setERS0_-._ZN14__gnu_parallel9_Settings3setERS0_
	.short 38
	.byte "_ZN14__gnu_parallel9_Settings3setERS0_"
	.align 2
LFE..148:
	.toc
	.set LC..8,LC..6
	.csect ..text.startup._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0[PR],2
	.align 2
	.align 4
	.globl _GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0
	.globl ._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0
	.csect _GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0[DS]
_GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0:
	.long ._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0, TOC[tc0], 0
	.csect ..text.startup._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0[PR],2
._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0:
LFB..151:
	lwz 3,LC..8(2)
	b ._ZN14__gnu_parallel9_SettingsC1Ev.localalias.0
LT.._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0:
	.long 0
	.byte 0,9,32,64,0,0,0,0
	.long LT.._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0-._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0
	.short 91
	.byte "_GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0"
	.align 2
LFE..151:
	.lcomm _ZN12_GLOBAL__N_11sE,352,_parallelsettings.bss_,3
	.csect _parallelsettings.rw_[RO],4
	.align 3
	.set LANCHOR..0,$ + 0
LC..1:
	.vbyte	4,1073741824
	.space 4
LC..3:
	.vbyte	4,0
	.vbyte	4,0
LC..4:
	.vbyte	4,1065353216
LC..5:
	.vbyte	4,1008981770
	.csect .text[PR]
_section_.text:
	.csect .data[RW],4
	.long _section_.text

[-- Attachment #3: parallel_settings.good.s --]
[-- Type: application/octet-stream, Size: 5020 bytes --]

	.file	"/home/dje/src/src/libstdc++-v3/src/c++98/parallel_settings.cc"
	.csect .text[PR]
	.toc
	.csect .text[PR]
	.toc
LC..0:
	.tc _ZN12_GLOBAL__N_11sE[TC],_ZN12_GLOBAL__N_11sE
	.csect ._ZN14__gnu_parallel9_Settings3getEv[PR],2
	.align 2
	.align 4
	.globl _ZN14__gnu_parallel9_Settings3getEv
	.globl ._ZN14__gnu_parallel9_Settings3getEv
	.csect _ZN14__gnu_parallel9_Settings3getEv[DS]
_ZN14__gnu_parallel9_Settings3getEv:
	.long ._ZN14__gnu_parallel9_Settings3getEv, TOC[tc0], 0
	.csect ._ZN14__gnu_parallel9_Settings3getEv[PR],2
._ZN14__gnu_parallel9_Settings3getEv:
LFB..147:
	lwz 3,LC..0(2)
	blr
LT.._ZN14__gnu_parallel9_Settings3getEv:
	.long 0
	.byte 0,9,32,64,0,0,0,0
	.long LT.._ZN14__gnu_parallel9_Settings3getEv-._ZN14__gnu_parallel9_Settings3getEv
	.short 35
	.byte "_ZN14__gnu_parallel9_Settings3getEv"
	.align 2
LFE..147:
	.toc
	.set LC..1,LC..0
	.csect ._ZN14__gnu_parallel9_Settings3setERS0_[PR],2
	.align 2
	.align 4
	.globl _ZN14__gnu_parallel9_Settings3setERS0_
	.globl ._ZN14__gnu_parallel9_Settings3setERS0_
	.csect _ZN14__gnu_parallel9_Settings3setERS0_[DS]
_ZN14__gnu_parallel9_Settings3setERS0_:
	.long ._ZN14__gnu_parallel9_Settings3setERS0_, TOC[tc0], 0
	.csect ._ZN14__gnu_parallel9_Settings3setERS0_[PR],2
._ZN14__gnu_parallel9_Settings3setERS0_:
LFB..148:
	mflr 0
	mr 4,3
	lwz 3,LC..1(2)
	li 5,348
	stw 0,8(1)
	stwu 1,-64(1)
LCFI..0:
	bl .memcpy
	nop
	addi 1,1,64
LCFI..1:
	lwz 0,8(1)
	mtlr 0
LCFI..2:
	blr
LT.._ZN14__gnu_parallel9_Settings3setERS0_:
	.long 0
	.byte 0,9,32,65,128,0,1,0
	.long 0
	.long LT.._ZN14__gnu_parallel9_Settings3setERS0_-._ZN14__gnu_parallel9_Settings3setERS0_
	.short 38
	.byte "_ZN14__gnu_parallel9_Settings3setERS0_"
	.align 2
LFE..148:
	.toc
	.set LC..2,LC..0
LC..5:
	.tc LANCHOR..0[TC],LANCHOR..0
	.csect ..text.startup._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0[PR],2
	.align 2
	.align 4
	.globl _GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0
	.globl ._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0
	.csect _GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0[DS]
_GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0:
	.long ._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0, TOC[tc0], 0
	.csect ..text.startup._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0[PR],2
._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0:
LFB..151:
	lwz 7,LC..5(2)
	lwz 9,LC..2(2)
	li 10,0
	li 11,1000
	stw 13,-76(1)
	stw 30,-8(1)
	li 8,1
	li 4,0
	stw 31,-4(1)
LCFI..3:
	li 5,1000
	lfs 0,0(7)
	li 12,0
	li 13,256
	li 30,0
	li 31,8192
	stw 8,8(9)
	stw 8,16(9)
	stw 8,20(9)
	stw 8,24(9)
	stw 8,28(9)
	stw 12,72(9)
	stw 13,76(9)
	stw 30,80(9)
	stw 31,84(9)
	stw 12,88(9)
	stfd 0,64(9)
	stw 13,92(9)
	stw 4,0(9)
	stw 4,4(9)
	li 6,10
	stw 4,12(9)
	stw 10,32(9)
	stw 11,36(9)
	stw 5,40(9)
	stw 10,48(9)
	stw 11,52(9)
	stw 10,56(9)
	stw 11,60(9)
	stw 10,96(9)
	stw 11,100(9)
	stw 10,104(9)
	stw 11,108(9)
	stw 10,112(9)
	stw 11,116(9)
	lfd 0,8(7)
	lfs 12,16(7)
	li 8,2
	stw 6,128(9)
	stw 6,156(9)
	stw 10,120(9)
	stw 11,124(9)
	stw 8,152(9)
	stw 10,136(9)
	stw 11,140(9)
	stw 10,144(9)
	stw 11,148(9)
	stw 10,160(9)
	stfd 0,176(9)
	stw 11,164(9)
	lfs 0,20(7)
	stw 10,168(9)
	li 7,10000
	stw 11,172(9)
	stw 10,184(9)
	li 13,100
	stw 11,188(9)
	stw 10,192(9)
	stw 11,196(9)
	stfs 12,200(9)
	stw 5,204(9)
	stw 5,208(9)
	li 8,100
	stw 10,216(9)
	stw 11,220(9)
	stw 10,224(9)
	stw 11,228(9)
	stw 10,232(9)
	stw 11,236(9)
	stw 10,240(9)
	stw 11,244(9)
	stw 10,248(9)
	stw 11,252(9)
	stw 6,264(9)
	stw 7,292(9)
	stw 8,268(9)
	stw 13,276(9)
	stw 13,300(9)
	lwz 30,-8(1)
	li 6,0
	lwz 13,-76(1)
	lwz 31,-4(1)
	li 7,16384
	li 8,128
	stw 10,256(9)
	stw 11,260(9)
	stw 6,288(9)
	stw 10,280(9)
	stw 7,308(9)
	stw 8,320(9)
	stw 11,284(9)
	stw 10,336(9)
	stw 11,340(9)
	stfs 0,344(9)
	li 6,0
	lis 7,0x4
	li 12,0
	stw 6,304(9)
	li 6,0
	li 8,64
	stw 7,316(9)
	li 7,0
	stw 12,272(9)
	stw 6,312(9)
	stw 12,296(9)
	stw 8,324(9)
	stw 7,332(9)
	li 6,0
	stw 6,328(9)
LCFI..4:
	blr
LT.._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0:
	.long 0
	.byte 0,9,32,64,0,19,0,0
	.long LT.._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0-._GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0
	.short 91
	.byte "_GLOBAL__I_65535_0__home_dje_src_src_libstdc___v3_src_c__98_parallel_settings.cc_2984A295_0"
	.align 2
LFE..151:
	.lcomm _ZN12_GLOBAL__N_11sE,352,_parallelsettings.bss_,3
	.csect _parallelsettings.rw_[RO],4
	.align 3
	.set LANCHOR..0,$ + 0
LC..4:
	.vbyte	4,1073741824
	.space 4
LC..6:
	.vbyte	4,0
	.vbyte	4,0
LC..7:
	.vbyte	4,1065353216
LC..8:
	.vbyte	4,1008981770
	.csect .text[PR]
_section_.text:
	.csect .data[RW],4
	.long _section_.text

Re: Another AIX Bootstrap failure

[David Edelsohn]

It looks like _Settings is miscompiled, or more specifically a
function descriptor has a bogus value

   0x100bcbac <_ZN14__gnu_parallel9_SettingsC1Ev>:      addi    r12,r2,-168
   0x100bcbb0 <_ZN14__gnu_parallel9_SettingsC1Ev+4>:    stw     r2,20(r1)
   0x100bcbb4 <_ZN14__gnu_parallel9_SettingsC1Ev+8>:    lwz     r0,0(r12)
   0x100bcbb8 <_ZN14__gnu_parallel9_SettingsC1Ev+12>:   lwz     r2,4(r12)
   0x100bcbbc <_ZN14__gnu_parallel9_SettingsC1Ev+16>:   mtctr   r0
=> 0x100bcbc0 <_ZN14__gnu_parallel9_SettingsC1Ev+20>:   bctr

(gdb) x/x $r2-168
0x3002f90c <__dbargs+11440>:    0x100bcbac
(gdb) x/x $r12
0x3002f90c <__dbargs+11440>:    0x100bcbac
(gdb) p/x $ctr
$5 = 0x100bcbac

For some reason, the additional inlining is creating the wrong address
for the target.

- David



On Mon, Jun 16, 2014 at 6:06 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Mon, Jun 16, 2014 at 12:35 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>
>> > @@ -512,9 +516,9 @@ function_and_variable_visibility (bool w
>> >                      next = next->same_comdat_group)
>> >                 {
>> >                   next->set_comdat_group (NULL);
>> > -                 if (!next->alias)
>> > -                   next->set_section (NULL);
>> >                   symtab_make_decl_local (next->decl);
>> > +                 if (!node->alias)
>> > +                   node->reset_section ();
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> >                   next->unique_name = ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
>> >                                         || next->unique_name
>> >                                         || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
>>
>> Honza, did you really intend to change the above from next->alias and
>> next->set_section () to node->alias and node->reset_section () ?  That
>> doesn't look right.
>
> You are right, this is a pasto in anoying code duplication here.  I fixed that
> in my local copy of the patch.  I have a followup patch in the ipa-visibility
> TLC to merge the code duplication here, I am just holding it until we debug the
> issues.
>
> This bug will not affect AIX, becuase there are no comdat groups.  It only
> wastes a bit of code size on ELF systems because of extra alignment coming
> from the named section.
>
> The hang happens in
> (gdb) bt
> #0  0x100be244 in __gnu_parallel::_Settings::_Settings() ()
> #1  0x10008d54 in _GLOBAL__FI_libstdc___so ()
> #2  0x10008e88 in _GLOBAL__AIXI_libstdc___so ()
> #3  0x100be954 in _GLOBAL__FI_genconstants ()
> #4  0xd017fa54 in mod_init1 () from /usr/lib/libc.a(shr.o)
> #5  0xd017f774 in __modinit () from /usr/lib/libc.a(shr.o)
> #6  0x100001a0 in __start ()
>
> I suppose it may be crtbegin/crtend miscompile.  Any insight would be welcome,
> otherwise I will try to progress on debugging tonight or tomorrow.
>
> Honza

Re: Another AIX Bootstrap failure

[David Edelsohn]

On Mon, Jun 16, 2014 at 5:44 PM, Jan Hubicka <hubicka@ucw.cz> wrote:

> If you don't mind, I would like to commit back the rest of changes (reset_section)
> cleanups after testing on AIX + testing the aforementioned ARM testcase.

Please go ahead and commit the parts of the patch that do not cause problems.

Thanks, DAvid

Re: Another AIX Bootstrap failure

[Jan Hubicka]

> On Mon, Jun 16, 2014 at 12:35 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> 
> > @@ -512,9 +516,9 @@ function_and_variable_visibility (bool w
> >                      next = next->same_comdat_group)
> >                 {
> >                   next->set_comdat_group (NULL);
> > -                 if (!next->alias)
> > -                   next->set_section (NULL);
> >                   symtab_make_decl_local (next->decl);
> > +                 if (!node->alias)
> > +                   node->reset_section ();
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >                   next->unique_name = ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
> >                                         || next->unique_name
> >                                         || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> 
> Honza, did you really intend to change the above from next->alias and
> next->set_section () to node->alias and node->reset_section () ?  That
> doesn't look right.

You are right, this is a pasto in anoying code duplication here.  I fixed that
in my local copy of the patch.  I have a followup patch in the ipa-visibility
TLC to merge the code duplication here, I am just holding it until we debug the
issues. 

This bug will not affect AIX, becuase there are no comdat groups.  It only
wastes a bit of code size on ELF systems because of extra alignment coming
from the named section.

The hang happens in 
(gdb) bt
#0  0x100be244 in __gnu_parallel::_Settings::_Settings() ()
#1  0x10008d54 in _GLOBAL__FI_libstdc___so ()
#2  0x10008e88 in _GLOBAL__AIXI_libstdc___so ()
#3  0x100be954 in _GLOBAL__FI_genconstants ()
#4  0xd017fa54 in mod_init1 () from /usr/lib/libc.a(shr.o)
#5  0xd017f774 in __modinit () from /usr/lib/libc.a(shr.o)
#6  0x100001a0 in __start ()

I suppose it may be crtbegin/crtend miscompile.  Any insight would be welcome,
otherwise I will try to progress on debugging tonight or tomorrow.

Honza

Re: Another AIX Bootstrap failure

[Jan Hubicka]

> Honza,
> 
> FYI, your bootstrap on gcc111 is hung in the exact same place as I
> have observed: all of the stage2 gen* programs spinning.

Yes (will kill it now), I was tracking down what change it is.
It is actually the inliner hunk that is independent of rest of changes
(and one that should be obviously safe, as famous last words):

Index: ipa-inline-analysis.c
===================================================================
--- ipa-inline-analysis.c	(revision 211488)
+++ ipa-inline-analysis.c	(working copy)
@@ -3877,7 +3877,7 @@ do_estimate_growth (struct cgraph_node *
       /* COMDAT functions are very often not shared across multiple units
          since they come from various template instantiations.
          Take this into account.  */
-      else if (DECL_COMDAT (node->decl)
+      else if (node->externally_visible && node->get_comdat_group ()
 	       && cgraph_can_remove_if_no_direct_calls_p (node))
 	d.growth -= (info->size
 		     * (100 - PARAM_VALUE (PARAM_COMDAT_SHARING_PROBABILITY))
@@ -3928,7 +3928,7 @@ growth_likely_positive (struct cgraph_no
       && (ret = node_growth_cache[node->uid]))
     return ret > 0;
   if (!cgraph_will_be_removed_from_program_if_no_direct_calls (node)
-      && (!DECL_COMDAT (node->decl)
+      && (!node->externally_visible || !node->get_comdat_group ()
 	  || !cgraph_can_remove_if_no_direct_calls_p (node)))
     return true;
   max_callers = inline_summary (node)->size * 4 / edge_growth + 2;

This hunk promotes more inlining of COMDAT functions in anticipation that
if they get inlined in each module independently, they will be optimized out
and that in C++ high percentage of comdat functions is actually used by
one unit only.

The change disables this heuristic tweek for targets without comdat support,
like AIX is. Here we do not want to compute probability of section sharing, 
becuase there are no comdat sections (and if there is other mechanism, we should
let middle-end know). The main motviation was however COMDAT locals on ELF
(introduced by cdtor decloning) where we want to handle them as statics
rather than COMDATs.

No mater of what these functions return, the final program should be correct-
they only affect inlining decisions, not correctness of the inliner.  We ought
to debug this.  Do you have any clue what goes wrong?

If you don't mind, I would like to commit back the rest of changes (reset_section)
cleanups after testing on AIX + testing the aforementioned ARM testcase. 

I will also make more AIX firendly version of this patch. But please, lets debug
the problem, so we don't have random wrong code issues here. 
Honza

Re: Another AIX Bootstrap failure

[David Edelsohn]

On Mon, Jun 16, 2014 at 12:35 AM, Jan Hubicka <hubicka@ucw.cz> wrote:

> @@ -512,9 +516,9 @@ function_and_variable_visibility (bool w
>                      next = next->same_comdat_group)
>                 {
>                   next->set_comdat_group (NULL);
> -                 if (!next->alias)
> -                   next->set_section (NULL);
>                   symtab_make_decl_local (next->decl);
> +                 if (!node->alias)
> +                   node->reset_section ();
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                   next->unique_name = ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
>                                         || next->unique_name
>                                         || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)

Honza, did you really intend to change the above from next->alias and
next->set_section () to node->alias and node->reset_section () ?  That
doesn't look right.

- David

Re: Another AIX Bootstrap failure

[David Edelsohn]

Honza,

FYI, your bootstrap on gcc111 is hung in the exact same place as I
have observed: all of the stage2 gen* programs spinning.

- David


On Mon, Jun 16, 2014 at 11:08 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Honza,
>>
>> Thanks for reverting the patch. I will check if this resolves the
>> current bootstrap problem.
>>
>> I was suggesting that you create a branch for all of the visibility
>> changes to make it easier to track the various original patches and
>> later correction patches from you.
>>
>> I don't know if the gen* programs hang because of the visibility
>> changes or because of the change in sections. The change in sections
>> could conflict with the GCC code to handle AIX XCOFF CSECTs for
>> functions.
>>
>> AIX recently added support for ELF-like visibility. AIX previously
>> supported the equivalent of visibility through "export" files. The
>> recent problems could be due to issues with assembly file ordering,
>> but they also could be related to the visibility changes affecting the
>> way that GCC emits code to branch to global functions.
>
> I comitted the revert now (my original testing got struct on ICE in
> auto-inc-dec pass that is unrelated).  I probably won't have time to analye
> what went wrong until Wednesday. The patch did not really play with
> ELF visibilities it was again related to bringing symbols local.
> I tried a case disabling the new conditional on clearning user section
> but that did not help. The patch basically collected few cleanups
> and fixes of corner case.  Last change is fix in the inline heuristics
> to not try to enale DECL_ONE_ONLY section sharing on targets not supporting
> it.  Obviously it should not lead to wrong code, since any inlining decision
> change should not, but I am testing it independnely now.
>
> Honza

Re: Another AIX Bootstrap failure

[Ramana Radhakrishnan]

On Mon, Jun 16, 2014 at 4:08 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Honza,
>>
>> Thanks for reverting the patch. I will check if this resolves the
>> current bootstrap problem.
>>
>> I was suggesting that you create a branch for all of the visibility
>> changes to make it easier to track the various original patches and
>> later correction patches from you.
>>
>> I don't know if the gen* programs hang because of the visibility
>> changes or because of the change in sections. The change in sections
>> could conflict with the GCC code to handle AIX XCOFF CSECTs for
>> functions.
>>
>> AIX recently added support for ELF-like visibility. AIX previously
>> supported the equivalent of visibility through "export" files. The
>> recent problems could be due to issues with assembly file ordering,
>> but they also could be related to the visibility changes affecting the
>> way that GCC emits code to branch to global functions.
>
> I comitted the revert now (my original testing got struct on ICE in
> auto-inc-dec pass that is unrelated).  I probably won't have time to analye
> what went wrong until Wednesday. The patch did not really play with
> ELF visibilities it was again related to bringing symbols local.
> I tried a case disabling the new conditional on clearning user section
> but that did not help. The patch basically collected few cleanups
> and fixes of corner case.  Last change is fix in the inline heuristics
> to not try to enale DECL_ONE_ONLY section sharing on targets not supporting
> it.  Obviously it should not lead to wrong code, since any inlining decision
> change should not, but I am testing it independnely now.

Can you please verify the testcase in PR61523 doesn't fail with your
reworked patch for arm-none-linux-gnueabihf ?

Thanks,
Ramana

>
> Honza

Re: Another AIX Bootstrap failure

[Jan Hubicka]

> Honza,
> 
> Thanks for reverting the patch. I will check if this resolves the
> current bootstrap problem.
> 
> I was suggesting that you create a branch for all of the visibility
> changes to make it easier to track the various original patches and
> later correction patches from you.
> 
> I don't know if the gen* programs hang because of the visibility
> changes or because of the change in sections. The change in sections
> could conflict with the GCC code to handle AIX XCOFF CSECTs for
> functions.
> 
> AIX recently added support for ELF-like visibility. AIX previously
> supported the equivalent of visibility through "export" files. The
> recent problems could be due to issues with assembly file ordering,
> but they also could be related to the visibility changes affecting the
> way that GCC emits code to branch to global functions.

I comitted the revert now (my original testing got struct on ICE in
auto-inc-dec pass that is unrelated).  I probably won't have time to analye
what went wrong until Wednesday. The patch did not really play with
ELF visibilities it was again related to bringing symbols local.
I tried a case disabling the new conditional on clearning user section
but that did not help. The patch basically collected few cleanups
and fixes of corner case.  Last change is fix in the inline heuristics
to not try to enale DECL_ONE_ONLY section sharing on targets not supporting
it.  Obviously it should not lead to wrong code, since any inlining decision
change should not, but I am testing it independnely now.

Honza

Re: Another AIX Bootstrap failure

[Ramana Radhakrishnan]

On Mon, Jun 16, 2014 at 5:35 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Honza,
>>
>> The cgraph patch in r211600 broke AIX bootstrap again.  I cannot find
>> the corresponding patch in the GCC Patches mailing list, so I do not
>> see where this was discussed or approved.
>
> Sorry, I remember writting mail about this patch but also can't find it.  The
> patch introduces reset_section that is used when function or variable is
> brought local by the visibility code. In this case we previously incorrectly
> cleared user sections and we forgot to introduce implicit sections with
> -fdata-section and -ffunction-section.
>
> I will revert this change and lets debug it.
>

It looks like this broke arm-none-linux-gnueabihf bootstraps with
section conflicts in strstream.c. Is that the kind of failures you are
seeing on AIX ?

/work/trunk-nightlies/builds/build-210919/armv7l-unknown-linux-gnueabihf/libstdc++-v3/include/backward/strstream:126:9:
error: std::istrstream::_ZTVSt10istrstream.localalias.0 causes a
section type conflict with std::istrstream::_ZTVSt10istrstream

 Can you please test it on this platform before reapplying / or let me
know when you have the smaller patches. I'll see what I can do.

regards
Ramana

>>
>> With the patch in r211600, the "gen*" programs in stage2 go into an
>> endless loop.
>>
>> Please revert these comdat patches.  These were not tested appropriately.
>>
>> Please create a branch and we can debug the problems on AIX.
>
> What patches you refer to? I already disabled the localization within
> initializers on AIX (well all targets w/o MAKE_ONE_ONLY support) and
> I am testing patch reverting this change (I want to keep reset section
> just remove the call to resolve unique section) and will commit once
> it converges.
>
> I will send you patch to enable those two changes and lets look into why
> they break. What is common to both patches is that they deal with static
> symbol in named sections, perhaps that is not correctly supported by AIX
> toolchain...
>
> My apologizes for the breakage. I am attaching the patch for reference.
> Honza
>
>
>         * symtab.c (symtab_node::reset_section): New method.
>         * cgraph.c (cgraph_node_cannot_be_local_p_1): Accept non-local
>         for localization.
>         * cgraph.h (reset_section): Declare.
>         * ipa-inline-analysis.c (do_estimate_growth): Check for comdat groups;
>         do not consider comdat locals.
>         * cgraphclones.c (set_new_clone_decl_and_node_flags): Get section
>         for new symbol.
>         * ipa-visiblity.c (cgraph_externally_visible_p): Cleanup.
>         (update_visibility_by_resolution_info): Consider UNDEF; fix checking;
>         reset sections of symbols dragged out of the comdats.
>         (function_and_variable_visibility): Reset sections of localized symbols.
> Index: symtab.c
> ===================================================================
> --- symtab.c    (revision 211489)
> +++ symtab.c    (working copy)
> @@ -1176,6 +1176,21 @@ symtab_node::set_section (const char *se
>    symtab_for_node_and_aliases (this, set_section_1, const_cast<char *>(section), true);
>  }
>
> +/* Reset section of NODE.  That is when NODE is being brought local
> +   we may want to clear section produced for comdat group and depending
> +   on function-sections produce now, local, unique section for it.  */
> +
> +void
> +symtab_node::reset_section ()
> +{
> +  if (!this->implicit_section)
> +    return;
> +  this->set_section (NULL);
> +  resolve_unique_section (this->decl, 0,
> +                         is_a <cgraph_node *> (this)
> +                         ? flag_function_sections : flag_data_sections);
> +}
> +
>  /* Worker for symtab_resolve_alias.  */
>
>  static bool
> Index: cgraph.c
> ===================================================================
> --- cgraph.c    (revision 211488)
> +++ cgraph.c    (working copy)
> @@ -2169,6 +2169,7 @@ cgraph_node_cannot_be_local_p_1 (struct
>                 && !node->forced_by_abi
>                 && !symtab_used_from_object_file_p (node)
>                 && !node->same_comdat_group)
> +              || DECL_EXTERNAL (node->decl)
>                || !node->externally_visible));
>  }
>
> @@ -2259,14 +2260,14 @@ cgraph_make_node_local_1 (struct cgraph_
>      {
>        symtab_make_decl_local (node->decl);
>
> -      node->set_section (NULL);
>        node->set_comdat_group (NULL);
>        node->externally_visible = false;
>        node->forced_by_abi = false;
>        node->local.local = true;
> -      node->set_section (NULL);
> +      node->reset_section ();
>        node->unique_name = (node->resolution == LDPR_PREVAILING_DEF_IRONLY
> -                                 || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP);
> +                          || node->unique_name
> +                          || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP);
>        node->resolution = LDPR_PREVAILING_DEF_IRONLY;
>        gcc_assert (cgraph_function_body_availability (node) == AVAIL_LOCAL);
>      }
> Index: cgraph.h
> ===================================================================
> --- cgraph.h    (revision 211489)
> +++ cgraph.h    (working copy)
> @@ -208,6 +208,7 @@ public:
>    /* Set section for symbol and its aliases.  */
>    void set_section (const char *section);
>    void set_section_for_node (const char *section);
> +  void reset_section ();
>  };
>
>  enum availability
> Index: ipa-inline-analysis.c
> ===================================================================
> --- ipa-inline-analysis.c       (revision 211488)
> +++ ipa-inline-analysis.c       (working copy)
> @@ -3877,7 +3877,7 @@ do_estimate_growth (struct cgraph_node *
>        /* COMDAT functions are very often not shared across multiple units
>           since they come from various template instantiations.
>           Take this into account.  */
> -      else if (DECL_COMDAT (node->decl)
> +      else if (node->externally_visible && node->get_comdat_group ()
>                && cgraph_can_remove_if_no_direct_calls_p (node))
>         d.growth -= (info->size
>                      * (100 - PARAM_VALUE (PARAM_COMDAT_SHARING_PROBABILITY))
> @@ -3928,7 +3928,7 @@ growth_likely_positive (struct cgraph_no
>        && (ret = node_growth_cache[node->uid]))
>      return ret > 0;
>    if (!cgraph_will_be_removed_from_program_if_no_direct_calls (node)
> -      && (!DECL_COMDAT (node->decl)
> +      && (!node->externally_visible || !node->get_comdat_group ()
>           || !cgraph_can_remove_if_no_direct_calls_p (node)))
>      return true;
>    max_callers = inline_summary (node)->size * 4 / edge_growth + 2;
> Index: cgraphclones.c
> ===================================================================
> --- cgraphclones.c      (revision 211488)
> +++ cgraphclones.c      (working copy)
> @@ -293,6 +293,7 @@ set_new_clone_decl_and_node_flags (cgrap
>    new_node->externally_visible = 0;
>    new_node->local.local = 1;
>    new_node->lowered = true;
> +  new_node->reset_section ();
>  }
>
>  /* Duplicate thunk THUNK if necessary but make it to refer to NODE.
> Index: ipa-visibility.c
> ===================================================================
> --- ipa-visibility.c    (revision 211488)
> +++ ipa-visibility.c    (working copy)
> @@ -236,7 +236,7 @@ cgraph_externally_visible_p (struct cgra
>      return true;
>    if (node->resolution == LDPR_PREVAILING_DEF_IRONLY)
>      return false;
> -  /* When doing LTO or whole program, we can bring COMDAT functoins static.
> +  /* When doing LTO or whole program, we can bring COMDAT functions static.
>       This improves code quality and we know we will duplicate them at most twice
>       (in the case that we are not using plugin and link with object file
>        implementing same COMDAT)  */
> @@ -295,8 +295,6 @@ varpool_externally_visible_p (varpool_no
>       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 (symtab_used_from_object_file_p (vnode))
> -    return true;
>    if (vnode->resolution == LDPR_PREVAILING_DEF_IRONLY)
>      return false;
>
> @@ -386,7 +384,8 @@ update_visibility_by_resolution_info (sy
>
>    if (!node->externally_visible
>        || (!DECL_WEAK (node->decl) && !DECL_ONE_ONLY (node->decl))
> -      || node->resolution == LDPR_UNKNOWN)
> +      || node->resolution == LDPR_UNKNOWN
> +      || node->resolution == LDPR_UNDEF)
>      return;
>
>    define = (node->resolution == LDPR_PREVAILING_DEF_IRONLY
> @@ -397,7 +396,7 @@ update_visibility_by_resolution_info (sy
>    if (node->same_comdat_group)
>      for (symtab_node *next = node->same_comdat_group;
>          next != node; next = next->same_comdat_group)
> -      gcc_assert (!node->externally_visible
> +      gcc_assert (!next->externally_visible
>                   || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
>                                 || next->resolution == LDPR_PREVAILING_DEF
>                                 || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP));
> @@ -411,11 +410,15 @@ update_visibility_by_resolution_info (sy
>         if (next->externally_visible
>             && !define)
>           DECL_EXTERNAL (next->decl) = true;
> +       if (!next->alias)
> +         next->reset_section ();
>        }
>    node->set_comdat_group (NULL);
>    DECL_WEAK (node->decl) = false;
>    if (!define)
>      DECL_EXTERNAL (node->decl) = true;
> +  if (!node->alias)
> +    node->reset_section ();
>    symtab_dissolve_same_comdat_group_list (node);
>  }
>
> @@ -476,7 +479,7 @@ function_and_variable_visibility (bool w
>           symtab_dissolve_same_comdat_group_list (node);
>         }
>        gcc_assert ((!DECL_WEAK (node->decl)
> -                 && !DECL_COMDAT (node->decl))
> +                  && !DECL_COMDAT (node->decl))
>                   || TREE_PUBLIC (node->decl)
>                   || node->weakref
>                   || DECL_EXTERNAL (node->decl));
> @@ -494,6 +497,7 @@ function_and_variable_visibility (bool w
>           && node->definition && !node->weakref
>           && !DECL_EXTERNAL (node->decl))
>         {
> +         bool reset = TREE_PUBLIC (node->decl);
>           gcc_assert (whole_program || in_lto_p
>                       || !TREE_PUBLIC (node->decl));
>           node->unique_name = ((node->resolution == LDPR_PREVAILING_DEF_IRONLY
> @@ -512,9 +516,9 @@ function_and_variable_visibility (bool w
>                      next = next->same_comdat_group)
>                 {
>                   next->set_comdat_group (NULL);
> -                 if (!next->alias)
> -                   next->set_section (NULL);
>                   symtab_make_decl_local (next->decl);
> +                 if (!node->alias)
> +                   node->reset_section ();
>                   next->unique_name = ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
>                                         || next->unique_name
>                                         || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> @@ -528,9 +532,9 @@ function_and_variable_visibility (bool w
>             }
>           if (TREE_PUBLIC (node->decl))
>             node->set_comdat_group (NULL);
> -         if (DECL_COMDAT (node->decl) && !node->alias)
> -           node->set_section (NULL);
>           symtab_make_decl_local (node->decl);
> +         if (reset && !node->alias)
> +           node->reset_section ();
>         }
>
>        if (node->thunk.thunk_p
> @@ -632,6 +636,7 @@ function_and_variable_visibility (bool w
>        if (!vnode->externally_visible
>           && !vnode->weakref)
>         {
> +         bool reset = TREE_PUBLIC (vnode->decl);
>           gcc_assert (in_lto_p || whole_program || !TREE_PUBLIC (vnode->decl));
>           vnode->unique_name = ((vnode->resolution == LDPR_PREVAILING_DEF_IRONLY
>                                        || vnode->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> @@ -647,9 +652,9 @@ function_and_variable_visibility (bool w
>                      next = next->same_comdat_group)
>                 {
>                   next->set_comdat_group (NULL);
> -                 if (!next->alias)
> -                   next->set_section (NULL);
>                   symtab_make_decl_local (next->decl);
> +                 if (!next->alias)
> +                   next->reset_section ();
>                   next->unique_name = ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
>                                         || next->unique_name
>                                         || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> @@ -659,9 +664,9 @@ function_and_variable_visibility (bool w
>             }
>           if (TREE_PUBLIC (vnode->decl))
>             vnode->set_comdat_group (NULL);
> -         if (DECL_COMDAT (vnode->decl) && !vnode->alias)
> -           vnode->set_section (NULL);
>           symtab_make_decl_local (vnode->decl);
> +         if (reset && !vnode->alias)
> +           vnode->reset_section ();
>           vnode->resolution = LDPR_PREVAILING_DEF_IRONLY;
>         }
>        update_visibility_by_resolution_info (vnode);

Re: Another AIX Bootstrap failure

[David Edelsohn]

Honza,

Thanks for reverting the patch. I will check if this resolves the
current bootstrap problem.

I was suggesting that you create a branch for all of the visibility
changes to make it easier to track the various original patches and
later correction patches from you.

I don't know if the gen* programs hang because of the visibility
changes or because of the change in sections. The change in sections
could conflict with the GCC code to handle AIX XCOFF CSECTs for
functions.

AIX recently added support for ELF-like visibility. AIX previously
supported the equivalent of visibility through "export" files. The
recent problems could be due to issues with assembly file ordering,
but they also could be related to the visibility changes affecting the
way that GCC emits code to branch to global functions.

Also, your recent ChangeLog entry included a Subversion conflict marker:


2014-06-15  Jan Hubicka  <hubicka@ucw.cz>

        * c-family/c-common.c (handle_tls_model_attribute): Use set_decl_tls_mod
el.
        * c-family/c-common.c (handle_tls_model_attribute): Use
        set_decl_tls_model.
>>>>>>> .r211699
        * cgraph.h (struct varpool_node): Add tls_model.
        * tree.c (decl_tls_model, set_decl_tls_model): New functions.
        * tree.h (DECL_TLS_MODEL): Update.


Thanks, David



On Mon, Jun 16, 2014 at 12:35 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Honza,
>>
>> The cgraph patch in r211600 broke AIX bootstrap again.  I cannot find
>> the corresponding patch in the GCC Patches mailing list, so I do not
>> see where this was discussed or approved.
>
> Sorry, I remember writting mail about this patch but also can't find it.  The
> patch introduces reset_section that is used when function or variable is
> brought local by the visibility code. In this case we previously incorrectly
> cleared user sections and we forgot to introduce implicit sections with
> -fdata-section and -ffunction-section.
>
> I will revert this change and lets debug it.
>
>>
>> With the patch in r211600, the "gen*" programs in stage2 go into an
>> endless loop.
>>
>> Please revert these comdat patches.  These were not tested appropriately.
>>
>> Please create a branch and we can debug the problems on AIX.
>
> What patches you refer to? I already disabled the localization within
> initializers on AIX (well all targets w/o MAKE_ONE_ONLY support) and
> I am testing patch reverting this change (I want to keep reset section
> just remove the call to resolve unique section) and will commit once
> it converges.
>
> I will send you patch to enable those two changes and lets look into why
> they break. What is common to both patches is that they deal with static
> symbol in named sections, perhaps that is not correctly supported by AIX
> toolchain...
>
> My apologizes for the breakage. I am attaching the patch for reference.
> Honza
>
>
>         * symtab.c (symtab_node::reset_section): New method.
>         * cgraph.c (cgraph_node_cannot_be_local_p_1): Accept non-local
>         for localization.
>         * cgraph.h (reset_section): Declare.
>         * ipa-inline-analysis.c (do_estimate_growth): Check for comdat groups;
>         do not consider comdat locals.
>         * cgraphclones.c (set_new_clone_decl_and_node_flags): Get section
>         for new symbol.
>         * ipa-visiblity.c (cgraph_externally_visible_p): Cleanup.
>         (update_visibility_by_resolution_info): Consider UNDEF; fix checking;
>         reset sections of symbols dragged out of the comdats.
>         (function_and_variable_visibility): Reset sections of localized symbols.
> Index: symtab.c
> ===================================================================
> --- symtab.c    (revision 211489)
> +++ symtab.c    (working copy)
> @@ -1176,6 +1176,21 @@ symtab_node::set_section (const char *se
>    symtab_for_node_and_aliases (this, set_section_1, const_cast<char *>(section), true);
>  }
>
> +/* Reset section of NODE.  That is when NODE is being brought local
> +   we may want to clear section produced for comdat group and depending
> +   on function-sections produce now, local, unique section for it.  */
> +
> +void
> +symtab_node::reset_section ()
> +{
> +  if (!this->implicit_section)
> +    return;
> +  this->set_section (NULL);
> +  resolve_unique_section (this->decl, 0,
> +                         is_a <cgraph_node *> (this)
> +                         ? flag_function_sections : flag_data_sections);
> +}
> +
>  /* Worker for symtab_resolve_alias.  */
>
>  static bool
> Index: cgraph.c
> ===================================================================
> --- cgraph.c    (revision 211488)
> +++ cgraph.c    (working copy)
> @@ -2169,6 +2169,7 @@ cgraph_node_cannot_be_local_p_1 (struct
>                 && !node->forced_by_abi
>                 && !symtab_used_from_object_file_p (node)
>                 && !node->same_comdat_group)
> +              || DECL_EXTERNAL (node->decl)
>                || !node->externally_visible));
>  }
>
> @@ -2259,14 +2260,14 @@ cgraph_make_node_local_1 (struct cgraph_
>      {
>        symtab_make_decl_local (node->decl);
>
> -      node->set_section (NULL);
>        node->set_comdat_group (NULL);
>        node->externally_visible = false;
>        node->forced_by_abi = false;
>        node->local.local = true;
> -      node->set_section (NULL);
> +      node->reset_section ();
>        node->unique_name = (node->resolution == LDPR_PREVAILING_DEF_IRONLY
> -                                 || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP);
> +                          || node->unique_name
> +                          || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP);
>        node->resolution = LDPR_PREVAILING_DEF_IRONLY;
>        gcc_assert (cgraph_function_body_availability (node) == AVAIL_LOCAL);
>      }
> Index: cgraph.h
> ===================================================================
> --- cgraph.h    (revision 211489)
> +++ cgraph.h    (working copy)
> @@ -208,6 +208,7 @@ public:
>    /* Set section for symbol and its aliases.  */
>    void set_section (const char *section);
>    void set_section_for_node (const char *section);
> +  void reset_section ();
>  };
>
>  enum availability
> Index: ipa-inline-analysis.c
> ===================================================================
> --- ipa-inline-analysis.c       (revision 211488)
> +++ ipa-inline-analysis.c       (working copy)
> @@ -3877,7 +3877,7 @@ do_estimate_growth (struct cgraph_node *
>        /* COMDAT functions are very often not shared across multiple units
>           since they come from various template instantiations.
>           Take this into account.  */
> -      else if (DECL_COMDAT (node->decl)
> +      else if (node->externally_visible && node->get_comdat_group ()
>                && cgraph_can_remove_if_no_direct_calls_p (node))
>         d.growth -= (info->size
>                      * (100 - PARAM_VALUE (PARAM_COMDAT_SHARING_PROBABILITY))
> @@ -3928,7 +3928,7 @@ growth_likely_positive (struct cgraph_no
>        && (ret = node_growth_cache[node->uid]))
>      return ret > 0;
>    if (!cgraph_will_be_removed_from_program_if_no_direct_calls (node)
> -      && (!DECL_COMDAT (node->decl)
> +      && (!node->externally_visible || !node->get_comdat_group ()
>           || !cgraph_can_remove_if_no_direct_calls_p (node)))
>      return true;
>    max_callers = inline_summary (node)->size * 4 / edge_growth + 2;
> Index: cgraphclones.c
> ===================================================================
> --- cgraphclones.c      (revision 211488)
> +++ cgraphclones.c      (working copy)
> @@ -293,6 +293,7 @@ set_new_clone_decl_and_node_flags (cgrap
>    new_node->externally_visible = 0;
>    new_node->local.local = 1;
>    new_node->lowered = true;
> +  new_node->reset_section ();
>  }
>
>  /* Duplicate thunk THUNK if necessary but make it to refer to NODE.
> Index: ipa-visibility.c
> ===================================================================
> --- ipa-visibility.c    (revision 211488)
> +++ ipa-visibility.c    (working copy)
> @@ -236,7 +236,7 @@ cgraph_externally_visible_p (struct cgra
>      return true;
>    if (node->resolution == LDPR_PREVAILING_DEF_IRONLY)
>      return false;
> -  /* When doing LTO or whole program, we can bring COMDAT functoins static.
> +  /* When doing LTO or whole program, we can bring COMDAT functions static.
>       This improves code quality and we know we will duplicate them at most twice
>       (in the case that we are not using plugin and link with object file
>        implementing same COMDAT)  */
> @@ -295,8 +295,6 @@ varpool_externally_visible_p (varpool_no
>       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 (symtab_used_from_object_file_p (vnode))
> -    return true;
>    if (vnode->resolution == LDPR_PREVAILING_DEF_IRONLY)
>      return false;
>
> @@ -386,7 +384,8 @@ update_visibility_by_resolution_info (sy
>
>    if (!node->externally_visible
>        || (!DECL_WEAK (node->decl) && !DECL_ONE_ONLY (node->decl))
> -      || node->resolution == LDPR_UNKNOWN)
> +      || node->resolution == LDPR_UNKNOWN
> +      || node->resolution == LDPR_UNDEF)
>      return;
>
>    define = (node->resolution == LDPR_PREVAILING_DEF_IRONLY
> @@ -397,7 +396,7 @@ update_visibility_by_resolution_info (sy
>    if (node->same_comdat_group)
>      for (symtab_node *next = node->same_comdat_group;
>          next != node; next = next->same_comdat_group)
> -      gcc_assert (!node->externally_visible
> +      gcc_assert (!next->externally_visible
>                   || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
>                                 || next->resolution == LDPR_PREVAILING_DEF
>                                 || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP));
> @@ -411,11 +410,15 @@ update_visibility_by_resolution_info (sy
>         if (next->externally_visible
>             && !define)
>           DECL_EXTERNAL (next->decl) = true;
> +       if (!next->alias)
> +         next->reset_section ();
>        }
>    node->set_comdat_group (NULL);
>    DECL_WEAK (node->decl) = false;
>    if (!define)
>      DECL_EXTERNAL (node->decl) = true;
> +  if (!node->alias)
> +    node->reset_section ();
>    symtab_dissolve_same_comdat_group_list (node);
>  }
>
> @@ -476,7 +479,7 @@ function_and_variable_visibility (bool w
>           symtab_dissolve_same_comdat_group_list (node);
>         }
>        gcc_assert ((!DECL_WEAK (node->decl)
> -                 && !DECL_COMDAT (node->decl))
> +                  && !DECL_COMDAT (node->decl))
>                   || TREE_PUBLIC (node->decl)
>                   || node->weakref
>                   || DECL_EXTERNAL (node->decl));
> @@ -494,6 +497,7 @@ function_and_variable_visibility (bool w
>           && node->definition && !node->weakref
>           && !DECL_EXTERNAL (node->decl))
>         {
> +         bool reset = TREE_PUBLIC (node->decl);
>           gcc_assert (whole_program || in_lto_p
>                       || !TREE_PUBLIC (node->decl));
>           node->unique_name = ((node->resolution == LDPR_PREVAILING_DEF_IRONLY
> @@ -512,9 +516,9 @@ function_and_variable_visibility (bool w
>                      next = next->same_comdat_group)
>                 {
>                   next->set_comdat_group (NULL);
> -                 if (!next->alias)
> -                   next->set_section (NULL);
>                   symtab_make_decl_local (next->decl);
> +                 if (!node->alias)
> +                   node->reset_section ();
>                   next->unique_name = ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
>                                         || next->unique_name
>                                         || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> @@ -528,9 +532,9 @@ function_and_variable_visibility (bool w
>             }
>           if (TREE_PUBLIC (node->decl))
>             node->set_comdat_group (NULL);
> -         if (DECL_COMDAT (node->decl) && !node->alias)
> -           node->set_section (NULL);
>           symtab_make_decl_local (node->decl);
> +         if (reset && !node->alias)
> +           node->reset_section ();
>         }
>
>        if (node->thunk.thunk_p
> @@ -632,6 +636,7 @@ function_and_variable_visibility (bool w
>        if (!vnode->externally_visible
>           && !vnode->weakref)
>         {
> +         bool reset = TREE_PUBLIC (vnode->decl);
>           gcc_assert (in_lto_p || whole_program || !TREE_PUBLIC (vnode->decl));
>           vnode->unique_name = ((vnode->resolution == LDPR_PREVAILING_DEF_IRONLY
>                                        || vnode->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> @@ -647,9 +652,9 @@ function_and_variable_visibility (bool w
>                      next = next->same_comdat_group)
>                 {
>                   next->set_comdat_group (NULL);
> -                 if (!next->alias)
> -                   next->set_section (NULL);
>                   symtab_make_decl_local (next->decl);
> +                 if (!next->alias)
> +                   next->reset_section ();
>                   next->unique_name = ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
>                                         || next->unique_name
>                                         || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
> @@ -659,9 +664,9 @@ function_and_variable_visibility (bool w
>             }
>           if (TREE_PUBLIC (vnode->decl))
>             vnode->set_comdat_group (NULL);
> -         if (DECL_COMDAT (vnode->decl) && !vnode->alias)
> -           vnode->set_section (NULL);
>           symtab_make_decl_local (vnode->decl);
> +         if (reset && !vnode->alias)
> +           vnode->reset_section ();
>           vnode->resolution = LDPR_PREVAILING_DEF_IRONLY;
>         }
>        update_visibility_by_resolution_info (vnode);

Re: Another AIX Bootstrap failure

[Jan Hubicka]

> Honza,
> 
> The cgraph patch in r211600 broke AIX bootstrap again.  I cannot find
> the corresponding patch in the GCC Patches mailing list, so I do not
> see where this was discussed or approved.

Sorry, I remember writting mail about this patch but also can't find it.  The
patch introduces reset_section that is used when function or variable is
brought local by the visibility code. In this case we previously incorrectly
cleared user sections and we forgot to introduce implicit sections with
-fdata-section and -ffunction-section.

I will revert this change and lets debug it.

> 
> With the patch in r211600, the "gen*" programs in stage2 go into an
> endless loop.
> 
> Please revert these comdat patches.  These were not tested appropriately.
> 
> Please create a branch and we can debug the problems on AIX.

What patches you refer to? I already disabled the localization within
initializers on AIX (well all targets w/o MAKE_ONE_ONLY support) and
I am testing patch reverting this change (I want to keep reset section
just remove the call to resolve unique section) and will commit once
it converges.

I will send you patch to enable those two changes and lets look into why 
they break. What is common to both patches is that they deal with static
symbol in named sections, perhaps that is not correctly supported by AIX
toolchain...

My apologizes for the breakage. I am attaching the patch for reference.
Honza


	* symtab.c (symtab_node::reset_section): New method.
	* cgraph.c (cgraph_node_cannot_be_local_p_1): Accept non-local
	for localization.
	* cgraph.h (reset_section): Declare.
	* ipa-inline-analysis.c (do_estimate_growth): Check for comdat groups;
	do not consider comdat locals.
	* cgraphclones.c (set_new_clone_decl_and_node_flags): Get section
	for new symbol.
	* ipa-visiblity.c (cgraph_externally_visible_p): Cleanup.
	(update_visibility_by_resolution_info): Consider UNDEF; fix checking;
	reset sections of symbols dragged out of the comdats.
	(function_and_variable_visibility): Reset sections of localized symbols.
Index: symtab.c
===================================================================
--- symtab.c	(revision 211489)
+++ symtab.c	(working copy)
@@ -1176,6 +1176,21 @@ symtab_node::set_section (const char *se
   symtab_for_node_and_aliases (this, set_section_1, const_cast<char *>(section), true);
 }
 
+/* Reset section of NODE.  That is when NODE is being brought local
+   we may want to clear section produced for comdat group and depending
+   on function-sections produce now, local, unique section for it.  */
+
+void
+symtab_node::reset_section ()
+{
+  if (!this->implicit_section)
+    return;
+  this->set_section (NULL);
+  resolve_unique_section (this->decl, 0,
+			  is_a <cgraph_node *> (this)
+			  ? flag_function_sections : flag_data_sections);
+}
+
 /* Worker for symtab_resolve_alias.  */
 
 static bool
Index: cgraph.c
===================================================================
--- cgraph.c	(revision 211488)
+++ cgraph.c	(working copy)
@@ -2169,6 +2169,7 @@ cgraph_node_cannot_be_local_p_1 (struct
 		&& !node->forced_by_abi
 	        && !symtab_used_from_object_file_p (node)
 		&& !node->same_comdat_group)
+	       || DECL_EXTERNAL (node->decl)
 	       || !node->externally_visible));
 }
 
@@ -2259,14 +2260,14 @@ cgraph_make_node_local_1 (struct cgraph_
     {
       symtab_make_decl_local (node->decl);
 
-      node->set_section (NULL);
       node->set_comdat_group (NULL);
       node->externally_visible = false;
       node->forced_by_abi = false;
       node->local.local = true;
-      node->set_section (NULL);
+      node->reset_section ();
       node->unique_name = (node->resolution == LDPR_PREVAILING_DEF_IRONLY
-				  || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP);
+			   || node->unique_name
+			   || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP);
       node->resolution = LDPR_PREVAILING_DEF_IRONLY;
       gcc_assert (cgraph_function_body_availability (node) == AVAIL_LOCAL);
     }
Index: cgraph.h
===================================================================
--- cgraph.h	(revision 211489)
+++ cgraph.h	(working copy)
@@ -208,6 +208,7 @@ public:
   /* Set section for symbol and its aliases.  */
   void set_section (const char *section);
   void set_section_for_node (const char *section);
+  void reset_section ();
 };
 
 enum availability
Index: ipa-inline-analysis.c
===================================================================
--- ipa-inline-analysis.c	(revision 211488)
+++ ipa-inline-analysis.c	(working copy)
@@ -3877,7 +3877,7 @@ do_estimate_growth (struct cgraph_node *
       /* COMDAT functions are very often not shared across multiple units
          since they come from various template instantiations.
          Take this into account.  */
-      else if (DECL_COMDAT (node->decl)
+      else if (node->externally_visible && node->get_comdat_group ()
 	       && cgraph_can_remove_if_no_direct_calls_p (node))
 	d.growth -= (info->size
 		     * (100 - PARAM_VALUE (PARAM_COMDAT_SHARING_PROBABILITY))
@@ -3928,7 +3928,7 @@ growth_likely_positive (struct cgraph_no
       && (ret = node_growth_cache[node->uid]))
     return ret > 0;
   if (!cgraph_will_be_removed_from_program_if_no_direct_calls (node)
-      && (!DECL_COMDAT (node->decl)
+      && (!node->externally_visible || !node->get_comdat_group ()
 	  || !cgraph_can_remove_if_no_direct_calls_p (node)))
     return true;
   max_callers = inline_summary (node)->size * 4 / edge_growth + 2;
Index: cgraphclones.c
===================================================================
--- cgraphclones.c	(revision 211488)
+++ cgraphclones.c	(working copy)
@@ -293,6 +293,7 @@ set_new_clone_decl_and_node_flags (cgrap
   new_node->externally_visible = 0;
   new_node->local.local = 1;
   new_node->lowered = true;
+  new_node->reset_section ();
 }
 
 /* Duplicate thunk THUNK if necessary but make it to refer to NODE.
Index: ipa-visibility.c
===================================================================
--- ipa-visibility.c	(revision 211488)
+++ ipa-visibility.c	(working copy)
@@ -236,7 +236,7 @@ cgraph_externally_visible_p (struct cgra
     return true;
   if (node->resolution == LDPR_PREVAILING_DEF_IRONLY)
     return false;
-  /* When doing LTO or whole program, we can bring COMDAT functoins static.
+  /* When doing LTO or whole program, we can bring COMDAT functions static.
      This improves code quality and we know we will duplicate them at most twice
      (in the case that we are not using plugin and link with object file
       implementing same COMDAT)  */
@@ -295,8 +295,6 @@ varpool_externally_visible_p (varpool_no
      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 (symtab_used_from_object_file_p (vnode))
-    return true;
   if (vnode->resolution == LDPR_PREVAILING_DEF_IRONLY)
     return false;
 
@@ -386,7 +384,8 @@ update_visibility_by_resolution_info (sy
 
   if (!node->externally_visible
       || (!DECL_WEAK (node->decl) && !DECL_ONE_ONLY (node->decl))
-      || node->resolution == LDPR_UNKNOWN)
+      || node->resolution == LDPR_UNKNOWN
+      || node->resolution == LDPR_UNDEF)
     return;
 
   define = (node->resolution == LDPR_PREVAILING_DEF_IRONLY
@@ -397,7 +396,7 @@ update_visibility_by_resolution_info (sy
   if (node->same_comdat_group)
     for (symtab_node *next = node->same_comdat_group;
 	 next != node; next = next->same_comdat_group)
-      gcc_assert (!node->externally_visible
+      gcc_assert (!next->externally_visible
 		  || define == (next->resolution == LDPR_PREVAILING_DEF_IRONLY
 			        || next->resolution == LDPR_PREVAILING_DEF
 			        || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP));
@@ -411,11 +410,15 @@ update_visibility_by_resolution_info (sy
 	if (next->externally_visible
 	    && !define)
 	  DECL_EXTERNAL (next->decl) = true;
+	if (!next->alias)
+	  next->reset_section ();
       }
   node->set_comdat_group (NULL);
   DECL_WEAK (node->decl) = false;
   if (!define)
     DECL_EXTERNAL (node->decl) = true;
+  if (!node->alias)
+    node->reset_section ();
   symtab_dissolve_same_comdat_group_list (node);
 }
 
@@ -476,7 +479,7 @@ function_and_variable_visibility (bool w
 	  symtab_dissolve_same_comdat_group_list (node);
 	}
       gcc_assert ((!DECL_WEAK (node->decl)
-		  && !DECL_COMDAT (node->decl))
+		   && !DECL_COMDAT (node->decl))
       	          || TREE_PUBLIC (node->decl)
 		  || node->weakref
 		  || DECL_EXTERNAL (node->decl));
@@ -494,6 +497,7 @@ function_and_variable_visibility (bool w
 	  && node->definition && !node->weakref
 	  && !DECL_EXTERNAL (node->decl))
 	{
+	  bool reset = TREE_PUBLIC (node->decl);
 	  gcc_assert (whole_program || in_lto_p
 		      || !TREE_PUBLIC (node->decl));
 	  node->unique_name = ((node->resolution == LDPR_PREVAILING_DEF_IRONLY
@@ -512,9 +516,9 @@ function_and_variable_visibility (bool w
 		     next = next->same_comdat_group)
 		{
 		  next->set_comdat_group (NULL);
-		  if (!next->alias)
-		    next->set_section (NULL);
 		  symtab_make_decl_local (next->decl);
+		  if (!node->alias)
+		    node->reset_section ();
 		  next->unique_name = ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
 					|| next->unique_name
 					|| next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
@@ -528,9 +532,9 @@ function_and_variable_visibility (bool w
 	    }
 	  if (TREE_PUBLIC (node->decl))
 	    node->set_comdat_group (NULL);
-	  if (DECL_COMDAT (node->decl) && !node->alias)
-	    node->set_section (NULL);
 	  symtab_make_decl_local (node->decl);
+	  if (reset && !node->alias)
+	    node->reset_section ();
 	}
 
       if (node->thunk.thunk_p
@@ -632,6 +636,7 @@ function_and_variable_visibility (bool w
       if (!vnode->externally_visible
 	  && !vnode->weakref)
 	{
+	  bool reset = TREE_PUBLIC (vnode->decl);
 	  gcc_assert (in_lto_p || whole_program || !TREE_PUBLIC (vnode->decl));
 	  vnode->unique_name = ((vnode->resolution == LDPR_PREVAILING_DEF_IRONLY
 				       || vnode->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
@@ -647,9 +652,9 @@ function_and_variable_visibility (bool w
 		     next = next->same_comdat_group)
 		{
 		  next->set_comdat_group (NULL);
-		  if (!next->alias)
-		    next->set_section (NULL);
 		  symtab_make_decl_local (next->decl);
+		  if (!next->alias)
+		    next->reset_section ();
 		  next->unique_name = ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
 					|| next->unique_name
 					|| next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
@@ -659,9 +664,9 @@ function_and_variable_visibility (bool w
 	    }
 	  if (TREE_PUBLIC (vnode->decl))
 	    vnode->set_comdat_group (NULL);
-	  if (DECL_COMDAT (vnode->decl) && !vnode->alias)
-	    vnode->set_section (NULL);
 	  symtab_make_decl_local (vnode->decl);
+	  if (reset && !vnode->alias)
+	    vnode->reset_section ();
 	  vnode->resolution = LDPR_PREVAILING_DEF_IRONLY;
 	}
       update_visibility_by_resolution_info (vnode);

Another AIX Bootstrap failure

[David Edelsohn]

Honza,

The cgraph patch in r211600 broke AIX bootstrap again.  I cannot find
the corresponding patch in the GCC Patches mailing list, so I do not
see where this was discussed or approved.

With the patch in r211600, the "gen*" programs in stage2 go into an
endless loop.

Please revert these comdat patches.  These were not tested appropriately.

Please create a branch and we can debug the problems on AIX.

Thanks, David