Patchwork Generate a label for the split cold function while using -freorder-blocks-and-partition

login
register
mail settings
Submitter Sriraman Tallam
Date April 25, 2013, 11:50 p.m.
Message ID <CAAs8Hmy9-2GQU3CKkQ-n_otn4Opwy8cEt-uzw8C2BYZgp-KqHQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/239606/
State New
Headers show

Comments

Sriraman Tallam - April 25, 2013, 11:50 p.m.
Attaching an updated patch.

Thanks
Sri

On Thu, Apr 25, 2013 at 4:42 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> On Tue, Apr 23, 2013 at 9:59 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Tue, Apr 23, 2013 at 03:58:06PM -0700, Sriraman Tallam wrote:
>>>   This patch generates labels for cold function parts that are split when
>>> using the option -freorder-blocks-and-partition.  The cold label name
>>> is generated by suffixing ".cold" to the assembler name of the hot
>>> function.
>>>
>>>   This is useful when getting back traces from gdb when the cold function
>>> part does get executed.
>>>
>>>         * final.c (final_scan_insn): Generate cold label name by suffixing
>>>         ".cold" to function's assembler name.
>>>         * gcc.dg/tree-prof/cold_partition_label.c: New test.
>>
>> This doesn't honor NO_DOT_IN_LABEL (and NO_DOLLAR_IN_LABEL).
>
> Fixed, by calling clean_symbol_name
>
>> Also, don't some function start in cold section and then switch into hot
>> section?
>
> I am not able to generate a test where this happens. However, I  fixed
> this problem by generating the cold label only when the first function
> block is not cold.
>
> Patch attached, please see if this is ok.
>
> Thanks
> Sri
>
>>
>>         Jakub
Sriraman Tallam - May 7, 2013, 9:41 p.m.
Ping.

On Thu, Apr 25, 2013 at 4:50 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> Attaching an updated patch.
>
> Thanks
> Sri
>
> On Thu, Apr 25, 2013 at 4:42 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>> On Tue, Apr 23, 2013 at 9:59 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> On Tue, Apr 23, 2013 at 03:58:06PM -0700, Sriraman Tallam wrote:
>>>>   This patch generates labels for cold function parts that are split when
>>>> using the option -freorder-blocks-and-partition.  The cold label name
>>>> is generated by suffixing ".cold" to the assembler name of the hot
>>>> function.
>>>>
>>>>   This is useful when getting back traces from gdb when the cold function
>>>> part does get executed.
>>>>
>>>>         * final.c (final_scan_insn): Generate cold label name by suffixing
>>>>         ".cold" to function's assembler name.
>>>>         * gcc.dg/tree-prof/cold_partition_label.c: New test.
>>>
>>> This doesn't honor NO_DOT_IN_LABEL (and NO_DOLLAR_IN_LABEL).
>>
>> Fixed, by calling clean_symbol_name
>>
>>> Also, don't some function start in cold section and then switch into hot
>>> section?
>>
>> I am not able to generate a test where this happens. However, I  fixed
>> this problem by generating the cold label only when the first function
>> block is not cold.
>>
>> Patch attached, please see if this is ok.
>>
>> Thanks
>> Sri
>>
>>>
>>>         Jakub
Sriraman Tallam - May 9, 2013, 9:22 p.m.
cc:Diego

