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

[Caroline Tice <cmtice@google.com>]

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

The attached patch can revert the previous patch, if that is the way
we should proceed on this.  If you want me to apply the reversion,
please let me know.

I would be happy to fix to the problem, rather than just reverting the
patch, but I do not have expertise in assembly language on other
platforms, so I would need some help, if anyone would be interested in
helping me?

-- Caroline Tice
cmtice@google.com

ChangeLog (gcc):

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

        Revert patch from 2015-04-27
        * final.c (final_scan_insn): Remove code to output
        cold_function_name as a function type in assembly.
        * varasm.c (cold_function_name): Remove global declaration.
        (assemble_start_function): Remove code to re-set
        cold_function_name.
        (assemble_end_function):  Do not output side of cold
        partition.

ChangLog (gcc/testsuite):

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

        Revert patch from 2015-04-27
        * gcc.dg/tree-prof/cold_partition_label.c: Do not check
        for cold partition size.




On Wed, Apr 29, 2015 at 9:33 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
>> 2015-03-27  Caroline Tice  <cmtice@google.com>
>>
>>         * final.c (final_scan_insn): Change 'cold_function_name' to
>>         'cold_partition_name' and make it a global variable; also output
>>         assembly to give it a 'FUNC' type, if appropriate.
>>         * varasm.c (cold_partition_name): Declare and initialize global
>>         variable.
>>         (assemble_start_function): Re-set value for cold_partition_name.
>>         (assemble_end_function): Output assembly to calculate size of cold
>>         partition, and associate size with name, if appropriate.
>>         * varash.h (cold_partition_name): Add extern declaration for global
>>         variable.
>>         * testsuite/gcc.dg/tree-prof/cold_partition_label.c: Add dg-final-use
>>         to scan assembly for cold partition name and size.
>
> This patch caused PR 65929 [1].
>
> Targets are (ab)using ASM_DECLARE_FUNCTION_NAME and
> ASM_DECLARE_FUNCTION_SIZE for more things that their name suggests. As
> shown in the PR, some directives that are generated through these
> macros apply only to real functions, not to parts of function in the
> middle of the function.
>
> In the particular case in the PR, assembler doesn't tolerate nested
> function declaration.
>
> I suggest to revert the patch for now, until side effects of the patch
> are resolved.
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65929
>
> Uros.

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

Index: gcc/final.c
===================================================================
--- gcc/final.c	(revision 222584)
+++ gcc/final.c	(working copy)
@@ -2233,16 +2233,10 @@
 	     suffixing "cold" to the original function's name.  */
 	  if (in_cold_section_p)
 	    {
-	      cold_function_name
+	      tree 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);
-#else
 	      ASM_OUTPUT_LABEL (asm_out_file,
 				IDENTIFIER_POINTER (cold_function_name));
-#endif
 	    }
 	  break;
 
Index: gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c	(revision 222584)
+++ gcc/testsuite/gcc.dg/tree-prof/cold_partition_label.c	(working copy)
@@ -35,6 +35,4 @@
   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 { cleanup-saved-temps } } */
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	(revision 222584)
+++ gcc/varasm.c	(working copy)
@@ -187,13 +187,6 @@
    at the cold section.  */
 bool in_cold_section_p;
 
-/* The following global holds the "function name" for the code in the
-   cold section of a function, if hot/cold function splitting is enabled
-   and there was actually code that went into the cold section.  A
-   pseudo function name is needed for the cold section of code for some
-   debugging tools that perform symbolization. */
-tree cold_function_name = NULL_TREE;
-
 /* A linked list of all the unnamed sections.  */
 static GTY(()) section *unnamed_sections;
 
@@ -1726,7 +1719,6 @@
       ASM_GENERATE_INTERNAL_LABEL (tmp_label, "LCOLDE", const_labelno);
       crtl->subsections.cold_section_end_label = ggc_strdup (tmp_label);
       const_labelno++;
-      cold_function_name = NULL_TREE;
     }
   else
     {
@@ -1864,12 +1856,6 @@
 
       save_text_section = in_section;
       switch_to_section (unlikely_text_section ());
-#ifdef ASM_DECLARE_FUNCTION_SIZE
-      if (cold_function_name != NULL_TREE)
-	ASM_DECLARE_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)
 	switch_to_section (text_section);
Index: gcc/varasm.h
===================================================================
--- gcc/varasm.h	(revision 222584)
+++ gcc/varasm.h	(working copy)
@@ -20,13 +20,6 @@
 #ifndef GCC_VARASM_H
 #define GCC_VARASM_H
 
-/* The following global holds the "function name" for the code in the
-   cold section of a function, if hot/cold function splitting is enabled
-   and there was actually code that went into the cold section.  A
-   pseudo function name is needed for the cold section of code for some
-   debugging tools that perform symbolization. */
-extern tree cold_function_name;
-
 extern tree tree_output_constant_def (tree);
 extern void make_decl_rtl (tree);
 extern rtx make_decl_rtl_for_debug (tree);