Re: [PATCH ARM v2] PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc generated by -pg

2016-03-30 Thread Ramana Radhakrishnan 
And some more formatting issues.

On 30/03/16 10:33, Ramana Radhakrishnan wrote:
> 
> 
> On 24/03/16 21:02, Charles Baylis wrote:
>> When compiling with -mlong-calls and -pg, calls to the __gnu_mcount_nc
>> function are not generated as long calls.
>>
>> This is the sequel to this patch
>> https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00881.html
>>
>> This patch fixes the following problems with the previous patch.
>> . Nested functions now work (thanks to Richard E for spotting this)
>> . Thumb-1 now works
> 
>>
>> This patch works by adding new patterns (one for ARM/Thumb-2 and one
>> for Thumb-1) which are placed in the prologue as a placeholder for
>> some RTL which describes the address. This is either a SYMBOL_REF for
>> targets with MOVW/MOVT, or a literal pool reference for other targets.
>> The implementation of ARM_FUNCTION_PROFILER is changed to search for
>> this insn so that the the address of the __gnu_mcount_nc function can
>> be loaded using an appropriate sequence for the target.
> 
> I'm reasonably happy with the approach but there are nits.
> 
>>
>> I also tried generating the profiling call sequence directly in the
>> prologue, but this requires some unpleasant hacks to prevent spurious
>> register pushes from ASM_OUTPUT_REG_PUSH.
>>
>> Tested with no new regressions on arm-unknown-linux-gnueabihf on QEMU.
>> The generated code sequences have been inspected for normal and nested
>> functions on ARM v6, ARM v7, Thumb-1, and Thumb-2 targets.
>>
>> This does not fix a regression, so I don't expect to apply it for
>> GCC6, is it OK for when stage 1 re-opens.
> 
> 
> I'm not sure how much testing coverage you get by just running the testsuite. 
> Doing a profiled bootstrap with -mlong-calls and a regression test run for 
> arm and / or thumb2 would be a useful test to do additionally - Given that 
> this originally came from kernel builds with allyesconfig how important is 
> this to be fixed for GCC 6 ? I'd rather take the fix into GCC 6 to get the 
> kernel building again but that's something we can discuss with the RM's once 
> the issues with the patch are fixed.
>>
>> gcc/ChangeLog:
>>
>> 2016-03-24  Charles Baylis  
>>
>> * config/arm/arm-protos.h (arm_emit_long_call_profile): New function.
>> * config/arm/arm.c (arm_emit_long_call_profile_insn): New function.
>> (arm_expand_prologue): Likewise.
>> (thumb1_expand_prologue): Likewise.
>> (arm_output_long_call_to_profile_func): Likewise.
>> (arm_emit_long_call_profile): Likewise.
>> * config/arm/arm.h: (ASM_OUTPUT_REG_PUSH) Update comment.
>> * config/arm/arm.md (arm_long_call_profile): New pattern.
>> * config/arm/bpabi.h (ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS): New
>> define.
>> * config/arm/thumb1.md (thumb1_long_call_profile): New pattern.
>> * config/arm/unspecs.md (unspecv): Add VUNSPEC_LONG_CALL_PROFILE.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-03-24  Charles Baylis  
>>
>> * gcc.target/arm/pr69770.c: New test.
>>
>>
>> 0001-PR69770-mlong-calls-does-not-affect-calls-to-__gnu_m.patch
>>
>>
>> From 5a39451f34be9b6ca98b3460bf40d879d6ee61a5 Mon Sep 17 00:00:00 2001
>> From: Charles Baylis 
>> Date: Thu, 24 Mar 2016 20:43:25 +
>> Subject: [PATCH] PR69770 -mlong-calls does not affect calls to 
>> __gnu_mcount_nc
>>  generated by -pg
>>
>> gcc/ChangeLog:
>>
>> 2016-03-24  Charles Baylis  
>>
>> * config/arm/arm-protos.h (arm_emit_long_call_profile): New function.
>> * config/arm/arm.c (arm_emit_long_call_profile_insn): New function.
>> (arm_expand_prologue): Likewise.
>> (thumb1_expand_prologue): Likewise.
>> (arm_output_long_call_to_profile_func): Likewise.
>> (arm_emit_long_call_profile): Likewise.
>> * config/arm/arm.h: (ASM_OUTPUT_REG_PUSH) Update comment.
>> * config/arm/arm.md (arm_long_call_profile): New pattern.
>> * config/arm/bpabi.h (ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS): New
>>  define.
>> * config/arm/thumb1.md (thumb1_long_call_profile): New pattern.
>> * config/arm/unspecs.md (unspecv): Add VUNSPEC_LONG_CALL_PROFILE.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-03-24  Charles Baylis  
>>
>> * gcc.target/arm/pr69770.c: New test.
>>
>> Change-Id: I9b8de01fea083f17f729c3801f83174bedb3b0c6
>>
>> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
>> index 0083673..324c9f4 100644
>> --- a/gcc/config/arm/arm-protos.h
>> +++ b/gcc/config/arm/arm-protos.h
>> @@ -343,6 +343,7 @@ extern void arm_register_target_pragmas (void);
>>  extern void arm_cpu_cpp_builtins (struct cpp_reader *);
>>  
>>  extern bool arm_is_constant_pool_ref (rtx);
>> +void arm_emit_long_call_profile ();
>>  
>>  /* Flags used to identify the presence of processor capabilities.  */
>>  
>>