On Tue, May 7, 2013 at 2:41 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> Ping.
>
> On Thu, Apr 25, 2013 at 4:50 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>> Attaching an updated patch.
>>
>> Thanks
>> Sri
>>
>> On Thu, Apr 25, 2013 at 4:42 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>> On Tue, Apr 23, 2013 at 9:59 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>>> On Tue, Apr 23, 2013 at 03:58:06PM -0700, Sriraman Tallam wrote:
>>>>>   This patch generates labels for cold function parts that are split when
>>>>> using the option -freorder-blocks-and-partition.  The cold label name
>>>>> is generated by suffixing ".cold" to the assembler name of the hot
>>>>> function.
>>>>>
>>>>>   This is useful when getting back traces from gdb when the cold function
>>>>> part does get executed.
>>>>>
>>>>>         * final.c (final_scan_insn): Generate cold label name by suffixing
>>>>>         ".cold" to function's assembler name.
>>>>>         * gcc.dg/tree-prof/cold_partition_label.c: New test.
>>>>
>>>> This doesn't honor NO_DOT_IN_LABEL (and NO_DOLLAR_IN_LABEL).
>>>
>>> Fixed, by calling clean_symbol_name
>>>
>>>> Also, don't some function start in cold section and then switch into hot
>>>> section?
>>>
>>> I am not able to generate a test where this happens. However, I  fixed
>>> this problem by generating the cold label only when the first function
>>> block is not cold.
>>>
>>> Patch attached, please see if this is ok.
>>>
>>> Thanks
>>> Sri
>>>
>>>>
>>>>         Jakub
Sriraman Tallam - May 13, 2013, 9:58 p.m.
Ping.

On Thu, May 9, 2013 at 2:22 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> cc:Diego
>
> On Tue, May 7, 2013 at 2:41 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>> Ping.
>>
>> On Thu, Apr 25, 2013 at 4:50 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>> Attaching an updated patch.
>>>
>>> Thanks
>>> Sri
>>>
>>> On Thu, Apr 25, 2013 at 4:42 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>> On Tue, Apr 23, 2013 at 9:59 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>>>> On Tue, Apr 23, 2013 at 03:58:06PM -0700, Sriraman Tallam wrote:
>>>>>>   This patch generates labels for cold function parts that are split when
>>>>>> using the option -freorder-blocks-and-partition.  The cold label name
>>>>>> is generated by suffixing ".cold" to the assembler name of the hot
>>>>>> function.
>>>>>>
>>>>>>   This is useful when getting back traces from gdb when the cold function
>>>>>> part does get executed.
>>>>>>
>>>>>>         * final.c (final_scan_insn): Generate cold label name by suffixing
>>>>>>         ".cold" to function's assembler name.
>>>>>>         * gcc.dg/tree-prof/cold_partition_label.c: New test.
>>>>>
>>>>> This doesn't honor NO_DOT_IN_LABEL (and NO_DOLLAR_IN_LABEL).
>>>>
>>>> Fixed, by calling clean_symbol_name
>>>>
>>>>> Also, don't some function start in cold section and then switch into hot
>>>>> section?
>>>>
>>>> I am not able to generate a test where this happens. However, I  fixed
>>>> this problem by generating the cold label only when the first function
>>>> block is not cold.
>>>>
>>>> Patch attached, please see if this is ok.
>>>>
>>>> Thanks
>>>> Sri
>>>>
>>>>>
>>>>>         Jakub
Steven Bosscher - May 14, 2013, 4:57 a.m.
Hi,

I would prefer to encode section changes within a function in the
debug info. I will explain later today.

Ciao!
Steven



