Patchwork [GOOGLE] Change function naming to use context function assembler name to replace function id

login
register
mail settings
Submitter Dehao Chen
Date May 1, 2013, 3:01 p.m.
Message ID <CAO2gOZXh4ED4mPBY1YXNft1fJ1=e6Co10rq0gYL5WEzgaZEPnQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/240774/
State New
Headers show

Comments

Dehao Chen - May 1, 2013, 3:01 p.m.
The patch is updated to fix some bugs in the impl.

Thanks,
Dehao


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 - May 1, 2013, 4:49 p.m.
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
Dehao Chen - May 1, 2013, 5:27 p.m.
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

Patch

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);