diff mbox

Statically propagate basic blocks which are likely executed 0 times

Message ID 593E6AA1.6090900@foss.arm.com
State New
Headers show

Commit Message

Renlin Li June 12, 2017, 10:19 a.m. UTC 
Hi Honza & Christophe,

I have tested your suggested fix. It does fix the regression.
Here is a simple patch for it.

After r249013, die () and dump_stack () are both in cold section. This makes
the compiler generate bl instruction for the function call, instead of
honoring the -mlong-calls option.

This patch changes the dump_stack function call conditional, which fixes the
regression.

Okay to commit?

Regards,
Renlin

gcc/testsuite/ChangeLog:

2017-06-12  Renlin Li  <renlin.li@arm.com>

	* gcc.target/arm/cold-lc.c: Update coding style, call dump_stack
	conditionally.


On 09/06/17 10:54, Jan Hubicka wrote:
>> Since this commit (r249013), I've noticed a regression on arm targets:
>> FAIL: gcc.target/arm/cold-lc.c scan-assembler-not bl[^\n]*dump_stack
>
> I think that is because we optimize the testcase:
> /* { 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);
> extern int show_stack (struct task_struct *, unsigned long *);
>
> 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);
> }
>
> the new logic will move die() into cold section (because it unavoidably leads to cold
> code and thus allow using the bl instruction.
> I guess just modifying die to call dump_stack conditionally should fix the testcase.
>
> Honza
>>
>>
>>>> +  if (!n->analyzed
>>>> +      || n->decl == current_function_decl)
>>>> +    return false;
>>>> +  return n->frequency == NODE_FREQUENCY_UNLIKELY_EXECUTED;
>>>> +}
>>>

Comments

Jan Hubicka June 12, 2017, 10:36 a.m. UTC | #1 
> Hi Honza & Christophe,
> 
> I have tested your suggested fix. It does fix the regression.
> Here is a simple patch for it.
> 
> After r249013, die () and dump_stack () are both in cold section. This makes
> the compiler generate bl instruction for the function call, instead of
> honoring the -mlong-calls option.
> 
> This patch changes the dump_stack function call conditional, which fixes the
> regression.
> 
> Okay to commit?
> 
> Regards,
> Renlin
> 
> gcc/testsuite/ChangeLog:
> 
> 2017-06-12  Renlin Li  <renlin.li@arm.com>
> 
> 	* gcc.target/arm/cold-lc.c: Update coding style, call dump_stack
> 	conditionally.

Looks OK to me.  I think it does not change purpose of the testcase ;)

Honza
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.target/arm/cold-lc.c b/gcc/testsuite/gcc.target/arm/cold-lc.c
index 467a696..f0cd6df 100644
--- a/gcc/testsuite/gcc.target/arm/cold-lc.c
+++ b/gcc/testsuite/gcc.target/arm/cold-lc.c
@@ -11,13 +11,14 @@  extern int show_stack (struct task_struct *, unsigned long *);
 
 void dump_stack (void)
 {
-    unsigned long stack;
-    show_stack ((current_thread_info ()->task), &stack);
+  unsigned long stack;
+  show_stack ((current_thread_info ()->task), &stack);
 }
 
 void die (char *str, void *fp, int nr)
 {
+  if (nr)
     dump_stack ();
-    while (1);
+  while (1);
 }