Re: [PATCH ARM v2] PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc generated by -pg
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. */ >> >> 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
Re: [PATCH ARM v2] PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc generated by -pg
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 @@ output_probe_stack_range (rtx reg1, rtx reg2) >return ""; > } > > +static void > +arm_emit_long_call_profile_insn () s/()/(void) > +{ > + rtx sym_r
Re: [PATCH ARM v2] PR69770 -mlong-calls does not affect calls to __gnu_mcount_nc generated by -pg
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