| Message ID | CAO2gOZXh4ED4mPBY1YXNft1fJ1=e6Co10rq0gYL5WEzgaZEPnQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, May 1, 2013 at 8:01 AM, Dehao Chen <dehao@google.com> wrote: > The patch is updated to fix some bugs in the impl. > > Thanks, > Dehao > > Index: gcc/l-ipo.c > =================================================================== > --- gcc/l-ipo.c (revision 198362) > +++ gcc/l-ipo.c (working copy) > @@ -1713,10 +1713,11 @@ static tree > create_unique_name (tree decl, unsigned module_id) > { > tree id, assemb_id; > - char *assembler_name; > + char *assembler_name, *i; > + const char *context = NULL; > const char *name; > - struct function *context = NULL; > int seq = 0; > + int len; > > if (TREE_CODE (decl) == FUNCTION_DECL) > { > @@ -1740,7 +1741,8 @@ create_unique_name (tree decl, unsigned module_id) > else if (TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) > { > id = DECL_NAME (decl); > - context = DECL_STRUCT_FUNCTION (DECL_CONTEXT (decl)); > + context = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME ( > + DECL_CONTEXT (decl))); > } > else > /* file scope context */ > @@ -1748,17 +1750,15 @@ create_unique_name (tree decl, unsigned module_id) > } > > name = IDENTIFIER_POINTER (id); > + len = strlen (name) + (context ? strlen (context) : 0); > + assembler_name = (char*) alloca (len + 30); > if (context) > - { > - char *n; > - unsigned fno = FUNC_DECL_FUNC_ID (context); > - n = (char *)alloca (strlen (name) + 15); > - sprintf (n, "%s.%u", name, fno); > - name = n; > - } > - > - assembler_name = (char*) alloca (strlen (name) + 30); > - sprintf (assembler_name, "%s.cmo.%u", name, module_id); > + sprintf (assembler_name, "%s.%s.cmo.%u", context, name, module_id); > + else > + sprintf (assembler_name, "%s.cmo.%u", name, module_id); > + for (i = assembler_name; *i; i++) > + if (*i == '*' || *i == ' ') > + *i = '_'; I thought the DECL_ASSEMBLER_NAME was the mangled name, so why does it have '*' and ' ' chars? Oh, I do see in tree.h comments indicating the first char will be '*' if the user set the name, so maybe you just need to handle that? Teresa > seq = get_name_seq_num (assembler_name); > if (seq) > sprintf (assembler_name, "%s.%d", assembler_name, seq); > > On Tue, Apr 30, 2013 at 4:10 PM, Dehao Chen <dehao@google.com> wrote: >> This patch changes to use context function name to replace function >> id, which is not available in AutoFDO builds. >> >> Bootstrapped and passed regression tests. >> >> OK for google branches? >> >> Thanks, >> Dehao >> >> Index: gcc/l-ipo.c >> =================================================================== >> --- gcc/l-ipo.c (revision 198469) >> +++ gcc/l-ipo.c (working copy) >> @@ -1714,9 +1714,10 @@ create_unique_name (tree decl, unsigned module_id) >> { >> tree id, assemb_id; >> char *assembler_name; >> + const char *context = NULL; >> const char *name; >> - struct function *context = NULL; >> int seq = 0; >> + int len; >> >> if (TREE_CODE (decl) == FUNCTION_DECL) >> { >> @@ -1740,7 +1741,8 @@ create_unique_name (tree decl, unsigned module_id) >> else if (TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) >> { >> id = DECL_NAME (decl); >> - context = DECL_STRUCT_FUNCTION (DECL_CONTEXT (decl)); >> + context = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME ( >> + DECL_CONTEXT (decl))); >> } >> else >> /* file scope context */ >> @@ -1748,17 +1750,12 @@ create_unique_name (tree decl, unsigned module_id) >> } >> >> name = IDENTIFIER_POINTER (id); >> + len = strlen (name) + context ? strlen (context) : 0; >> + assembler_name = (char*) alloca (len + 30); >> if (context) >> - { >> - char *n; >> - unsigned fno = FUNC_DECL_FUNC_ID (context); >> - n = (char *)alloca (strlen (name) + 15); >> - sprintf (n, "%s.%u", name, fno); >> - name = n; >> - } >> - >> - assembler_name = (char*) alloca (strlen (name) + 30); >> - sprintf (assembler_name, "%s.cmo.%u", name, module_id); >> + sprintf (assembler_name, "%s.%s.cmo.%u", context, name, module_id); >> + else >> + sprintf (assembler_name, "%s.cmo.%u", name, module_id); >> seq = get_name_seq_num (assembler_name); >> if (seq) >> sprintf (assembler_name, "%s.%d", assembler_name, seq); -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
I see a case when the DECL_ASSEMBLER_NAME has " *INTERNAL* " as the suffix. Dehao On Wed, May 1, 2013 at 9:49 AM, Teresa Johnson <tejohnson@google.com> wrote: > On Wed, May 1, 2013 at 8:01 AM, Dehao Chen <dehao@google.com> wrote: >> The patch is updated to fix some bugs in the impl. >> >> Thanks, >> Dehao >> >> Index: gcc/l-ipo.c >> =================================================================== >> --- gcc/l-ipo.c (revision 198362) >> +++ gcc/l-ipo.c (working copy) >> @@ -1713,10 +1713,11 @@ static tree >> create_unique_name (tree decl, unsigned module_id) >> { >> tree id, assemb_id; >> - char *assembler_name; >> + char *assembler_name, *i; >> + const char *context = NULL; >> const char *name; >> - struct function *context = NULL; >> int seq = 0; >> + int len; >> >> if (TREE_CODE (decl) == FUNCTION_DECL) >> { >> @@ -1740,7 +1741,8 @@ create_unique_name (tree decl, unsigned module_id) >> else if (TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) >> { >> id = DECL_NAME (decl); >> - context = DECL_STRUCT_FUNCTION (DECL_CONTEXT (decl)); >> + context = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME ( >> + DECL_CONTEXT (decl))); >> } >> else >> /* file scope context */ >> @@ -1748,17 +1750,15 @@ create_unique_name (tree decl, unsigned module_id) >> } >> >> name = IDENTIFIER_POINTER (id); >> + len = strlen (name) + (context ? strlen (context) : 0); >> + assembler_name = (char*) alloca (len + 30); >> if (context) >> - { >> - char *n; >> - unsigned fno = FUNC_DECL_FUNC_ID (context); >> - n = (char *)alloca (strlen (name) + 15); >> - sprintf (n, "%s.%u", name, fno); >> - name = n; >> - } >> - >> - assembler_name = (char*) alloca (strlen (name) + 30); >> - sprintf (assembler_name, "%s.cmo.%u", name, module_id); >> + sprintf (assembler_name, "%s.%s.cmo.%u", context, name, module_id); >> + else >> + sprintf (assembler_name, "%s.cmo.%u", name, module_id); >> + for (i = assembler_name; *i; i++) >> + if (*i == '*' || *i == ' ') >> + *i = '_'; > > I thought the DECL_ASSEMBLER_NAME was the mangled name, so why does it > have '*' and ' ' chars? Oh, I do see in tree.h comments indicating > the first char will be '*' if the user set the name, so maybe you just > need to handle that? > > Teresa > >> seq = get_name_seq_num (assembler_name); >> if (seq) >> sprintf (assembler_name, "%s.%d", assembler_name, seq); >> >> On Tue, Apr 30, 2013 at 4:10 PM, Dehao Chen <dehao@google.com> wrote: >>> This patch changes to use context function name to replace function >>> id, which is not available in AutoFDO builds. >>> >>> Bootstrapped and passed regression tests. >>> >>> OK for google branches? >>> >>> Thanks, >>> Dehao >>> >>> Index: gcc/l-ipo.c >>> =================================================================== >>> --- gcc/l-ipo.c (revision 198469) >>> +++ gcc/l-ipo.c (working copy) >>> @@ -1714,9 +1714,10 @@ create_unique_name (tree decl, unsigned module_id) >>> { >>> tree id, assemb_id; >>> char *assembler_name; >>> + const char *context = NULL; >>> const char *name; >>> - struct function *context = NULL; >>> int seq = 0; >>> + int len; >>> >>> if (TREE_CODE (decl) == FUNCTION_DECL) >>> { >>> @@ -1740,7 +1741,8 @@ create_unique_name (tree decl, unsigned module_id) >>> else if (TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) >>> { >>> id = DECL_NAME (decl); >>> - context = DECL_STRUCT_FUNCTION (DECL_CONTEXT (decl)); >>> + context = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME ( >>> + DECL_CONTEXT (decl))); >>> } >>> else >>> /* file scope context */ >>> @@ -1748,17 +1750,12 @@ create_unique_name (tree decl, unsigned module_id) >>> } >>> >>> name = IDENTIFIER_POINTER (id); >>> + len = strlen (name) + context ? strlen (context) : 0; >>> + assembler_name = (char*) alloca (len + 30); >>> if (context) >>> - { >>> - char *n; >>> - unsigned fno = FUNC_DECL_FUNC_ID (context); >>> - n = (char *)alloca (strlen (name) + 15); >>> - sprintf (n, "%s.%u", name, fno); >>> - name = n; >>> - } >>> - >>> - assembler_name = (char*) alloca (strlen (name) + 30); >>> - sprintf (assembler_name, "%s.cmo.%u", name, module_id); >>> + sprintf (assembler_name, "%s.%s.cmo.%u", context, name, module_id); >>> + else >>> + sprintf (assembler_name, "%s.cmo.%u", name, module_id); >>> seq = get_name_seq_num (assembler_name); >>> if (seq) >>> sprintf (assembler_name, "%s.%d", assembler_name, seq); > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Index: gcc/l-ipo.c =================================================================== --- gcc/l-ipo.c (revision 198362) +++ gcc/l-ipo.c (working copy) @@ -1713,10 +1713,11 @@ static tree create_unique_name (tree decl, unsigned module_id) { tree id, assemb_id; - char *assembler_name; + char *assembler_name, *i; + const char *context = NULL; const char *name; - struct function *context = NULL; int seq = 0; + int len; if (TREE_CODE (decl) == FUNCTION_DECL) { @@ -1740,7 +1741,8 @@ create_unique_name (tree decl, unsigned module_id) else if (TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) { id = DECL_NAME (decl); - context = DECL_STRUCT_FUNCTION (DECL_CONTEXT (decl)); + context = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME ( + DECL_CONTEXT (decl))); } else /* file scope context */ @@ -1748,17 +1750,15 @@ create_unique_name (tree decl, unsigned module_id) } name = IDENTIFIER_POINTER (id); + len = strlen (name) + (context ? strlen (context) : 0); + assembler_name = (char*) alloca (len + 30); if (context) - { - char *n; - unsigned fno = FUNC_DECL_FUNC_ID (context); - n = (char *)alloca (strlen (name) + 15); - sprintf (n, "%s.%u", name, fno); - name = n; - } - - assembler_name = (char*) alloca (strlen (name) + 30); - sprintf (assembler_name, "%s.cmo.%u", name, module_id); + sprintf (assembler_name, "%s.%s.cmo.%u", context, name, module_id); + else + sprintf (assembler_name, "%s.cmo.%u", name, module_id); + for (i = assembler_name; *i; i++) + if (*i == '*' || *i == ' ') + *i = '_'; seq = get_name_seq_num (assembler_name); if (seq) sprintf (assembler_name, "%s.%d", assembler_name, seq);