Re: [PATCH] PR rtl-optimization/32219: optimizer causees wrong code in pic/hidden/weak symbol checking

[Richard Henderson <rth at redhat dot com>]

On 02/10/2015 01:19 PM, Richard Henderson wrote:
> As an existing issue, I'm not sure why "specified" visibility is any different
> from unspecified visibility.  As far as I'm aware, the "specified" bit simply
> means that the decl doesn't inherit inherit visibility from the class, or from
> the command-line.  But once we're this far, the visibility actually applied to
> the symbol should be all that matters.

The test is there to differentiate explicit visibility from that implied from
the command-line.  Without it, we assume hidden visibility for external symbols
too early, making the command-line option useless.  This is visible even in
building libgcc.

I believe this set of patches does what we want, and cleans things up a bit in
the process.


r~

From bf5d4f20368d1173a8d586f6bdd258b00b807e33 Mon Sep 17 00:00:00 2001
From: Richard Henderson <rth@redhat.com>
Date: Wed, 11 Feb 2015 17:37:48 -0800
Subject: [PATCH 1/5] Replace local_p with direct returns

---
 gcc/varasm.c | 66 +++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/gcc/varasm.c b/gcc/varasm.c
index 3f62fca..83d9de3 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -6814,7 +6814,6 @@ default_binds_local_p (const_tree exp)
 bool
 default_binds_local_p_1 (const_tree exp, int shlib)
 {
-  bool local_p;
   bool resolved_locally = false;
   bool resolved_to_local_def = false;
 
@@ -6845,54 +6844,57 @@ default_binds_local_p_1 (const_tree exp, int shlib)
 
   /* A non-decl is an entry in the constant pool.  */
   if (!DECL_P (exp))
-    local_p = true;
+    return true;
+
   /* Weakrefs may not bind locally, even though the weakref itself is always
      static and therefore local.  Similarly, the resolver for ifunc functions
      might resolve to a non-local function.
      FIXME: We can resolve the weakref case more curefuly by looking at the
      weakref alias.  */
-  else if (lookup_attribute ("weakref", DECL_ATTRIBUTES (exp))
+  if (lookup_attribute ("weakref", DECL_ATTRIBUTES (exp))
 	   || (TREE_CODE (exp) == FUNCTION_DECL
 	       && lookup_attribute ("ifunc", DECL_ATTRIBUTES (exp))))
-    local_p = false;
+    return false;
+
   /* Static variables are always local.  */
-  else if (! TREE_PUBLIC (exp))
-    local_p = true;
-  /* A variable is local if the user has said explicitly that it will
-     be.  */
-  else if ((DECL_VISIBILITY_SPECIFIED (exp)
-	    || resolved_to_local_def)
-	   && DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
-    local_p = true;
+  if (! TREE_PUBLIC (exp))
+    return true;
+
+  /* A variable is local if the user has said explicitly that it will be.  */
+  if ((DECL_VISIBILITY_SPECIFIED (exp) || resolved_to_local_def)
+      && DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
+    return true;
+
   /* Variables defined outside this object might not be local.  */
-  else if (DECL_EXTERNAL (exp) && !resolved_locally)
-    local_p = false;
-  /* If defined in this object and visibility is not default, must be
-     local.  */
-  else if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
-    local_p = true;
+  if (DECL_EXTERNAL (exp) && !resolved_locally)
+    return false;
+
+  /* If defined in this object and visibility is not default,
+     must be local.  */
+  if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
+    return true;
+
   /* Default visibility weak data can be overridden by a strong symbol
      in another module and so are not local.  */
-  else if (DECL_WEAK (exp)
-	   && !resolved_locally)
-    local_p = false;
+  if (DECL_WEAK (exp) && !resolved_locally)
+    return false;
+
   /* If PIC, then assume that any global name can be overridden by
      symbols resolved from other modules.  */
-  else if (shlib)
-    local_p = false;
+  if (shlib)
+    return false;
+
   /* Uninitialized COMMON variable may be unified with symbols
      resolved from other modules.  */
-  else if (DECL_COMMON (exp)
-	   && !resolved_locally
-	   && (DECL_INITIAL (exp) == NULL
-	       || (!in_lto_p && DECL_INITIAL (exp) == error_mark_node)))
-    local_p = false;
+  if (DECL_COMMON (exp)
+      && !resolved_locally
+      && (DECL_INITIAL (exp) == NULL
+	  || (!in_lto_p && DECL_INITIAL (exp) == error_mark_node)))
+    return false;
+
   /* Otherwise we're left with initialized (or non-common) global data
      which is of necessity defined locally.  */
-  else
-    local_p = true;
-
-  return local_p;
+  return true;
 }
 
 /* Return true when references to DECL must bind to current definition in
-- 
2.1.0

From 0b1ac37c688093d720a99b5ae446602a0c1120b3 Mon Sep 17 00:00:00 2001
From: Richard Henderson <rth@redhat.com>
Date: Wed, 11 Feb 2015 19:27:14 -0800
Subject: [PATCH 2/5] Use symtab_node to unify var and function handling

Delay that until after static symbols are eliminated.
---
 gcc/varasm.c | 59 +++++++++++++++++++----------------------------------------
 1 file changed, 19 insertions(+), 40 deletions(-)

diff --git a/gcc/varasm.c b/gcc/varasm.c
index 83d9de3..dcc70e3 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -6814,34 +6814,6 @@ default_binds_local_p (const_tree exp)
 bool
 default_binds_local_p_1 (const_tree exp, int shlib)
 {
-  bool resolved_locally = false;
-  bool resolved_to_local_def = false;
-
-  /* With resolution file in hands, take look into resolutions.
-     We can't just return true for resolved_locally symbols,
-     because dynamic linking might overwrite symbols
-     in shared libraries.  */
-  if (TREE_CODE (exp) == VAR_DECL && TREE_PUBLIC (exp)
-      && (TREE_STATIC (exp) || DECL_EXTERNAL (exp)))
-    {
-      varpool_node *vnode = varpool_node::get (exp);
-      if (vnode && (resolution_local_p (vnode->resolution) || vnode->in_other_partition))
-	resolved_locally = true;
-      if (vnode
-	  && resolution_to_local_definition_p (vnode->resolution))
-	resolved_to_local_def = true;
-    }
-  else if (TREE_CODE (exp) == FUNCTION_DECL && TREE_PUBLIC (exp))
-    {
-      struct cgraph_node *node = cgraph_node::get (exp);
-      if (node
-	  && (resolution_local_p (node->resolution) || node->in_other_partition))
-	resolved_locally = true;
-      if (node
-	  && resolution_to_local_definition_p (node->resolution))
-	resolved_to_local_def = true;
-    }
-
   /* A non-decl is an entry in the constant pool.  */
   if (!DECL_P (exp))
     return true;
@@ -6860,6 +6832,21 @@ default_binds_local_p_1 (const_tree exp, int shlib)
   if (! TREE_PUBLIC (exp))
     return true;
 
+  /* With resolution file in hand, take look into resolutions.
+     We can't just return true for resolved_locally symbols,
+     because dynamic linking might overwrite symbols
+     in shared libraries.  */
+  bool resolved_locally = false;
+  bool resolved_to_local_def = false;
+  if (symtab_node *node = symtab_node::get (exp))
+    {
+      if (node->in_other_partition
+	  || resolution_local_p (node->resolution))
+	resolved_locally = true;
+      if (resolution_to_local_definition_p (node->resolution))
+	resolved_to_local_def = true;
+    }
+
   /* A variable is local if the user has said explicitly that it will be.  */
   if ((DECL_VISIBILITY_SPECIFIED (exp) || resolved_to_local_def)
       && DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
@@ -6916,22 +6903,14 @@ decl_binds_to_current_def_p (const_tree decl)
     return false;
   if (!TREE_PUBLIC (decl))
     return true;
+
   /* When resolution is available, just use it.  */
-  if (TREE_CODE (decl) == VAR_DECL
-      && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)))
+  if (symtab_node *node = symtab_node::get (decl))
     {
-      varpool_node *vnode = varpool_node::get (decl);
-      if (vnode
-	  && vnode->resolution != LDPR_UNKNOWN)
-	return resolution_to_local_definition_p (vnode->resolution);
-    }
-  else if (TREE_CODE (decl) == FUNCTION_DECL)
-    {
-      struct cgraph_node *node = cgraph_node::get (decl);
-      if (node
-	  && node->resolution != LDPR_UNKNOWN)
+      if (node->resolution != LDPR_UNKNOWN)
 	return resolution_to_local_definition_p (node->resolution);
     }
+
   /* Otherwise we have to assume the worst for DECL_WEAK (hidden weaks
      binds locally but still can be overwritten), DECL_COMMON (can be merged
      with a non-common definition somewhere in the same module) or
-- 
2.1.0

From 2a5208dfd10bf5d9b9c63c1f7464c6b6cacbcd51 Mon Sep 17 00:00:00 2001
From: Richard Henderson <rth@redhat.com>
Date: Wed, 11 Feb 2015 19:37:25 -0800
Subject: [PATCH 3/5] Don't use resolution_to_local_definition_p in
 binds_local_p

---
 gcc/varasm.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/gcc/varasm.c b/gcc/varasm.c
index dcc70e3..f467cc0 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -6837,18 +6837,15 @@ default_binds_local_p_1 (const_tree exp, int shlib)
      because dynamic linking might overwrite symbols
      in shared libraries.  */
   bool resolved_locally = false;
-  bool resolved_to_local_def = false;
   if (symtab_node *node = symtab_node::get (exp))
     {
       if (node->in_other_partition
 	  || resolution_local_p (node->resolution))
 	resolved_locally = true;
-      if (resolution_to_local_definition_p (node->resolution))
-	resolved_to_local_def = true;
     }
 
   /* A variable is local if the user has said explicitly that it will be.  */
-  if ((DECL_VISIBILITY_SPECIFIED (exp) || resolved_to_local_def)
+  if (DECL_VISIBILITY_SPECIFIED (exp)
       && DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
     return true;
 
-- 
2.1.0

From aa50d80d97a27e99cc295e3f2a00662dc73ce235 Mon Sep 17 00:00:00 2001
From: Richard Henderson <rth@redhat.com>
Date: Wed, 11 Feb 2015 20:11:09 -0800
Subject: [PATCH 4/5] Add weak_dominate logic

---
 gcc/varasm.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/gcc/varasm.c b/gcc/varasm.c
index f467cc0..dea9b36 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -6802,17 +6802,8 @@ resolution_local_p (enum ld_plugin_symbol_resolution resolution)
 	  || resolution == LDPR_RESOLVED_EXEC);
 }
 
-/* Assume ELF-ish defaults, since that's pretty much the most liberal
-   wrt cross-module name binding.  */
-
-bool
-default_binds_local_p (const_tree exp)
-{
-  return default_binds_local_p_1 (exp, flag_shlib);
-}
-
-bool
-default_binds_local_p_1 (const_tree exp, int shlib)
+static bool
+default_binds_local_p_2 (const_tree exp, bool shlib, bool weak_dominate)
 {
   /* A non-decl is an entry in the constant pool.  */
   if (!DECL_P (exp))
@@ -6839,11 +6830,18 @@ default_binds_local_p_1 (const_tree exp, int shlib)
   bool resolved_locally = false;
   if (symtab_node *node = symtab_node::get (exp))
     {
-      if (node->in_other_partition
+      /* When not building shared library and weak_dominate is true:
+         weak, common or initialized symbols are resolved locally.  */
+      if ((weak_dominate && !shlib && node->definition)
+	  || node->in_other_partition
 	  || resolution_local_p (node->resolution))
 	resolved_locally = true;
     }
 
+  /* Undefined (or non-dominant) weak symbols are not defined locally.  */
+  if (DECL_WEAK (exp) && !resolved_locally)
+    return false;
+
   /* A variable is local if the user has said explicitly that it will be.  */
   if (DECL_VISIBILITY_SPECIFIED (exp)
       && DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
@@ -6858,11 +6856,6 @@ default_binds_local_p_1 (const_tree exp, int shlib)
   if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT)
     return true;
 
-  /* Default visibility weak data can be overridden by a strong symbol
-     in another module and so are not local.  */
-  if (DECL_WEAK (exp) && !resolved_locally)
-    return false;
-
   /* If PIC, then assume that any global name can be overridden by
      symbols resolved from other modules.  */
   if (shlib)
@@ -6881,6 +6874,21 @@ default_binds_local_p_1 (const_tree exp, int shlib)
   return true;
 }
 
+/* Assume ELF-ish defaults, since that's pretty much the most liberal
+   wrt cross-module name binding.  */
+
+bool
+default_binds_local_p (const_tree exp)
+{
+  return default_binds_local_p_2 (exp, flag_shlib != 0, true);
+}
+
+bool
+default_binds_local_p_1 (const_tree exp, int shlib)
+{
+  return default_binds_local_p_2 (exp, shlib != 0, false);
+}
+
 /* Return true when references to DECL must bind to current definition in
    final executable.
 
-- 
2.1.0

From 81db9e951ac27c9a7f94f88546d90c852a927d54 Mon Sep 17 00:00:00 2001
From: Richard Henderson <rth@redhat.com>
Date: Wed, 11 Feb 2015 22:16:38 -0800
Subject: [PATCH 5/5] Rest of HJL patch

---
 gcc/cgraphunit.c                          |  4 +++-
 gcc/testsuite/gcc.dg/visibility-22.c      | 17 +++++++++++++++++
 gcc/testsuite/gcc.dg/visibility-23.c      | 15 +++++++++++++++
 gcc/testsuite/gcc.target/i386/pr32219-1.c | 16 ++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr32219-2.c | 16 ++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr32219-3.c | 17 +++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr32219-4.c | 17 +++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr32219-5.c | 16 ++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr32219-6.c | 16 ++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr32219-7.c | 17 +++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr32219-8.c | 17 +++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr64317.c   |  2 +-
 gcc/varasm.c                              |  5 +++--
 13 files changed, 171 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/visibility-22.c
 create mode 100644 gcc/testsuite/gcc.dg/visibility-23.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-5.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-6.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-7.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr32219-8.c

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index f2c40d4..057eedb 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -803,8 +803,10 @@ varpool_node::finalize_decl (tree decl)
 
   if (node->definition)
     return;
-  notice_global_symbol (decl);
+  /* Set definition first before calling notice_global_symbol so that
+     it is available to notice_global_symbol.  */
   node->definition = true;
+  notice_global_symbol (decl);
   if (TREE_THIS_VOLATILE (decl) || DECL_PRESERVE_P (decl)
       /* Traditionally we do not eliminate static variables when not
 	 optimizing and when not doing toplevel reoder.  */
diff --git a/gcc/testsuite/gcc.dg/visibility-22.c b/gcc/testsuite/gcc.dg/visibility-22.c
new file mode 100644
index 0000000..52f59be
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/visibility-22.c
@@ -0,0 +1,17 @@
+/* PR target/32219 */
+/* { dg-do run } */
+/* { dg-require-visibility "" } */
+/* { dg-options "-O2 -fPIC" { target fpic } } */
+/* This test requires support for undefined weak symbols.  This support
+   is not available on hppa*-*-hpux*.  The test is skipped rather than
+   xfailed to suppress the warning that would otherwise arise.  */
+/* { dg-skip-if "" { "hppa*-*-hpux*" "*-*-aix*" "*-*-darwin*" } "*" { "" } }  */
+
+extern void foo () __attribute__((weak,visibility("hidden")));
+int
+main()
+{
+  if (foo)
+    foo ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/visibility-23.c b/gcc/testsuite/gcc.dg/visibility-23.c
new file mode 100644
index 0000000..0fa9ef4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/visibility-23.c
@@ -0,0 +1,15 @@
+/* PR target/32219 */
+/* { dg-do compile } */
+/* { dg-require-visibility "" } */
+/* { dg-final { scan-hidden "foo" } } */
+/* { dg-options "-O2 -fPIC" { target fpic } } */
+/* { dg-skip-if "" { "hppa*-*-hpux*" "*-*-aix*" "*-*-darwin*" } "*" { "" } }  */
+
+extern void foo () __attribute__((weak,visibility("hidden")));
+int
+main()
+{
+  if (foo)
+    foo ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-1.c b/gcc/testsuite/gcc.target/i386/pr32219-1.c
new file mode 100644
index 0000000..5bd80a0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-1.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie" } */
+
+/* Common symbol with -fpie.  */
+int xxx;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-2.c b/gcc/testsuite/gcc.target/i386/pr32219-2.c
new file mode 100644
index 0000000..0cf2eb5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic" } */
+
+/* Common symbol with -fpic.  */
+int xxx;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-3.c b/gcc/testsuite/gcc.target/i386/pr32219-3.c
new file mode 100644
index 0000000..911f2a5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-3.c
@@ -0,0 +1,17 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie" } */
+
+/* Weak common symbol with -fpie.  */
+__attribute__((weak))
+int xxx;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-4.c b/gcc/testsuite/gcc.target/i386/pr32219-4.c
new file mode 100644
index 0000000..3d43439
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-4.c
@@ -0,0 +1,17 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic" } */
+
+/* Weak common symbol with -fpic.  */
+__attribute__((weak))
+int xxx;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-5.c b/gcc/testsuite/gcc.target/i386/pr32219-5.c
new file mode 100644
index 0000000..ee7442e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-5.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie" } */
+
+/* Initialized symbol with -fpie.  */
+int xxx = -1;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-6.c b/gcc/testsuite/gcc.target/i386/pr32219-6.c
new file mode 100644
index 0000000..f261433
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-6.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic" } */
+
+/* Initialized symbol with -fpic.  */
+int xxx = -1;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-7.c b/gcc/testsuite/gcc.target/i386/pr32219-7.c
new file mode 100644
index 0000000..12aaf72
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-7.c
@@ -0,0 +1,17 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie" } */
+
+/* Weak initialized symbol with -fpie.  */
+__attribute__((weak))
+int xxx = -1;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr32219-8.c b/gcc/testsuite/gcc.target/i386/pr32219-8.c
new file mode 100644
index 0000000..2e4fba0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr32219-8.c
@@ -0,0 +1,17 @@
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpic" } */
+
+/* Weak initialized symbol with -fpic.  */
+__attribute__((weak))
+int xxx = -1;
+
+int
+foo ()
+{
+  return xxx;
+}
+
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx\\(%rip\\), %eax" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "xxx@GOTPCREL" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-not "movl\[ \t\]xxx@GOTOFF\\(%\[^,\]*\\), %eax" { target ia32 } } } */
+/* { dg-final { scan-assembler "movl\[ \t\]xxx@GOT\\(%\[^,\]*\\), %eax" { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr64317.c b/gcc/testsuite/gcc.target/i386/pr64317.c
index 33f5b5d..32969fc 100644
--- a/gcc/testsuite/gcc.target/i386/pr64317.c
+++ b/gcc/testsuite/gcc.target/i386/pr64317.c
@@ -1,7 +1,7 @@
 /* { dg-do compile { target { *-*-linux* && ia32 } } } */
 /* { dg-options "-O2 -fpie" } */
 /* { dg-final { scan-assembler "addl\[ \\t\]+\[$\]_GLOBAL_OFFSET_TABLE_, %ebx" } } */
-/* { dg-final { scan-assembler "movl\[ \\t\]+c@GOT\[(\]%ebx\[)\]" } } */
+/* { dg-final { scan-assembler "movl\[ \\t\]+c@GOTOFF\[(\]%ebx\[)\]" } } */
 /* { dg-final { scan-assembler-not "movl\[ \\t\]+\[0-9]+\[(\]%esp\[)\], %ebx" } } */
 long c;
 
diff --git a/gcc/varasm.c b/gcc/varasm.c
index dea9b36..9f79416 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -7435,9 +7435,10 @@ default_elf_asm_output_external (FILE *file ATTRIBUTE_UNUSED,
 {
   /* We output the name if and only if TREE_SYMBOL_REFERENCED is
      set in order to avoid putting out names that are never really
-     used. */
+     used.  Always output visibility specified in the source.  */
   if (TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl))
-      && targetm.binds_local_p (decl))
+      && (DECL_VISIBILITY_SPECIFIED (decl)
+	  || targetm.binds_local_p (decl)))
     maybe_assemble_visibility (decl);
 }
 
-- 
2.1.0