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
cmt...@google.com
ChangeLog (gcc):
2015-04-29 Caroline Tice <cmt...@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 <cmt...@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:59 AM, Caroline Tice <cmt...@google.com> wrote:
> Thank you; I will work with your suggestions and try to get a new
> patch done soon.
>
> -- Caroline Tice
> cmt...@google.com
>
>
> On Wed, Apr 29, 2015 at 11:34 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
>> On Wed, Apr 29, 2015 at 7:47 PM, Uros Bizjak <ubiz...@gmail.com> wrote:
>>> On Wed, Apr 29, 2015 at 7:38 PM, Caroline Tice <cmt...@google.com> wrote:
>>>> 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?
>>>
>>> How about adding ASM_DECLARE_COLD_FUNCTION_NAME and
>>> ASM_DECLARE_COLD_FUNCTION_SIZE? If these are defined, they can be used
>>> instead, and targets are free to define them in any way.
>>
>> Something like the attached prototype RFC patch. Using this patch,
>> readelf -sW returns:
>>
>> Symbol table '.symtab' contains 18 entries:
>> Num: Value Size Type Bind Vis Ndx Name
>> 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
>> 1: 0000000000000000 0 SECTION LOCAL DEFAULT 1
>> 2: 0000000000000000 0 SECTION LOCAL DEFAULT 3
>> 3: 0000000000000000 0 SECTION LOCAL DEFAULT 4
>> 4: 0000000000000000 0 SECTION LOCAL DEFAULT 5
>> 5: 0000000000000000 0 SECTION LOCAL DEFAULT 6
>> 6: 0000000000000000 0 SECTION LOCAL DEFAULT 8
>> 7: 0000000000000000 8 FUNC LOCAL DEFAULT 6 main.cold.0
>> 8: 0000000000000000 0 SECTION LOCAL DEFAULT 10
>> 9: 0000000000000000 0 SECTION LOCAL DEFAULT 13
>> 10: 0000000000000000 0 SECTION LOCAL DEFAULT 12
>> 11: 0000000000000000 312 FUNC GLOBAL DEFAULT [<other>: 88] 8
>> main
>> 12: 0000000000000008 160 OBJECT GLOBAL DEFAULT COM buf
>> 13: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND memset
>> 14: 0000000000000000 44 FUNC GLOBAL DEFAULT [<other>: 88] 1
>> sub2
>> 15: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND strcmp
>> 16: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND exit
>> 17: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND abort
>>
>> Uros.
Index: gcc/config/elfos.h
===================================================================
--- gcc/config/elfos.h (revision 222584)
+++ 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/final.c
===================================================================
--- gcc/final.c (revision 222587)
+++ 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 222588)
+++ 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 222587)
+++ 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)
