This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.
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);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]