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

login
register
mail settings
Submitter Dehao Chen
Date May 2, 2013, 11:38 p.m.
Message ID <CAO2gOZUgdYj9Dz-iOMy3f4tKsAhKK60_YLc_rFakG6Vd9ae7iQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/241118/
State New
Headers show

Comments

Dehao Chen - May 2, 2013, 11:38 p.m.
The root problem is that -fsigned-char and -funsigned-char is
incompatible. The fix of the problem:

Ok for google branches?

Thanks,
Dehao

On Wed, May 1, 2013 at 10:26 AM, Dehao Chen <dehao@google.com> wrote:
> 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);
Xinliang David Li - May 2, 2013, 11:42 p.m.
ok.

thanks,

David

On Thu, May 2, 2013 at 4:38 PM, Dehao Chen <dehao@google.com> wrote:
> The root problem is that -fsigned-char and -funsigned-char is
> incompatible. The fix of the problem:
>
> Index: coverage.c
> ===================================================================
> --- coverage.c (revision 198362)
> +++ coverage.c (working copy)
> @@ -336,6 +336,7 @@
>      { "-fsized-delete", "-fno-sized-delete", false },
>      { "-frtti", "-fno-rtti", true },
>      { "-fstrict-aliasing", "-fno-strict-aliasing", true },
> +    { "-fsigned-char", "-funsigned-char", true},
>      { NULL, NULL, false }
>    };
>
> Ok for google branches?
>
> Thanks,
> Dehao
>
> On Wed, May 1, 2013 at 10:26 AM, Dehao Chen <dehao@google.com> wrote:
>> 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: coverage.c
===================================================================
--- coverage.c (revision 198362)
+++ coverage.c (working copy)
@@ -336,6 +336,7 @@ 
     { "-fsized-delete", "-fno-sized-delete", false },
     { "-frtti", "-fno-rtti", true },
     { "-fstrict-aliasing", "-fno-strict-aliasing", true },
+    { "-fsigned-char", "-funsigned-char", true},
     { NULL, NULL, false }
   };