Re: [PATCH] Fix size & type for cold partition names (hot-cold function partitioning)

[Caroline Tice <cmtice@google.com>]

[-- Attachment #1: Type: text/plain, Size: 2636 bytes --]

Done.  Here is the updated patch (with ChangeLog entries).  Only
change was to update tm.texi.in.

The bootstrap passed.  Is the patch ok to commit?

-- Caroline
cmtice@google.com

ChangeLog (gcc):

2015-04-30  Caroline Tice  <cmtice@google.com>

 PR 65929
* config/elfos.h (ASM_DECLARE_COLD_FUNCTION_NAME): New macro definition.
(ASM_DECLARE_COLD_FUNCTION_SIZE): New macro definition.
* doc/tm.texi.in (ASM_DECLARE_COLD_FUNCTION_NAME): Document new macro.
(ASM_DECLARE_COLD_FUNCTION_SIZE): Document new macro.
* final.c (final_scan_insn):  Use ASM_DECLARE_COLD_FUNCTION_NAME
instead of ASM_DECLARE_FUNCTION_NAME for cold partition name.
* varasm.c (assemble_end_function):  Use ASM_DECLARE_COLD_FUNCTION_SIZE
instead of ASM_DECLARE_FUNCTION_SIZE for cold partition size.

ChangeLog (gcc/testsuite):

2015-04-30  Caroline Tice  <cmtice@google.com>

PR  65929
* gcc.dg/tree-prof/cold_partition_label.c:  Only check for cold
partition size on certain targets.



On Wed, Apr 29, 2015 at 11:12 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Apr 29, 2015 at 11:22 PM, Caroline Tice <cmtice@google.com> wrote:
>> Here is a new patch to update the cold name partition so that it will
>> only be treated like a function name and be given a size on the
>> architectures that specifically define macros for such.
>>
>> I also updated the test case to try to only test on the appropriate
>> architectures.  I am not sure I got the target triples correct for
>> this, so I would appreciate some extra attention to that in the
>> review.  I have tested this new patch on my workstation and it works
>> as intended.  I am in the process of bootstrapping with the new patch.
>> Assuming that the bootstrap passes, is this ok to commit?
>>
>> -- Caroline Tice
>> cmtice@google.com
>>
>> ChangeLog (gcc):
>>
>> 2015-04-29  Caroline Tice  <cmtice@google.com>
>>
>>         PR 65929
>>         * config/elfos.h (ASM_DECLARE_COLD_FUNCTION_NAME): New macro definition.
>>         (ASM_DECLARE_COLD_FUNCTION_SIZE): New macro definition.
>>         * final.c (final_scan_insn):  Use ASM_DECLARE_COLD_FUNCTION_NAME
>>         instead of ASM_DECLARE_FUNCTION_NAME for cold partition name.
>>         * varasm.c (assemble_end_function):  Use ASM_DECLARE_COLD_FUNCTION_SIZE
>>         instead of ASM_DECLARE_FUNCTION_SIZE for cold partition size.
>>
>> ChangeLog (testsuite):
>>
>> 2015-04-29  Caroline Tice  <cmtice@google.com>
>>
>>        PR  65929
>>         * gcc.dg/tree-prof/cold_partition_label.c:  Only check for cold
>>         partition size on certain targets.
>
> Documentation for new macros is missing (please see doc/tm.texi.in).
>
> Uros.

[-- Attachment #2: new-cold-partition-name.patch --]
[-- Type: text/x-patch, Size: 5423 bytes --]

Index: gcc/config/elfos.h
===================================================================
--- gcc/config/elfos.h	(revision 222635)
+++ gcc/config/elfos.h	(working copy)
@@ -284,6 +284,22 @@
   while (0)
 #endif
 
+/* Write the extra assembler code needed to declare the name of a
+   cold function partition properly. Some svr4 assemblers need to also
+   have something extra said about the function's return value.  We
+   allow for that here.  */
+
+#ifndef ASM_DECLARE_COLD_FUNCTION_NAME
+#define ASM_DECLARE_COLD_FUNCTION_NAME(FILE, NAME, DECL)	\
+  do								\
+    {								\
+      ASM_OUTPUT_TYPE_DIRECTIVE (FILE, NAME, "function");	\
+      ASM_DECLARE_RESULT (FILE, DECL_RESULT (DECL));		\
+      ASM_OUTPUT_FUNCTION_LABEL (FILE, NAME, DECL);		\
+    }								\
+  while (0)
+#endif
+
 /* Write the extra assembler code needed to declare an object properly.  */
 
 #ifdef HAVE_GAS_GNU_UNIQUE_OBJECT
@@ -358,6 +374,17 @@
   while (0)
 #endif
 
+/* This is how to declare the size of a cold function partition.  */
+#ifndef ASM_DECLARE_COLD_FUNCTION_SIZE
+#define ASM_DECLARE_COLD_FUNCTION_SIZE(FILE, FNAME, DECL)	\
+  do								\
+    {								\
+      if (!flag_inhibit_size_directive)				\
+	ASM_OUTPUT_MEASURED_SIZE (FILE, FNAME);			\
+    }								\
+  while (0)
+#endif
+
 /* A table of bytes codes used by the ASM_OUTPUT_ASCII and
    ASM_OUTPUT_LIMITED_STRING macros.  Each byte in the table
    corresponds to a particular byte value [0..255].  For any
Index: gcc/doc/tm.texi.in
===================================================================
--- gcc/doc/tm.texi.in	(revision 222635)
+++ gcc/doc/tm.texi.in	(working copy)
@@ -5574,6 +5574,34 @@
 of this macro.
 @end defmac
 
+@defmac ASM_DECLARE_COLD_FUNCTION_NAME (@var{stream}, @var{name}, @var{decl})
+A C statement (sans semicolon) to output to the stdio stream
+@var{stream} any text necessary for declaring the name @var{name} of a
+cold function partition which is being defined.  This macro is responsible
+for outputting the label definition (perhaps using
+@code{ASM_OUTPUT_FUNCTION_LABEL}).  The argument @var{decl} is the
+@code{FUNCTION_DECL} tree node representing the function.
+
+If this macro is not defined, then the cold partition name is defined in the
+usual manner as a label (by means of @code{ASM_OUTPUT_LABEL}).
+
+You may wish to use @code{ASM_OUTPUT_TYPE_DIRECTIVE} in the definition
+of this macro.
+@end defmac
+
+@defmac ASM_DECLARE_COLD_FUNCTION_SIZE (@var{stream}, @var{name}, @var{decl})
+A C statement (sans semicolon) to output to the stdio stream
+@var{stream} any text necessary for declaring the size of a cold function
+partition which is being defined.  The argument @var{name} is the name of the
+cold partition of the function.  The argument @var{decl} is the
+@code{FUNCTION_DECL} tree node representing the function.
+
+If this macro is not defined, then the partition size is not defined.
+
+You may wish to use @code{ASM_OUTPUT_MEASURED_SIZE} in the definition
+of this macro.
+@end defmac
+
 @defmac ASM_DECLARE_OBJECT_NAME (@var{stream}, @var{name}, @var{decl})
 A C statement (sans semicolon) to output to the stdio stream
 @var{stream} any text necessary for declaring the name @var{name} of an
Index: gcc/final.c
===================================================================
--- gcc/final.c	(revision 222635)
+++ gcc/final.c	(working copy)
@@ -2235,10 +2235,11 @@
 	    {
 	      cold_function_name
 		= clone_function_name (current_function_decl, "cold");
-#ifdef ASM_DECLARE_FUNCTION_NAME
-	      ASM_DECLARE_FUNCTION_NAME (asm_out_file,
-					 IDENTIFIER_POINTER (cold_function_name),
-					 current_function_decl);
+#ifdef ASM_DECLARE_COLD_FUNCTION_NAME
+	      ASM_DECLARE_COLD_FUNCTION_NAME (asm_out_file,
+					      IDENTIFIER_POINTER
+					          (cold_function_name),
+					      current_function_decl);
 #else
 	      ASM_OUTPUT_LABEL (asm_out_file,
 				IDENTIFIER_POINTER (cold_function_name));
Index: gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c	(revision 222635)
+++ gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c	(working copy)
@@ -35,6 +35,6 @@
   return 0;
 }
 
-/* { dg-final-use { scan-assembler "foo\[._\]+cold\[\._\]+0" } } */
-/* { dg-final-use { scan-assembler "size\[ \ta-zA-Z0-0\]+foo\[._\]+cold\[\._\]+0" } } */
+/* { dg-final-use { scan-assembler "foo\[._\]+cold\[\._\]+0" { target *-*-linux* *-*-gnu* } } } */
+/* { dg-final-use { scan-assembler "size\[ \ta-zA-Z0-0\]+foo\[._\]+cold\[\._\]+0" { target *-*-linux* *-*-gnu* } } } */
 /* { dg-final-use { cleanup-saved-temps } } */
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	(revision 222635)
+++ gcc/varasm.c	(working copy)
@@ -1864,11 +1864,11 @@
 
       save_text_section = in_section;
       switch_to_section (unlikely_text_section ());
-#ifdef ASM_DECLARE_FUNCTION_SIZE
+#ifdef ASM_DECLARE_COLD_FUNCTION_SIZE
       if (cold_function_name != NULL_TREE)
-	ASM_DECLARE_FUNCTION_SIZE (asm_out_file,
-				   IDENTIFIER_POINTER (cold_function_name),
-				   decl);
+	ASM_DECLARE_COLD_FUNCTION_SIZE (asm_out_file,
+					IDENTIFIER_POINTER (cold_function_name),
+					decl);
 #endif
       ASM_OUTPUT_LABEL (asm_out_file, crtl->subsections.cold_section_end_label);
       if (first_function_block_is_cold)