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

login
register
mail settings
Submitter Dehao Chen
Date April 30, 2013, 11:10 p.m.
Message ID <CAO2gOZUYnTbqnN_0vD_Q-K0KO7xFgrNXOoDx56eninn5jAjMYg@mail.gmail.com>
Download mbox | patch
Permalink /patch/240701/
State New
Headers show

Comments

Dehao Chen - April 30, 2013, 11:10 p.m.
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
Xinliang David Li - May 1, 2013, 4:57 p.m.
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.

Why isn't func_id not available in autofdo builds? The func-id for the
the same function should remain the same regardless whether the module
is compiled as an aux module or the primary module.

David

>
> 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);
Dehao Chen - May 1, 2013, 5:26 p.m.
I've seen a case when func_id in aux module is not the same (off by
1). This is when -fexception is specified. I had not looked into why
though. I'll find out why it is off-by-1

Dehao

On Wed, May 1, 2013 at 9:57 AM, Xinliang David Li <davidxl@google.com> wrote:
> 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.
>
> Why isn't func_id not available in autofdo builds? The func-id for the
> the same function should remain the same regardless whether the module
> is compiled as an aux module or the primary module.
>
> David
>
>>
>> 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);

Patch

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