* Problem with ARM longcalls
@ 2011-03-23 15:47 Bernd Schmidt
2011-03-24 14:24 ` Richard Earnshaw
2011-04-14 12:43 ` Bernd Schmidt
0 siblings, 2 replies; 6+ messages in thread
From: Bernd Schmidt @ 2011-03-23 15:47 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 848 bytes --]
I've discovered a problem with -mlong-calls on ARM. The bug was first
reported against a new target, but I'd copied the relevant code from the
ARM backend.
We use current_function_section in arm_is_long_call_p to decide whether
we're calling something that goes into the same section. The problem
with this is that current_function_section can only be used during
final, since it relies on the global variable in_cold_section_p which is
set up only in assemble_start_function. On ARM, this problem manifests
as short-calls when a long-call would be required; in the other port it
was an "insn doesn't satisfy its constraints" error.
The following patch is against 4.5, since the problem appears hidden in
mainline (the initialization of first_function_block_is_cold has
changed). Ok for trunk and branches after arm-linux tests complete?
Bernd
[-- Attachment #2: dfs.diff --]
[-- Type: text/plain, Size: 5111 bytes --]
* function.c (init_function_start): Call decide_function_section.
* varasm.c (decide_function_section): New function.
(assemble_start_function): When not using
flag_reorder_blocks_and_partition, don't compute in_cold_section_p
or first_function_block_is_cold.
* rtl.h (decide_function_section): Declare.
* gcc.target/arm/cold-lc.c: New test.
Index: gcc/testsuite/gcc.target/arm/cold-lc.c
===================================================================
--- gcc/testsuite/gcc.target/arm/cold-lc.c (revision 0)
+++ gcc/testsuite/gcc.target/arm/cold-lc.c (revision 0)
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mlong-calls" } */
+/* { dg-final { scan-assembler-not "bl\[^\n\]*dump_stack" } } */
+
+extern void dump_stack (void) __attribute__ ((__cold__)) __attribute__ ((noinline));
+struct thread_info {
+ struct task_struct *task;
+};
+extern struct thread_info *current_thread_info (void);
+
+void dump_stack (void)
+{
+ unsigned long stack;
+ show_stack ((current_thread_info ()->task), &stack);
+}
+
+void die (char *str, void *fp, int nr)
+{
+ dump_stack ();
+ while (1);
+}
+
Index: gcc/function.c
===================================================================
--- gcc/function.c (revision 170052)
+++ gcc/function.c (working copy)
@@ -4227,6 +4227,7 @@ init_function_start (tree subr)
else
allocate_struct_function (subr, false);
prepare_function_start ();
+ decide_function_section (subr);
/* Warn if this value is an aggregate type,
regardless of which calling convention we are using for it. */
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c (revision 170052)
+++ gcc/varasm.c (working copy)
@@ -1687,6 +1687,38 @@ notice_global_symbol (tree decl)
}
}
+/* If not using flag_reorder_blocks_and_partition, decide early whether the
+ current function goes into the cold section, so that targets can use
+ current_function_section during RTL expansion. DECL describes the
+ function. */
+
+void
+decide_function_section (tree decl)
+{
+ first_function_block_is_cold = false;
+
+ if (flag_reorder_blocks_and_partition)
+ /* We will decide in assemble_start_function. */
+ return;
+
+ if (DECL_SECTION_NAME (decl))
+ {
+ /* Calls to function_section rely on first_function_block_is_cold
+ being accurate. The first block may be cold even if we aren't
+ doing partitioning, if the entire function was decided by
+ choose_function_section (predict.c) to be cold. */
+
+ initialize_cold_section_name ();
+
+ if (crtl->subsections.unlikely_text_section_name
+ && strcmp (TREE_STRING_POINTER (DECL_SECTION_NAME (decl)),
+ crtl->subsections.unlikely_text_section_name) == 0)
+ first_function_block_is_cold = true;
+ }
+
+ in_cold_section_p = first_function_block_is_cold;
+}
+
/* Output assembler code for the constant pool of a function and associated
with defining the name of the function. DECL describes the function.
NAME is the function's name. For the constant pool, we use the current
@@ -1701,7 +1733,6 @@ assemble_start_function (tree decl, cons
crtl->subsections.unlikely_text_section_name = NULL;
- first_function_block_is_cold = false;
if (flag_reorder_blocks_and_partition)
{
ASM_GENERATE_INTERNAL_LABEL (tmp_label, "LHOTB", const_labelno);
@@ -1738,6 +1769,8 @@ assemble_start_function (tree decl, cons
if (flag_reorder_blocks_and_partition)
{
+ first_function_block_is_cold = false;
+
switch_to_section (unlikely_text_section ());
assemble_align (DECL_ALIGN (decl));
ASM_OUTPUT_LABEL (asm_out_file, crtl->subsections.cold_section_label);
@@ -1754,23 +1787,8 @@ assemble_start_function (tree decl, cons
hot_label_written = true;
first_function_block_is_cold = true;
}
+ in_cold_section_p = first_function_block_is_cold;
}
- else if (DECL_SECTION_NAME (decl))
- {
- /* Calls to function_section rely on first_function_block_is_cold
- being accurate. The first block may be cold even if we aren't
- doing partitioning, if the entire function was decided by
- choose_function_section (predict.c) to be cold. */
-
- initialize_cold_section_name ();
-
- if (crtl->subsections.unlikely_text_section_name
- && strcmp (TREE_STRING_POINTER (DECL_SECTION_NAME (decl)),
- crtl->subsections.unlikely_text_section_name) == 0)
- first_function_block_is_cold = true;
- }
-
- in_cold_section_p = first_function_block_is_cold;
/* Switch to the correct text section for the start of the function. */
Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h (revision 170052)
+++ gcc/rtl.h (working copy)
@@ -1644,6 +1644,7 @@ extern rtx get_pool_constant (rtx);
extern rtx get_pool_constant_mark (rtx, bool *);
extern enum machine_mode get_pool_mode (const_rtx);
extern rtx simplify_subtraction (rtx);
+extern void decide_function_section (tree);
/* In function.c */
extern rtx assign_stack_local (enum machine_mode, HOST_WIDE_INT, int);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem with ARM longcalls
2011-03-23 15:47 Problem with ARM longcalls Bernd Schmidt
@ 2011-03-24 14:24 ` Richard Earnshaw
2011-03-24 14:28 ` Bernd Schmidt
2011-04-14 12:43 ` Bernd Schmidt
1 sibling, 1 reply; 6+ messages in thread
From: Richard Earnshaw @ 2011-03-24 14:24 UTC (permalink / raw)
To: Bernd Schmidt; +Cc: GCC Patches
On Wed, 2011-03-23 at 16:46 +0100, Bernd Schmidt wrote:
> I've discovered a problem with -mlong-calls on ARM. The bug was first
> reported against a new target, but I'd copied the relevant code from the
> ARM backend.
>
> We use current_function_section in arm_is_long_call_p to decide whether
> we're calling something that goes into the same section. The problem
> with this is that current_function_section can only be used during
> final, since it relies on the global variable in_cold_section_p which is
> set up only in assemble_start_function. On ARM, this problem manifests
> as short-calls when a long-call would be required; in the other port it
> was an "insn doesn't satisfy its constraints" error.
>
> The following patch is against 4.5, since the problem appears hidden in
> mainline (the initialization of first_function_block_is_cold has
> changed). Ok for trunk and branches after arm-linux tests complete?
>
>
> Bernd
The ARM port currently doesn't support hot/cold partitioning of code
(and can't until the constant pool code is rewritten to deal with it),
so how is this a problem?
R.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem with ARM longcalls
2011-03-24 14:24 ` Richard Earnshaw
@ 2011-03-24 14:28 ` Bernd Schmidt
0 siblings, 0 replies; 6+ messages in thread
From: Bernd Schmidt @ 2011-03-24 14:28 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: GCC Patches
On 03/24/2011 03:24 PM, Richard Earnshaw wrote:
>
> On Wed, 2011-03-23 at 16:46 +0100, Bernd Schmidt wrote:
>> I've discovered a problem with -mlong-calls on ARM. The bug was first
>> reported against a new target, but I'd copied the relevant code from the
>> ARM backend.
>>
>> We use current_function_section in arm_is_long_call_p to decide whether
>> we're calling something that goes into the same section. The problem
>> with this is that current_function_section can only be used during
>> final, since it relies on the global variable in_cold_section_p which is
>> set up only in assemble_start_function. On ARM, this problem manifests
>> as short-calls when a long-call would be required; in the other port it
>> was an "insn doesn't satisfy its constraints" error.
>>
>> The following patch is against 4.5, since the problem appears hidden in
>> mainline (the initialization of first_function_block_is_cold has
>> changed). Ok for trunk and branches after arm-linux tests complete?
>>
>>
>> Bernd
>
> The ARM port currently doesn't support hot/cold partitioning of code
> (and can't until the constant pool code is rewritten to deal with it),
> so how is this a problem?
Different functions can still go into different sections. When we start
expanding a function, in_cold_section_p still contains the value that
was correct for the previous one. It will change at the start of final,
which means that current_function_section can return different values
while compiling a function. See the included testcase.
Bernd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem with ARM longcalls
2011-03-23 15:47 Problem with ARM longcalls Bernd Schmidt
2011-03-24 14:24 ` Richard Earnshaw
@ 2011-04-14 12:43 ` Bernd Schmidt
2011-04-14 13:00 ` Richard Guenther
1 sibling, 1 reply; 6+ messages in thread
From: Bernd Schmidt @ 2011-04-14 12:43 UTC (permalink / raw)
To: GCC Patches
Ping. Contains only changes outside config/arm.
http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01509.html
Bernd
On 03/23/2011 04:46 PM, Bernd Schmidt wrote:
> I've discovered a problem with -mlong-calls on ARM. The bug was first
> reported against a new target, but I'd copied the relevant code from the
> ARM backend.
>
> We use current_function_section in arm_is_long_call_p to decide whether
> we're calling something that goes into the same section. The problem
> with this is that current_function_section can only be used during
> final, since it relies on the global variable in_cold_section_p which is
> set up only in assemble_start_function. On ARM, this problem manifests
> as short-calls when a long-call would be required; in the other port it
> was an "insn doesn't satisfy its constraints" error.
>
> The following patch is against 4.5, since the problem appears hidden in
> mainline (the initialization of first_function_block_is_cold has
> changed). Ok for trunk and branches after arm-linux tests complete?
> * function.c (init_function_start): Call decide_function_section.
> * varasm.c (decide_function_section): New function.
> (assemble_start_function): When not using
> flag_reorder_blocks_and_partition, don't compute in_cold_section_p
> or first_function_block_is_cold.
> * rtl.h (decide_function_section): Declare.
>
> * gcc.target/arm/cold-lc.c: New test.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem with ARM longcalls
2011-04-14 12:43 ` Bernd Schmidt
@ 2011-04-14 13:00 ` Richard Guenther
2011-05-03 13:10 ` Bernd Schmidt
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guenther @ 2011-04-14 13:00 UTC (permalink / raw)
To: Bernd Schmidt; +Cc: GCC Patches
On Thu, Apr 14, 2011 at 2:40 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> Ping. Contains only changes outside config/arm.
>
> http://gcc.gnu.org/m/gcc-patches/2011-03/msg01509.html
Ok.
Thanks,
Richard.
>
> Bernd
>
> On 03/23/2011 04:46 PM, Bernd Schmidt wrote:
>> I've discovered a problem with -mlong-calls on ARM. The bug was first
>> reported against a new target, but I'd copied the relevant code from the
>> ARM backend.
>>
>> We use current_function_section in arm_is_long_call_p to decide whether
>> we're calling something that goes into the same section. The problem
>> with this is that current_function_section can only be used during
>> final, since it relies on the global variable in_cold_section_p which is
>> set up only in assemble_start_function. On ARM, this problem manifests
>> as short-calls when a long-call would be required; in the other port it
>> was an "insn doesn't satisfy its constraints" error.
>>
>> The following patch is against 4.5, since the problem appears hidden in
>> mainline (the initialization of first_function_block_is_cold has
>> changed). Ok for trunk and branches after arm-linux tests complete?
>
>> * function.c (init_function_start): Call decide_function_section.
>> * varasm.c (decide_function_section): New function.
>> (assemble_start_function): When not using
>> flag_reorder_blocks_and_partition, don't compute in_cold_section_p
>> or first_function_block_is_cold.
>> * rtl.h (decide_function_section): Declare.
>>
>> * gcc.target/arm/cold-lc.c: New test.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Problem with ARM longcalls
2011-04-14 13:00 ` Richard Guenther
@ 2011-05-03 13:10 ` Bernd Schmidt
0 siblings, 0 replies; 6+ messages in thread
From: Bernd Schmidt @ 2011-05-03 13:10 UTC (permalink / raw)
To: Richard Guenther; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 520 bytes --]
On 04/14/2011 03:00 PM, Richard Guenther wrote:
> On Thu, Apr 14, 2011 at 2:40 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>> Ping. Contains only changes outside config/arm.
>>
>> http://gcc.gnu.org/m/gcc-patches/2011-03/msg01509.html
>
> Ok.
Committed this version on trunk (which has different code to initialize
first_function_block_is_cold). Tested again, with a profiledbootstrap
(which I hope exercised this code). I'll wait a few days and then put
the originally posted version on the branches.
Bernd
[-- Attachment #2: dfs-4.7.diff --]
[-- Type: text/plain, Size: 5249 bytes --]
Index: ChangeLog
===================================================================
--- ChangeLog (revision 173301)
+++ ChangeLog (working copy)
@@ -1,3 +1,12 @@
+2011-05-03 Bernd Schmidt <bernds@codesourcery.com>
+
+ * function.c (init_function_start): Call decide_function_section.
+ * varasm.c (decide_function_section): New function.
+ (assemble_start_function): When not using
+ flag_reorder_blocks_and_partition, don't compute in_cold_section_p
+ or first_function_block_is_cold.
+ * rtl.h (decide_function_section): Declare.
+
2011-05-03 Uros Bizjak <ubizjak@gmail.com>
Jakub Jelinek <jakub@redhat.com>
Index: testsuite/gcc.target/arm/cold-lc.c
===================================================================
--- testsuite/gcc.target/arm/cold-lc.c (revision 0)
+++ testsuite/gcc.target/arm/cold-lc.c (revision 0)
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mlong-calls" } */
+/* { dg-final { scan-assembler-not "bl\[^\n\]*dump_stack" } } */
+
+extern void dump_stack (void) __attribute__ ((__cold__)) __attribute__ ((noinline));
+struct thread_info {
+ struct task_struct *task;
+};
+extern struct thread_info *current_thread_info (void);
+
+void dump_stack (void)
+{
+ unsigned long stack;
+ show_stack ((current_thread_info ()->task), &stack);
+}
+
+void die (char *str, void *fp, int nr)
+{
+ dump_stack ();
+ while (1);
+}
+
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 173301)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,7 @@
+2011-05-03 Bernd Schmidt <bernds@codesourcery.com>
+
+ * gcc.target/arm/cold-lc.c: New test.
+
2011-05-03 Jakub Jelinek <jakub@redhat.com>
PR target/48774
Index: function.c
===================================================================
--- function.c (revision 173297)
+++ function.c (working copy)
@@ -4515,6 +4515,7 @@ init_function_start (tree subr)
else
allocate_struct_function (subr, false);
prepare_function_start ();
+ decide_function_section (subr);
/* Warn if this value is an aggregate type,
regardless of which calling convention we are using for it. */
Index: varasm.c
===================================================================
--- varasm.c (revision 173297)
+++ varasm.c (working copy)
@@ -1512,6 +1512,33 @@ notice_global_symbol (tree decl)
}
}
+/* If not using flag_reorder_blocks_and_partition, decide early whether the
+ current function goes into the cold section, so that targets can use
+ current_function_section during RTL expansion. DECL describes the
+ function. */
+
+void
+decide_function_section (tree decl)
+{
+ first_function_block_is_cold = false;
+
+ if (flag_reorder_blocks_and_partition)
+ /* We will decide in assemble_start_function. */
+ return;
+
+ if (DECL_SECTION_NAME (decl))
+ {
+ struct cgraph_node *node = cgraph_get_node (current_function_decl);
+ /* Calls to function_section rely on first_function_block_is_cold
+ being accurate. */
+ first_function_block_is_cold = (node
+ && node->frequency
+ == NODE_FREQUENCY_UNLIKELY_EXECUTED);
+ }
+
+ in_cold_section_p = first_function_block_is_cold;
+}
+
/* Output assembler code for the constant pool of a function and associated
with defining the name of the function. DECL describes the function.
NAME is the function's name. For the constant pool, we use the current
@@ -1524,7 +1551,6 @@ assemble_start_function (tree decl, cons
char tmp_label[100];
bool hot_label_written = false;
- first_function_block_is_cold = false;
if (flag_reorder_blocks_and_partition)
{
ASM_GENERATE_INTERNAL_LABEL (tmp_label, "LHOTB", const_labelno);
@@ -1559,6 +1585,8 @@ assemble_start_function (tree decl, cons
if (flag_reorder_blocks_and_partition)
{
+ first_function_block_is_cold = false;
+
switch_to_section (unlikely_text_section ());
assemble_align (DECL_ALIGN (decl));
ASM_OUTPUT_LABEL (asm_out_file, crtl->subsections.cold_section_label);
@@ -1575,18 +1603,9 @@ assemble_start_function (tree decl, cons
hot_label_written = true;
first_function_block_is_cold = true;
}
- }
- else if (DECL_SECTION_NAME (decl))
- {
- struct cgraph_node *node = cgraph_get_node (current_function_decl);
- /* Calls to function_section rely on first_function_block_is_cold
- being accurate. */
- first_function_block_is_cold = (node
- && node->frequency
- == NODE_FREQUENCY_UNLIKELY_EXECUTED);
+ in_cold_section_p = first_function_block_is_cold;
}
- in_cold_section_p = first_function_block_is_cold;
/* Switch to the correct text section for the start of the function. */
Index: rtl.h
===================================================================
--- rtl.h (revision 173300)
+++ rtl.h (working copy)
@@ -1668,6 +1668,7 @@ extern rtx get_pool_constant (rtx);
extern rtx get_pool_constant_mark (rtx, bool *);
extern enum machine_mode get_pool_mode (const_rtx);
extern rtx simplify_subtraction (rtx);
+extern void decide_function_section (tree);
/* In function.c */
extern rtx assign_stack_local (enum machine_mode, HOST_WIDE_INT, int);
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-05-03 13:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-23 15:47 Problem with ARM longcalls Bernd Schmidt
2011-03-24 14:24 ` Richard Earnshaw
2011-03-24 14:28 ` Bernd Schmidt
2011-04-14 12:43 ` Bernd Schmidt
2011-04-14 13:00 ` Richard Guenther
2011-05-03 13:10 ` Bernd Schmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).