Re: [PATCH ARM v2] PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc generated by -pg

2016-03-30 Thread Ramana Radhakrishnan 


On 24/03/16 21:02, Charles Baylis wrote:
> When compiling with -mlong-calls and -pg, calls to the __gnu_mcount_nc
> function are not generated as long calls.
> 
> This is the sequel to this patch
> https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00881.html
> 
> This patch fixes the following problems with the previous patch.
> . Nested functions now work (thanks to Richard E for spotting this)
> . Thumb-1 now works

> 
> This patch works by adding new patterns (one for ARM/Thumb-2 and one
> for Thumb-1) which are placed in the prologue as a placeholder for
> some RTL which describes the address. This is either a SYMBOL_REF for
> targets with MOVW/MOVT, or a literal pool reference for other targets.
> The implementation of ARM_FUNCTION_PROFILER is changed to search for
> this insn so that the the address of the __gnu_mcount_nc function can
> be loaded using an appropriate sequence for the target.

I'm reasonably happy with the approach but there are nits.

> 
> I also tried generating the profiling call sequence directly in the
> prologue, but this requires some unpleasant hacks to prevent spurious
> register pushes from ASM_OUTPUT_REG_PUSH.
> 
> Tested with no new regressions on arm-unknown-linux-gnueabihf on QEMU.
> The generated code sequences have been inspected for normal and nested
> functions on ARM v6, ARM v7, Thumb-1, and Thumb-2 targets.
> 
> This does not fix a regression, so I don't expect to apply it for
> GCC6, is it OK for when stage 1 re-opens.


I'm not sure how much testing coverage you get by just running the testsuite. 
Doing a profiled bootstrap with -mlong-calls and a regression test run for arm 
and / or thumb2 would be a useful test to do additionally - Given that this 
originally came from kernel builds with allyesconfig how important is this to 
be fixed for GCC 6 ? I'd rather take the fix into GCC 6 to get the kernel 
building again but that's something we can discuss with the RM's once the 
issues with the patch are fixed.
> 
> gcc/ChangeLog:
> 
> 2016-03-24  Charles Baylis  
> 
> * config/arm/arm-protos.h (arm_emit_long_call_profile): New function.
> * config/arm/arm.c (arm_emit_long_call_profile_insn): New function.
> (arm_expand_prologue): Likewise.
> (thumb1_expand_prologue): Likewise.
> (arm_output_long_call_to_profile_func): Likewise.
> (arm_emit_long_call_profile): Likewise.
> * config/arm/arm.h: (ASM_OUTPUT_REG_PUSH) Update comment.
> * config/arm/arm.md (arm_long_call_profile): New pattern.
> * config/arm/bpabi.h (ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS): New
> define.
> * config/arm/thumb1.md (thumb1_long_call_profile): New pattern.
> * config/arm/unspecs.md (unspecv): Add VUNSPEC_LONG_CALL_PROFILE.
> 
> gcc/testsuite/ChangeLog:
> 
> 2016-03-24  Charles Baylis  
> 
> * gcc.target/arm/pr69770.c: New test.
> 
> 
> 0001-PR69770-mlong-calls-does-not-affect-calls-to-__gnu_m.patch
> 
> 
> From 5a39451f34be9b6ca98b3460bf40d879d6ee61a5 Mon Sep 17 00:00:00 2001
> From: Charles Baylis 
> Date: Thu, 24 Mar 2016 20:43:25 +
> Subject: [PATCH] PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc
>  generated by -pg
> 
> gcc/ChangeLog:
> 
> 2016-03-24  Charles Baylis  
> 
> * config/arm/arm-protos.h (arm_emit_long_call_profile): New function.
> * config/arm/arm.c (arm_emit_long_call_profile_insn): New function.
> (arm_expand_prologue): Likewise.
> (thumb1_expand_prologue): Likewise.
> (arm_output_long_call_to_profile_func): Likewise.
> (arm_emit_long_call_profile): Likewise.
> * config/arm/arm.h: (ASM_OUTPUT_REG_PUSH) Update comment.
> * config/arm/arm.md (arm_long_call_profile): New pattern.
> * config/arm/bpabi.h (ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS): New
>   define.
> * config/arm/thumb1.md (thumb1_long_call_profile): New pattern.
> * config/arm/unspecs.md (unspecv): Add VUNSPEC_LONG_CALL_PROFILE.
> 
> gcc/testsuite/ChangeLog:
> 
> 2016-03-24  Charles Baylis  
> 
> * gcc.target/arm/pr69770.c: New test.
> 
> Change-Id: I9b8de01fea083f17f729c3801f83174bedb3b0c6
> 
> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
> index 0083673..324c9f4 100644
> --- a/gcc/config/arm/arm-protos.h
> +++ b/gcc/config/arm/arm-protos.h
> @@ -343,6 +343,7 @@ extern void arm_register_target_pragmas (void);
>  extern void arm_cpu_cpp_builtins (struct cpp_reader *);
>  
>  extern bool arm_is_constant_pool_ref (rtx);
> +void arm_emit_long_call_profile ();
>  
>  /* Flags used to identify the presence of processor capabilities.  */
>  
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index c868490..040b255 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -21426,6 +21426,22 @@