On 5/13/13, Sriraman Tallam <tmsriram@google.com> wrote:
> Ping.
>
> On Thu, May 9, 2013 at 2:22 PM, Sriraman Tallam <tmsriram@google.com>
> wrote:
>> cc:Diego
>>
>> On Tue, May 7, 2013 at 2:41 PM, Sriraman Tallam <tmsriram@google.com>
>> wrote:
>>> Ping.
>>>
>>> On Thu, Apr 25, 2013 at 4:50 PM, Sriraman Tallam <tmsriram@google.com>
>>> wrote:
>>>> Attaching an updated patch.
>>>>
>>>> Thanks
>>>> Sri
>>>>
>>>> On Thu, Apr 25, 2013 at 4:42 PM, Sriraman Tallam <tmsriram@google.com>
>>>> wrote:
>>>>> On Tue, Apr 23, 2013 at 9:59 PM, Jakub Jelinek <jakub@redhat.com>
>>>>> wrote:
>>>>>> On Tue, Apr 23, 2013 at 03:58:06PM -0700, Sriraman Tallam wrote:
>>>>>>>   This patch generates labels for cold function parts that are split
>>>>>>> when
>>>>>>> using the option -freorder-blocks-and-partition.  The cold label
>>>>>>> name
>>>>>>> is generated by suffixing ".cold" to the assembler name of the hot
>>>>>>> function.
>>>>>>>
>>>>>>>   This is useful when getting back traces from gdb when the cold
>>>>>>> function
>>>>>>> part does get executed.
>>>>>>>
>>>>>>>         * final.c (final_scan_insn): Generate cold label name by
>>>>>>> suffixing
>>>>>>>         ".cold" to function's assembler name.
>>>>>>>         * gcc.dg/tree-prof/cold_partition_label.c: New test.
>>>>>>
>>>>>> This doesn't honor NO_DOT_IN_LABEL (and NO_DOLLAR_IN_LABEL).
>>>>>
>>>>> Fixed, by calling clean_symbol_name
>>>>>
>>>>>> Also, don't some function start in cold section and then switch into
>>>>>> hot
>>>>>> section?
>>>>>
>>>>> I am not able to generate a test where this happens. However, I  fixed
>>>>> this problem by generating the cold label only when the first function
>>>>> block is not cold.
>>>>>
>>>>> Patch attached, please see if this is ok.
>>>>>
>>>>> Thanks
>>>>> Sri
>>>>>
>>>>>>
>>>>>>         Jakub
>

Patch

Index: final.c
===================================================================
--- final.c	(revision 198212)
+++ final.c	(working copy)
@@ -2101,6 +2101,25 @@  final_scan_insn (rtx insn, FILE *file, int optimiz
 	  targetm.asm_out.function_switched_text_sections (asm_out_file,
 							   current_function_decl,
 							   in_cold_section_p);
+	  /* Emit a label for the split cold section.  Form label name by
+	     suffixing ".cold" to the function's assembler name.  Do not do
+	     this when the first function block of the split function is
+	     cold.  */
+	  if (in_cold_section_p
+	      && !first_function_block_is_cold)
+	    {
+	      char *cold_function_name;
+	      const char *mangled_function_name;
+	      tree asm_name = DECL_ASSEMBLER_NAME (current_function_decl);
+
+	      mangled_function_name = IDENTIFIER_POINTER (asm_name);
+	      cold_function_name = XNEWVEC (char,
+	          strlen (mangled_function_name) + strlen (".cold") + 1);
+	      sprintf (cold_function_name, "%s.cold", mangled_function_name);
+	      clean_symbol_name (cold_function_name);
+	      ASM_OUTPUT_LABEL (asm_out_file, cold_function_name);
+	      XDELETEVEC (cold_function_name);
+	    }
 	  break;
 
 	case NOTE_INSN_BASIC_BLOCK:
Index: testsuite/gcc.dg/tree-prof/cold_partition_label.c
===================================================================
--- testsuite/gcc.dg/tree-prof/cold_partition_label.c	(revision 0)
+++ testsuite/gcc.dg/tree-prof/cold_partition_label.c	(revision 0)
@@ -0,0 +1,39 @@ 
+/* Test case to check if function foo gets split and the cold function
+   gets a label.  */
+/* { dg-require-effective-target freorder } */
+/* { dg-options "-O2 -freorder-blocks-and-partition --save-temps" } */
+
+#define SIZE 10000
+
+const char *sarr[SIZE];
+const char *buf_hot;
+const char *buf_cold;
+
+__attribute__((noinline))
+void 
+foo (int path)
+{
+  int i;
+  if (path)
+    {
+      for (i = 0; i < SIZE; i++)
+	sarr[i] = buf_hot;
+    }
+  else
+    {
+      for (i = 0; i < SIZE; i++)
+	sarr[i] = buf_cold;
+    }
+}
+
+int
+main (int argc, char *argv[])
+{
+  buf_hot =  "hello";
+  buf_cold = "world";
+  foo (argc);
+  return 0;
+}
+
+/* { dg-final-use { scan-assembler "foo\.cold" } } */
+/* { dg-final-use { cleanup-saved-temps } } */