diff mbox

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

Message ID CAAs8HmyEqR4Q0hgqUY-iGjKpuK+VSRnZhrJC0rdehTHm8xvcOw@mail.gmail.com
State New
Headers show

Commit Message

Sriraman Tallam Nov. 18, 2013, 8:01 p.m. UTC 
On Wed, Nov 13, 2013 at 10:00 AM, Sriraman Tallam <tmsriram@google.com> wrote:
> On Tue, Nov 12, 2013 at 10:10 AM, Cary Coutant <ccoutant@google.com> wrote:
>>>>> Is there a format for compiler-defined labels that would not be able
>>>>> to clash with other user-generated labels?
>>>>
>>>> My understanding is that the "." in the generated name ensures that it
>>>> will not clash with user-generated labels which cannot contain ".". So
>>>> this should not be an issue.
>>>
>>> Is that true for all languages? I don't think we can make that
>>> assumption. Also, there may be clashes with other compiler generated
>>> symbols. I know of at least one Fortran compiler that composes names
>>> for module symbols as "modulename.symbolname", and if symbolname is
>>> "cold" then you can have a clash.
>>
>> GCC already uses "." to generate names for cloned functions, and picks
>> a different separator for targets where "." is legal (see
>> clone_function_name in cgraphclones.c). It would probably be a good
>> idea to use clone_function_name to generate the new name (in
>> particular, because the C++ demangler already knows how to handle that
>> style when it's applied to mangled names).
>
> I have modified my original patch to use clone_function_name to
> generate the cold part label and attached it.

Is this patch fine for commit?

Thanks
Sri


>
> Thanks
> Sri
>
>
>>
>> -cary
Patch to generate 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 in a demangler friendly way.

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 name.
	* gcc.dg/tree-prof/cold_partition_label.c: New test.

Comments

Steven Bosscher Nov. 18, 2013, 8:16 p.m. UTC | #1 
On Mon, Nov 18, 2013 at 9:01 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>> I have modified my original patch to use clone_function_name to
>> generate the cold part label and attached it.
>
> Is this patch fine for commit?


Yup, OK. Seems there's no better way :-)

Ciao!
Steven
Sriraman Tallam Nov. 18, 2013, 8:34 p.m. UTC | #2 
On Mon, Nov 18, 2013 at 12:16 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Mon, Nov 18, 2013 at 9:01 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>> I have modified my original patch to use clone_function_name to
>>> generate the cold part label and attached it.
>>
>> Is this patch fine for commit?
>
>
> Yup, OK. Seems there's no better way :-)

Thanks! Do I need any other maintainer's approval or can I go ahead
and commit, after checking for test regressions and boot-strap parity?

Sri


>
> Ciao!
> Steven
Eric Botcazou Nov. 19, 2013, 9:03 a.m. UTC | #3 
> Thanks! Do I need any other maintainer's approval or can I go ahead
> and commit, after checking for test regressions and boot-strap parity?

Steven's approval is sufficient here.
diff mbox

Patch

Index: final.c
===================================================================
--- final.c	(revision 204712)
+++ final.c	(working copy)
@@ -2167,6 +2167,15 @@  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 original function's name.  */
+	  if (in_cold_section_p)
+	    {
+	      tree cold_function_name
+		= clone_function_name (current_function_decl, "cold");
+	      ASM_OUTPUT_LABEL (asm_out_file,
+				IDENTIFIER_POINTER (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 } } */