[GOOGLE] Avoid emitting duplicated weakref

[GOOGLE] Avoid emitting duplicated weakref

[Dehao Chen]

This patch fixes a bug when two weakref symbols are mapped to a same
assembler name.

Testing on going.

OK for google branches if test is fine?

Thanks,
Dehao

Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c (revision 199844)
+++ gcc/varasm.c (working copy)
@@ -5502,6 +5502,10 @@ do_assemble_alias (tree decl, tree target)
   if (TREE_ASM_WRITTEN (decl))
     return;

+  if (lookup_attribute ("weakref", DECL_ATTRIBUTES (decl))
+      && TREE_ASM_WRITTEN (DECL_ASSEMBLER_NAME (decl)))
+    return;
+
   /* We must force creation of DECL_RTL for debug info generation, even though
      we don't use it here.  */
   make_decl_rtl (decl);

Re: [GOOGLE] Avoid emitting duplicated weakref

[Xinliang David Li]

Guard also with L_IPO_COMP_MODE as this is lipo specific.

David

On Sat, Jun 8, 2013 at 8:29 PM, Dehao Chen <dehao@google.com> wrote:
> This patch fixes a bug when two weakref symbols are mapped to a same
> assembler name.
>
> Testing on going.
>
> OK for google branches if test is fine?
>
> Thanks,
> Dehao
>
> Index: gcc/varasm.c
> ===================================================================
> --- gcc/varasm.c (revision 199844)
> +++ gcc/varasm.c (working copy)
> @@ -5502,6 +5502,10 @@ do_assemble_alias (tree decl, tree target)
>    if (TREE_ASM_WRITTEN (decl))
>      return;
>
> +  if (lookup_attribute ("weakref", DECL_ATTRIBUTES (decl))
> +      && TREE_ASM_WRITTEN (DECL_ASSEMBLER_NAME (decl)))
> +    return;
> +
>    /* We must force creation of DECL_RTL for debug info generation, even though
>       we don't use it here.  */
>    make_decl_rtl (decl);

Re: [GOOGLE] Avoid emitting duplicated weakref

[Bernhard Reutner-Fischer]

On 9 June 2013 05:31, Xinliang David Li <davidxl@google.com> wrote:
> Guard also with L_IPO_COMP_MODE as this is lipo specific.

Sounds like this is the LIPO incarnation of http://gcc.gnu.org/PR31537

See the patch at http://gcc.gnu.org/PR31537#c9 for not adding the
alias in the first place (back then, honza may have changed that
recently on trunk).

Honza, Joseph,
Are there rules for handling weakrefs in some standard? If so, which ones?

Just curious..
Thanks,
>
> David
>
> On Sat, Jun 8, 2013 at 8:29 PM, Dehao Chen <dehao@google.com> wrote:
>> This patch fixes a bug when two weakref symbols are mapped to a same
>> assembler name.
>>
>> Testing on going.
>>
>> OK for google branches if test is fine?
>>
>> Thanks,
>> Dehao
>>
>> Index: gcc/varasm.c
>> ===================================================================
>> --- gcc/varasm.c (revision 199844)
>> +++ gcc/varasm.c (working copy)
>> @@ -5502,6 +5502,10 @@ do_assemble_alias (tree decl, tree target)
>>    if (TREE_ASM_WRITTEN (decl))
>>      return;
>>
>> +  if (lookup_attribute ("weakref", DECL_ATTRIBUTES (decl))
>> +      && TREE_ASM_WRITTEN (DECL_ASSEMBLER_NAME (decl)))
>> +    return;
>> +
>>    /* We must force creation of DECL_RTL for debug info generation, even though
>>       we don't use it here.  */
>>    make_decl_rtl (decl);

Re: [GOOGLE] Avoid emitting duplicated weakref

[Jan Hubicka]

> On 9 June 2013 05:31, Xinliang David Li <davidxl@google.com> wrote:
> > Guard also with L_IPO_COMP_MODE as this is lipo specific.
> 
> Sounds like this is the LIPO incarnation of http://gcc.gnu.org/PR31537
> 
> See the patch at http://gcc.gnu.org/PR31537#c9 for not adding the
> alias in the first place (back then, honza may have changed that
> recently on trunk).
> 
> Honza, Joseph,
> Are there rules for handling weakrefs in some standard? If so, which ones?

Not that I would know of. It is an extension engineered for glibc pthreads.  in
mainline I quite changed way weakrefs are handled and I still plan to do more.
What LTO does now is to  handle weakref symbol as static symbol despite it has
DECL_EXTERNAL on it.  (I have patch in queue to really make it static).
This is because the symbol never hits the exported symbol table anyhow.

I.e. you can have:
 cat f1.i
static __gthrw_pthread_once __attribute__ ((__weakref__ ("pthread_once")));
$ cat f2.i
static __gthrw_pthread_once __attribute__ ((__weakref__ ("comething_completely_else")));

that will break with your do_assemble_alias change. with LTO you will now get
__gthrw_pthread_once.1 and __gthrw_pthread_once.2 with proper weakref targets.

Honza