public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jan Hubicka <hubicka@ucw.cz>
To: David Edelsohn <dje.gcc@gmail.com>
Cc: Jan Hubicka <hubicka@ucw.cz>, GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: Another AIX Bootstrap failure
Date: Sat, 21 Jun 2014 02:43:00 -0000	[thread overview]
Message-ID: <20140621024301.GE4551@kam.mff.cuni.cz> (raw)
In-Reply-To: <CAGWvnykZKOiQhM-N6TTCPa_E0d0TCK_PYs9bL_YZTJ7E193eUQ@mail.gmail.com>

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

  parent reply	other threads:[~2014-06-21  2:43 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-16  3:31 David Edelsohn
2014-06-16  4:36 ` Jan Hubicka
2014-06-16 11:53   ` David Edelsohn
2014-06-16 15:08     ` Jan Hubicka
2014-06-16 16:16       ` Ramana Radhakrishnan
2014-06-16 17:13       ` David Edelsohn
2014-06-16 21:44         ` Jan Hubicka
2014-06-16 22:42           ` David Edelsohn
2014-06-16 12:01   ` Ramana Radhakrishnan
2014-06-16 18:55   ` David Edelsohn
2014-06-16 22:06     ` Jan Hubicka
2014-06-17  1:51       ` David Edelsohn
2014-06-17  2:55       ` David Edelsohn
2014-06-17  3:44         ` Jan Hubicka
2014-06-17 14:07           ` David Edelsohn
2014-06-17 16:51             ` Jan Hubicka
2014-06-17 16:57               ` David Edelsohn
2014-06-21  2:43 ` Jan Hubicka [this message]
2014-06-21 22:47   ` David Edelsohn
2014-06-21 23:04     ` Jan Hubicka
2014-06-22 14:47   ` David Edelsohn
2014-06-22 19:12     ` Jan Hubicka
2014-06-23 16:49 Dominique Dhumieres
2014-06-23 17:36 ` Jan Hubicka
2014-06-24 19:36   ` Iain Sandoe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140621024301.GE4551@kam.mff.cuni.cz \
    --to=hubicka@ucw.cz \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).