Re: Statically propagate basic blocks which are likely executed 0 times

[Renlin Li <renlin.li@foss.arm.com>]

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

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;
>>>> +}
>>>

[-- Attachment #2: cold-1.diff --]
[-- Type: text/x-patch, Size: 603 bytes --]

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