Re: [PATCH ARM v2] PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc generated by -pg

2016-03-28 Thread Kugan 


On 25/03/16 08:02, Charles Baylis wrote:
> When compiling with -mlong-calls and -pg, calls to the __gnu_mcount_nc
> function are not generated as long calls.
> 
> This is the sequel to this patch
> https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00881.html
> 
> This patch fixes the following problems with the previous patch.
> . Nested functions now work (thanks to Richard E for spotting this)
> . Thumb-1 now works
> 
> This patch works by adding new patterns (one for ARM/Thumb-2 and one
> for Thumb-1) which are placed in the prologue as a placeholder for
> some RTL which describes the address. This is either a SYMBOL_REF for
> targets with MOVW/MOVT, or a literal pool reference for other targets.
> The implementation of ARM_FUNCTION_PROFILER is changed to search for
> this insn so that the the address of the __gnu_mcount_nc function can
> be loaded using an appropriate sequence for the target.
> 
> I also tried generating the profiling call sequence directly in the
> prologue, but this requires some unpleasant hacks to prevent spurious
> register pushes from ASM_OUTPUT_REG_PUSH.
> 
> Tested with no new regressions on arm-unknown-linux-gnueabihf on QEMU.
> The generated code sequences have been inspected for normal and nested
> functions on ARM v6, ARM v7, Thumb-1, and Thumb-2 targets.
> 
> This does not fix a regression, so I don't expect to apply it for
> GCC6, is it OK for when stage 1 re-opens.
> 

Hi Charles,

