Re: [RFC Patch, Aarch64] : Macros for profile code generation to enable gprof support
On 28 September 2013 11:57, Venkataramanan Kumar venkataramanan.ku...@linaro.org wrote: 2013-10-28 Venkataramanan Kumar venkataramanan.ku...@linaro.org * config/aarch64/aarch64.h (MCOUNT_NAME): Define. (NO_PROFILE_COUNTERS): Likewise. (PROFILE_HOOK): Likewise. (FUNCTION_PROFILER): Likewise. * config/aarch64/aarch64.c (aarch64_function_profiler): Remove. OK, Thank you. /Marcus
Re: [RFC Patch, Aarch64] : Macros for profile code generation to enable gprof support
Hi Marcus,
I have re-based the patch and tested for aarch64-none-elf with no regressions.
Also for aarch64-unknown-linux-gnu the following test cases passes.
Before:
UNSUPPORTED: gcc.dg/nested-func-4.c
UNSUPPORTED: gcc.dg/pr43643.c:
UNSUPPORTED: gcc.dg/nest.c
UNSUPPORTED: gcc.dg/20021014-1.c
UNSUPPORTED: gcc.dg/pr32450.c
UNSUPPORTED: g++.dg/other/profile1.C -std=gnu++98
UNSUPPORTED: g++.dg/other/profile1.C -std=gnu++11
After:
---
PASS: gcc.dg/nested-func-4.c (test for excess errors)
PASS: gcc.dg/nested-func-4.c execution test
PASS: gcc.dg/pr43643.c (test for excess errors)
PASS: gcc.dg/pr43643.c execution test
PASS: gcc.dg/nest.c (test for excess errors)
PASS: gcc.dg/nest.c execution test
PASS: gcc.dg/20021014-1.c (test for excess errors)
PASS: gcc.dg/20021014-1.c execution test
PASS: gcc.dg/pr32450.c (test for excess errors)
PASS: gcc.dg/pr32450.c execution test
PASS: g++.dg/other/profile1.C -std=gnu++98 (test for excess errors)
PASS: g++.dg/other/profile1.C -std=gnu++98 execution test
PASS: g++.dg/other/profile1.C -std=gnu++11 (test for excess errors)
PASS: g++.dg/other/profile1.C -std=gnu++11 execution test
Please let me know if I can commit it to trunk, given that glibc
patches are upstreamed.
2013-10-28 Venkataramanan Kumar venkataramanan.ku...@linaro.org
* config/aarch64/aarch64.h (MCOUNT_NAME): Define.
(NO_PROFILE_COUNTERS): Likewise.
(PROFILE_HOOK): Likewise.
(FUNCTION_PROFILER): Likewise.
* config/aarch64/aarch64.c (aarch64_function_profiler): Remove.
regards,
Venkat.
On 27 August 2013 13:05, Marcus Shawcroft marcus.shawcr...@gmail.com wrote:
Hi Venkat,
On 3 August 2013 19:01, Venkataramanan Kumar
venkataramanan.ku...@linaro.org wrote:
This patch adds macros to support gprof in Aarch64. The difference
from the previous patch is that the compiler, while generating
mcount routine for an instrumented function, also passes the return
address as argument.
The mcount routine in glibc will be modified as follows.
(-Snip-)
#define MCOUNT \
-void __mcount (void)
\
+void __mcount (void* frompc)
\
{
\
- mcount_internal ((u_long) RETURN_ADDRESS (1), (u_long) RETURN_ADDRESS
(0)); \
+ mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \
}
(-Snip-)
If this is Ok I will send the patch to glibc as well.
2013-08-02 Venkataramanan Kumar venkataramanan.ku...@linaro.org
* config/aarch64/aarch64.h (MCOUNT_NAME): Define.
(NO_PROFILE_COUNTERS): Likewise.
(PROFILE_HOOK): Likewise.
(FUNCTION_PROFILER): Likewise.
* config/aarch64/aarch64.c (aarch64_function_profiler): Remove.
.
regards,
Venkat.
+ emit_library_call (fun, LCT_NORMAL, VOIDmode, 1,lr,Pmode); \
+}
GNU coding style requires spaces after the commas, but otherwise I
have no further comments on this patch. Post the glibc patch please.
Thanks
/Marcus
Index: gcc/config/aarch64/aarch64.c
===
--- gcc/config/aarch64/aarch64.c(revision 202934)
+++ gcc/config/aarch64/aarch64.c(working copy)
@@ -3857,13 +3857,6 @@
output_addr_const (f, x);
}
-void
-aarch64_function_profiler (FILE *f ATTRIBUTE_UNUSED,
- int labelno ATTRIBUTE_UNUSED)
-{
- sorry (function profiling);
-}
-
bool
aarch64_label_mentioned_p (rtx x)
{
Index: gcc/config/aarch64/aarch64.h
===
--- gcc/config/aarch64/aarch64.h(revision 202934)
+++ gcc/config/aarch64/aarch64.h(working copy)
@@ -783,9 +783,23 @@
#define PRINT_OPERAND_ADDRESS(STREAM, X) \
aarch64_print_operand_address (STREAM, X)
-#define FUNCTION_PROFILER(STREAM, LABELNO) \
- aarch64_function_profiler (STREAM, LABELNO)
+#define MCOUNT_NAME _mcount
+#define NO_PROFILE_COUNTERS 1
+
+/* Emit rtl for profiling. Output assembler code to FILE
+ to call _mcount for profiling a function entry. */
+#define PROFILE_HOOK(LABEL)\
+{ \
+ rtx fun,lr; \
+ lr = get_hard_reg_initial_val (Pmode, LR_REGNUM);\
+ fun = gen_rtx_SYMBOL_REF (Pmode, MCOUNT_NAME); \
+ emit_library_call (fun, LCT_NORMAL, VOIDmode, 1, lr, Pmode); \
+}
+
+/* All the work done in PROFILE_HOOK, but still required. */
+#define FUNCTION_PROFILER(STREAM, LABELNO) do { } while (0)
+
/* For some reason, the Linux headers think they know how to define
these macros. They don't!!! */
#undef ASM_APP_ON
Index: gcc/testsuite/lib/target-supports.exp
===
--- gcc/testsuite/lib/target-supports.exp (revision
Re: [RFC Patch, Aarch64] : Macros for profile code generation to enable gprof support
Hi Venkat,
On 3 August 2013 19:01, Venkataramanan Kumar
venkataramanan.ku...@linaro.org wrote:
This patch adds macros to support gprof in Aarch64. The difference
from the previous patch is that the compiler, while generating
mcount routine for an instrumented function, also passes the return
address as argument.
The mcount routine in glibc will be modified as follows.
(-Snip-)
#define MCOUNT \
-void __mcount (void)
\
+void __mcount (void* frompc)
\
{
\
- mcount_internal ((u_long) RETURN_ADDRESS (1), (u_long) RETURN_ADDRESS
(0)); \
+ mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \
}
(-Snip-)
If this is Ok I will send the patch to glibc as well.
2013-08-02 Venkataramanan Kumar venkataramanan.ku...@linaro.org
* config/aarch64/aarch64.h (MCOUNT_NAME): Define.
(NO_PROFILE_COUNTERS): Likewise.
(PROFILE_HOOK): Likewise.
(FUNCTION_PROFILER): Likewise.
* config/aarch64/aarch64.c (aarch64_function_profiler): Remove.
.
regards,
Venkat.
+ emit_library_call (fun, LCT_NORMAL, VOIDmode, 1,lr,Pmode); \
+}
GNU coding style requires spaces after the commas, but otherwise I
have no further comments on this patch. Post the glibc patch please.
Thanks
/Marcus
Re: [RFC Patch, Aarch64] : Macros for profile code generation to enable gprof support
Marcus, On 9 August 2013 18:17, Marcus Shawcroft marcus.shawcr...@arm.com wrote: On 03/08/13 19:01, Venkataramanan Kumar wrote: 2013-08-02 Venkataramanan Kumar venkataramanan.ku...@linaro.org * config/aarch64/aarch64.h (MCOUNT_NAME): Define. (NO_PROFILE_COUNTERS): Likewise. (PROFILE_HOOK): Likewise. (FUNCTION_PROFILER): Likewise. * config/aarch64/aarch64.c (aarch64_function_profiler): Remove. . regards, Venkat. Hi Venkat, Looking at the various other ports it looks that the majority choose to use FUNCTION_PROFILER_HOOK rather than PROFILE_HOOK. Using PROFILE_HOOK to inject a regular call to to _mcount() means that all arguments passed in registers in every function will be spilled and reloaded because the _mcount call will kill the caller save registers. Using the FUNCTION_PROFILER_HOOK and taking care not to kill the caller save registers would be less invasive. The LR argument to _mcount would need to be passed in a temporary register, say x9 and _mcount would also need to ensure caller save registers are saved and restored. The latter seems to be a better option to me, is there compelling reason to choose PROFILE_HOOK over FUNCTION_PROFILER_HOOK ?? (I think you mean FUNCTION_PROFILER rather than FUNCTION_PROFILER_HOOK in all the above.) Using either PROFILE_HOOK or FUNCTION_PROFILER results in a call chain that looks like the following (assuming the C Library is glibc): Function - _mcount - _mcount_internal. Where _mcount_internal is the C function that does the real work and is provided in glibc. Importantly this means that _mcount_internal follows the normal ABI - so we have to save the caller saved registers somewhere. Using FUNCTION_PROFILER requires us to write assembler which saves and restores all caller saved registers every time it is called, and requires (as you say) a special ABI. This means _mcount ends up being a piece of assembly that saves all caller-saved registers (i.e. parameter-passing temporary registers) and then makes the call to _mcount internal before restoring everything on _mcount's return. Using PROFILE_HOOK will cause the compiler to do all the heavy lifting, and it will do the minimum required (for example with a function with one parameter it will only save and restore x0). _mcount in this case can be a simple function that sets up some parameters and calls _mcount_internal (or even _mcount could just alias _mcount_internal). As to which of PROFILE_HOOK or FUNCTION_PROFILER are the right way (TM) - I don't know - the documentation isn't very clear at all. PROFILE_HOOK was introduced to support profiling for AIX 4.3. http://gcc.gnu.org/ml/gcc-patches/2000-12/msg00580.html is the initial patch, with a reworked patch here: http://gcc.gnu.org/ml/gcc-patches/2001-02/msg00112.html. The final commit happening on 2001-02-05. The patch was introduced because it was impossible to make FUNCTION_PROFILER work for AIX 4.3 and so a new hook that worked earlier in the compiler was needed. There doesn't seem to have been a discussion about preferring one form over the other. In conclusion - I prefer the PROFILE_HOOK method because it makes the compiler do all the work, and results in less impact on stack usage and performance. FUNCTION_PROFILER may impact the code generated by the compiler less and produce a smaller overall image - but I'm not sure that's more beneficial. Thanks, Matt -- Matthew Gretton-Dann Linaro Toolchain Working Group matthew.gretton-d...@linaro.org
Re: [RFC Patch, Aarch64] : Macros for profile code generation to enable gprof support
ping!
On 3 August 2013 23:31, Venkataramanan Kumar
venkataramanan.ku...@linaro.org wrote:
Hi Maintainers,
This patch adds macros to support gprof in Aarch64. The difference
from the previous patch is that the compiler, while generating
mcount routine for an instrumented function, also passes the return
address as argument.
The mcount routine in glibc will be modified as follows.
(-Snip-)
#define MCOUNT \
-void __mcount (void)
\
+void __mcount (void* frompc)
\
{
\
- mcount_internal ((u_long) RETURN_ADDRESS (1), (u_long) RETURN_ADDRESS
(0)); \
+ mcount_internal ((u_long) frompc, (u_long) RETURN_ADDRESS (0)); \
}
(-Snip-)
Also in Aarch64 cases__builtin_return_adderess(n) where n0, still be
returning 0 as it was doing before.
If this is Ok I will send the patch to glibc as well.
2013-08-02 Venkataramanan Kumar venkataramanan.ku...@linaro.org
* config/aarch64/aarch64.h (MCOUNT_NAME): Define.
(NO_PROFILE_COUNTERS): Likewise.
(PROFILE_HOOK): Likewise.
(FUNCTION_PROFILER): Likewise.
* config/aarch64/aarch64.c (aarch64_function_profiler): Remove.
.
regards,
Venkat.
Re: [RFC Patch, Aarch64] : Macros for profile code generation to enable gprof support
On 03/08/13 19:01, Venkataramanan Kumar wrote: 2013-08-02 Venkataramanan Kumar venkataramanan.ku...@linaro.org * config/aarch64/aarch64.h (MCOUNT_NAME): Define. (NO_PROFILE_COUNTERS): Likewise. (PROFILE_HOOK): Likewise. (FUNCTION_PROFILER): Likewise. * config/aarch64/aarch64.c (aarch64_function_profiler): Remove. . regards, Venkat. Hi Venkat, Looking at the various other ports it looks that the majority choose to use FUNCTION_PROFILER_HOOK rather than PROFILE_HOOK. Using PROFILE_HOOK to inject a regular call to to _mcount() means that all arguments passed in registers in every function will be spilled and reloaded because the _mcount call will kill the caller save registers. Using the FUNCTION_PROFILER_HOOK and taking care not to kill the caller save registers would be less invasive. The LR argument to _mcount would need to be passed in a temporary register, say x9 and _mcount would also need to ensure caller save registers are saved and restored. The latter seems to be a better option to me, is there compelling reason to choose PROFILE_HOOK over FUNCTION_PROFILER_HOOK ?? Cheers /Marcus
