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

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, Feb 12, 2015 at 3:04 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Feb 12, 2015 at 11:25 AM, Richard Henderson <rth@redhat.com> wrote:
>> On 02/12/2015 10:58 AM, H.J. Lu wrote:
>>>    if (DECL_VISIBILITY_SPECIFIED (exp)
>>> +      && (resolved_locally
>>> +       || !flag_pic
>>
>> Yes, this essentially goes back to your original patch, which I claim still
>> conflates issues.
>>
>> In particular, I believe mentioning flag_pic here is a serious error.
>>
>> There are two problems that I see so far,
>>
>>  (1) node->definition isn't set for this symbol.  This is because you
>>      only fixed varpool_node::finalize_decl, and not
>>      cgraph_node::finalize_function.
>>
>>  (2) The weak test should probably be split into two pieces, like the
>>      visibility test: exclude undefined weak, include specified visibility,
>>      exclude non-dominant weak, exclude external, include implied visibility.
>>
>
> How about this patch?
>

Oops.  Wrong one.


-- 
H.J.

From afbb56ab924d9f419ac4f65f5c535ebdbd22f16e Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 12 Feb 2015 15:03:00 -0800
Subject: [PATCH] non-default visibility is local

---
 gcc/cgraphunit.c |  4 +++-
 gcc/varasm.c     | 12 +++++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 057eedb..942826d 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -442,8 +442,10 @@ cgraph_node::finalize_function (tree decl, bool no_collect)
       node->local.redefined_extern_inline = true;
     }
 
-  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);
   node->lowered = DECL_STRUCT_FUNCTION (decl)->cfg != NULL;
 
   /* With -fkeep-inline-functions we are keeping all inline functions except
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 9f79416..5d7cba1 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -6830,9 +6830,15 @@ default_binds_local_p_2 (const_tree exp, bool shlib, bool weak_dominate)
   bool resolved_locally = false;
   if (symtab_node *node = symtab_node::get (exp))
     {
-      /* When not building shared library and weak_dominate is true:
-         weak, common or initialized symbols are resolved locally.  */
-      if ((weak_dominate && !shlib && node->definition)
+      /* When weak_dominate is true and not building shared library or
+	 non-default visibility is specified by user: weak, common or
+	 initialized symbols are resolved locally.
+	 */
+      if (((!shlib
+	    || (DECL_VISIBILITY_SPECIFIED (exp)
+		&& DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT))
+	   && weak_dominate
+	   && node->definition)
 	  || node->in_other_partition
 	  || resolution_local_p (node->resolution))
 	resolved_locally = true;
-- 
1.9.3