+static void
+arm_emit_long_call_profile_insn ()
+{
+  rtx sym_ref = gen_rtx_SYMBOL_REF (Pmode, "__gnu_mcount_nc");
+  /* if movt/movw are not available, use a constant pool */
+  if (!arm_arch_thumb2)

Should this be !TARGET_USE_MOVT?

Thanks,
Kugan

[PATCH ARM v2] PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc generated by -pg

2016-03-24 Thread Charles Baylis 
When compiling with -mlong-calls and -pg, calls to the __gnu_mcount_nc
function are not generated as long calls.

This is the sequel to this patch
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00881.html

This patch fixes the following problems with the previous patch.
. Nested functions now work (thanks to Richard E for spotting this)
. Thumb-1 now works

This patch works by adding new patterns (one for ARM/Thumb-2 and one
for Thumb-1) which are placed in the prologue as a placeholder for
some RTL which describes the address. This is either a SYMBOL_REF for
targets with MOVW/MOVT, or a literal pool reference for other targets.
The implementation of ARM_FUNCTION_PROFILER is changed to search for
this insn so that the the address of the __gnu_mcount_nc function can
be loaded using an appropriate sequence for the target.

I also tried generating the profiling call sequence directly in the
prologue, but this requires some unpleasant hacks to prevent spurious
register pushes from ASM_OUTPUT_REG_PUSH.

Tested with no new regressions on arm-unknown-linux-gnueabihf on QEMU.
The generated code sequences have been inspected for normal and nested
functions on ARM v6, ARM v7, Thumb-1, and Thumb-2 targets.

This does not fix a regression, so I don't expect to apply it for
GCC6, is it OK for when stage 1 re-opens.

gcc/ChangeLog:

2016-03-24  Charles Baylis  

* config/arm/arm-protos.h (arm_emit_long_call_profile): New function.
* config/arm/arm.c (arm_emit_long_call_profile_insn): New function.
(arm_expand_prologue): Likewise.
(thumb1_expand_prologue): Likewise.
(arm_output_long_call_to_profile_func): Likewise.
(arm_emit_long_call_profile): Likewise.
* config/arm/arm.h: (ASM_OUTPUT_REG_PUSH) Update comment.
* config/arm/arm.md (arm_long_call_profile): New pattern.
* config/arm/bpabi.h (ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS): New
define.
* config/arm/thumb1.md (thumb1_long_call_profile): New pattern.
* config/arm/unspecs.md (unspecv): Add VUNSPEC_LONG_CALL_PROFILE.

gcc/testsuite/ChangeLog:

2016-03-24  Charles Baylis  

* gcc.target/arm/pr69770.c: New test.
From 5a39451f34be9b6ca98b3460bf40d879d6ee61a5 Mon Sep 17 00:00:00 2001
From: Charles Baylis 
Date: Thu, 24 Mar 2016 20:43:25 +
Subject: [PATCH] PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc
 generated by -pg

gcc/ChangeLog:

2016-03-24  Charles Baylis  

* config/arm/arm-protos.h (arm_emit_long_call_profile): New function.
* config/arm/arm.c (arm_emit_long_call_profile_insn): New function.
(arm_expand_prologue): Likewise.
(thumb1_expand_prologue): Likewise.
(arm_output_long_call_to_profile_func): Likewise.
(arm_emit_long_call_profile): Likewise.
* config/arm/arm.h: (ASM_OUTPUT_REG_PUSH) Update comment.
* config/arm/arm.md (arm_long_call_profile): New pattern.
* config/arm/bpabi.h (ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS): New
	define.
* config/arm/thumb1.md (thumb1_long_call_profile): New pattern.
* config/arm/unspecs.md (unspecv): Add VUNSPEC_LONG_CALL_PROFILE.

gcc/testsuite/ChangeLog:

2016-03-24  Charles Baylis  

* gcc.target/arm/pr69770.c: New test.

Change-Id: I9b8de01fea083f17f729c3801f83174bedb3b0c6

diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 0083673..324c9f4 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -343,6 +343,7 @@ extern void arm_register_target_pragmas (void);
 extern void arm_cpu_cpp_builtins (struct cpp_reader *);
 
 extern bool arm_is_constant_pool_ref (rtx);
+void arm_emit_long_call_profile ();
 
 /* Flags used to identify the presence of processor capabilities.  */
 
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c868490..040b255 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -21426,6 +21426,22 @@ output_probe_stack_range (rtx reg1, rtx reg2)
   return "";
 }
 
+static void
+arm_emit_long_call_profile_insn ()
+{
+  rtx sym_ref = gen_rtx_SYMBOL_REF (Pmode, "__gnu_mcount_nc");
+  /* if movt/movw are not available, use a constant pool */
+  if (!arm_arch_thumb2)
+  {
+sym_ref = force_const_mem(Pmode, sym_ref);
+  }
+  rtvec vec = gen_rtvec (1, sym_ref);
+  rtx tmp =
+gen_rtx_UNSPEC_VOLATILE (VOIDmode, vec, VUNSPEC_LONG_CALL_PROFILE);
+  emit_insn (tmp);
+}
+
+
 /* Generate the prologue instructions for entry into an ARM or Thumb-2
function.  */
 void
@@ -21789,6 +21805,10 @@ arm_expand_prologue (void)
   arm_load_pic_register (mask);
 }
 
+  if (crtl->profile && TARGET_LONG_CALLS
+  && ARM_FUNCTION_PROFILER_SUPPORTS_LONG_CALLS)
+arm_emit_long_call_profile_insn ();
+
   /* If we are profiling, make sure no instructions are scheduled before
  the call to