Another AIX Bootstrap failure

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;
^^^^^^^^^^^^^^^^^ typo
> +}

return 0;

You probably should run the testcases before committing them.

Thanks, David

> > +  /* 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

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));
>

> 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