| Message ID | 4CF58F86.1080305@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Dec 1, 2010 at 12:57 AM, Jie Zhang <jie@codesourcery.com> wrote: > PR 46674 is same as PR 46221 indeed. It happens when one alias has a user > set name. > > extern void jelly (void) __asm__ ("jelly2") __attribute__ ((alias > ("dessert"), weak)); > extern void wobbly (void) __attribute__ ((alias ("jelly2"), weak)); > > When jelly was added to the visible point set, the identifier was "*jelly2". > But later GCC looked for identifier "jelly2" when deciding if alias wobbly > could be removed or not. Since "jelly2" was not in the visible point set, > alias wobbly would be removed. > > With this patch, the identifier added to the visible point set has the > prefix '*' stripped if there is one. > > Bootstrapped and regression tested on x86_64-linux-gnu with c,c++,lto. > > OK? Hm. If a target prefixes an underscore does the alias attribute argument need to include that underscore? I'm somewhat worried about consistency on those weird targets, but the patch is clearly a progression. It would be really nice if DECL_ASSEMBLER_NAME would already contain all target mangling from the start (also for LTO which has similar problems when doing symbol name merging). Thus, ok. Thanks, Richard. > Regards, > -- > Jie Zhang > > >
On 12/01/2010 07:50 PM, Richard Guenther wrote: > On Wed, Dec 1, 2010 at 12:57 AM, Jie Zhang<jie@codesourcery.com> wrote: >> PR 46674 is same as PR 46221 indeed. It happens when one alias has a user >> set name. >> >> extern void jelly (void) __asm__ ("jelly2") __attribute__ ((alias >> ("dessert"), weak)); >> extern void wobbly (void) __attribute__ ((alias ("jelly2"), weak)); >> >> When jelly was added to the visible point set, the identifier was "*jelly2". >> But later GCC looked for identifier "jelly2" when deciding if alias wobbly >> could be removed or not. Since "jelly2" was not in the visible point set, >> alias wobbly would be removed. >> >> With this patch, the identifier added to the visible point set has the >> prefix '*' stripped if there is one. >> >> Bootstrapped and regression tested on x86_64-linux-gnu with c,c++,lto. >> >> OK? > > Hm. If a target prefixes an underscore does the alias attribute argument > need to include that underscore? I'm somewhat worried about > consistency on those weird targets, but the patch is clearly a > progression. > > It would be really nice if DECL_ASSEMBLER_NAME would already > contain all target mangling from the start (also for LTO which has > similar problems when doing symbol name merging). > Yeah. At least it's not a good to use a prefix '*' to mark its name is set by user. I think it will be better to use a tree field to record this but keep the symbol name unchanged. > Thus, ok. > Thanks. Committed on trunk.
On 01/12/2010 11:50, Richard Guenther wrote: > Hm. If a target prefixes an underscore does the alias attribute argument > need to include that underscore? FAIL: gcc.dg/pr46674.c (test for excess errors) That answer your question? ;-) How about the attached? It fixes the test on i686-pc-cygwin, checked with both variants of f{no-,}leading-underscore (and none). gcc/testsuite/ChangeLog: * gcc.dg/pr46674.c (LABEL3): New macro definition. (LABEL2): Likewise. (LABEL): Likewise. (jelly): Account for user label prefix in asm name. > It would be really nice if DECL_ASSEMBLER_NAME would already > contain all target mangling from the start (also for LTO which has > similar problems when doing symbol name merging). We're working towards that, as you'll have seen from the "Enable -fuse-linker-plugin by default when possible, take 2" thread. :) cheers, DaveK
On Sun, Dec 5, 2010 at 2:50 PM, Dave Korn <dave.korn.cygwin@gmail.com> wrote: > On 01/12/2010 11:50, Richard Guenther wrote: > >> Hm. If a target prefixes an underscore does the alias attribute argument >> need to include that underscore? > > FAIL: gcc.dg/pr46674.c (test for excess errors) > > That answer your question? ;-) How about the attached? It fixes the test > on i686-pc-cygwin, checked with both variants of f{no-,}leading-underscore > (and none). Ok ;) Thanks, Richard. > gcc/testsuite/ChangeLog: > > * gcc.dg/pr46674.c (LABEL3): New macro definition. > (LABEL2): Likewise. > (LABEL): Likewise. > (jelly): Account for user label prefix in asm name. > >> It would be really nice if DECL_ASSEMBLER_NAME would already >> contain all target mangling from the start (also for LTO which has >> similar problems when doing symbol name merging). > > We're working towards that, as you'll have seen from the "Enable > -fuse-linker-plugin by default when possible, take 2" thread. :) > > > cheers, > DaveK >
On 05/12/2010 20:47, Richard Guenther wrote: > On Sun, Dec 5, 2010 at 2:50 PM, Dave Korn <dave.korn.cygwin@gmail.com> wrote: >> On 01/12/2010 11:50, Richard Guenther wrote: >> >>> Hm. If a target prefixes an underscore does the alias attribute argument >>> need to include that underscore? >> FAIL: gcc.dg/pr46674.c (test for excess errors) >> >> That answer your question? ;-) How about the attached? It fixes the test >> on i686-pc-cygwin, checked with both variants of f{no-,}leading-underscore >> (and none). > > Ok ;) Committed revision 167483. cheers, DaveK
PR middle-end/46674 * varasm.c (compute_visible_aliases): Handle user set assembler name. testsuite/ PR middle-end/46674 * gcc.dg/pr46674.c: New test. Index: testsuite/gcc.dg/pr46674.c =================================================================== --- testsuite/gcc.dg/pr46674.c (revision 0) +++ testsuite/gcc.dg/pr46674.c (revision 0) @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-require-alias "" } */ +/* { dg-options "-O2" } */ + +int yum; +void dessert (void) { ++yum; } +extern void jelly (void) __asm__ ("jelly2") __attribute__ ((alias ("dessert"), weak)); +extern void wobbly (void) __attribute__ ((alias ("jelly2"), weak)); + +/* { dg-final { scan-assembler "wobbly" } } */ +/* { dg-final { scan-assembler "jelly2" } } */ Index: varasm.c =================================================================== --- varasm.c (revision 167217) +++ varasm.c (working copy) @@ -5526,12 +5526,21 @@ compute_visible_aliases (void) { struct cgraph_node *fnode = NULL; struct varpool_node *vnode = NULL; + tree asmname = DECL_ASSEMBLER_NAME (p->decl); + const char *str = IDENTIFIER_POINTER (asmname); + + if (str[0] == '*') + { + str ++; + asmname = get_identifier (str); + } + fnode = cgraph_node_for_asm (p->target); vnode = (fnode == NULL) ? varpool_node_for_asm (p->target) : NULL; if ((fnode || vnode || pointer_set_contains (visible, p->target)) - && !pointer_set_insert (visible, DECL_ASSEMBLER_NAME (p->decl))) + && !pointer_set_insert (visible, asmname)) changed = true; } }
PR 46674 is same as PR 46221 indeed. It happens when one alias has a user set name. extern void jelly (void) __asm__ ("jelly2") __attribute__ ((alias ("dessert"), weak)); extern void wobbly (void) __attribute__ ((alias ("jelly2"), weak)); When jelly was added to the visible point set, the identifier was "*jelly2". But later GCC looked for identifier "jelly2" when deciding if alias wobbly could be removed or not. Since "jelly2" was not in the visible point set, alias wobbly would be removed. With this patch, the identifier added to the visible point set has the prefix '*' stripped if there is one. Bootstrapped and regression tested on x86_64-linux-gnu with c,c++,lto. OK? Regards,