This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 1/3] S/390: Implement -mfentry
On 07/16/2018 09:48 AM, Ilya Leoshkevich wrote:
> This is the counterpart of the i386 feature introduced by
> 39a5a6a4: Add direct support for Linux kernel __fentry__ patching.
>
> On i386, the difference between mcount and fentry is that fentry
> comes before the prolog. On s390 mcount already comes before the
> prolog, but takes 4 instructions. This patch introduces the more
> efficient implementation (just 1 instruction) and puts it under
> -mfentry flag.
>
> The produced code is compatible only with newer glibc versions,
> which provide the __fentry__ symbol and do not clobber %r0 when
> resolving lazily bound functions. Because 31-bit PLT stubs assume
> %r12 contains GOT address, which is not the case when the code runs
> before the prolog, -mfentry is allowed only for 64-bit code.
>
> Also, code compiled with -mfentry cannot be used for the nested C
> functions, since they both use %r0. In this case instrumentation is
> not insterted, and a new warning is issued for each affected nested
> function.
>
> * gcc/common.opt: Add the new warning.
> * gcc/config/s390/s390.c (s390_function_profiler): Emit
> "brasl %r0,__fentry__" when -mfentry is specified.
> (s390_option_override_internal): Disallow -mfentry for
> 31-bit CPUs.
> * gcc/config/s390/s390.opt: Add the new option.
> * gcc/testsuite/gcc.target/s390/mfentry-m64.c:
> New testcase.
Thanks! I've committed your patch with a modified changelog entry.
There are several ChangeLog files in the GCC source tree. Paths have to be relative to these. There
is e.g. a separate ChangeLog file for the testsuite.
Bye,
Andreas
> ---
> gcc/common.opt | 5 +++++
> gcc/config/s390/s390.c | 18 ++++++++++++++++--
> gcc/config/s390/s390.opt | 5 +++++
> gcc/testsuite/gcc.target/s390/mfentry-m64.c | 8 ++++++++
> 4 files changed, 34 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/s390/mfentry-m64.c
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index c29abdb5cb1..4d031e81b09 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -571,6 +571,11 @@ Wattribute-alias
> Common Var(warn_attributes) Init(1) Warning
> Warn about type safety and similar errors in attribute alias and related.
>
> +Wcannot-profile
> +Common Var(warn_cannot_profile) Init(1) Warning
> +Warn when profiling instrumentation was requested, but could not be applied to
> +a certain function.
> +
> Wcast-align
> Common Var(warn_cast_align) Warning
> Warn about pointer casts which increase alignment.
> diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
> index 23c3f3db621..3a406b955a0 100644
> --- a/gcc/config/s390/s390.c
> +++ b/gcc/config/s390/s390.c
> @@ -13144,14 +13144,22 @@ s390_function_profiler (FILE *file, int labelno)
> op[3] = gen_rtx_SYMBOL_REF (Pmode, label);
> SYMBOL_REF_FLAGS (op[3]) = SYMBOL_FLAG_LOCAL;
>
> - op[4] = gen_rtx_SYMBOL_REF (Pmode, "_mcount");
> + op[4] = gen_rtx_SYMBOL_REF (Pmode, flag_fentry ? "__fentry__" : "_mcount");
> if (flag_pic)
> {
> op[4] = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, op[4]), UNSPEC_PLT);
> op[4] = gen_rtx_CONST (Pmode, op[4]);
> }
>
> - if (TARGET_64BIT)
> + if (flag_fentry)
> + {
> + if (cfun->static_chain_decl)
> + warning (OPT_Wcannot_profile, "nested functions cannot be profiled "
> + "with -mfentry on s390");
> + else
> + output_asm_insn ("brasl\t0,%4", op);
> + }
> + else if (TARGET_64BIT)
> {
> output_asm_insn ("stg\t%0,%1", op);
> output_asm_insn ("larl\t%2,%3", op);
> @@ -15562,6 +15570,12 @@ s390_option_override_internal (bool main_args_p,
> /* Call target specific restore function to do post-init work. At the moment,
> this just sets opts->x_s390_cost_pointer. */
> s390_function_specific_restore (opts, NULL);
> +
> + /* Check whether -mfentry is supported. It cannot be used in 31-bit mode,
> + because 31-bit PLT stubs assume that %r12 contains GOT address, which is
> + not the case when the code runs before the prolog. */
> + if (opts->x_flag_fentry && !TARGET_64BIT)
> + error ("-mfentry is supported only for 64-bit CPUs");
> }
>
> static void
> diff --git a/gcc/config/s390/s390.opt b/gcc/config/s390/s390.opt
> index eb16f9c821f..59e97d031b4 100644
> --- a/gcc/config/s390/s390.opt
> +++ b/gcc/config/s390/s390.opt
> @@ -293,3 +293,8 @@ locations which have been patched as part of using one of the
> -mindirect-branch* or -mfunction-return* options. The sections
> consist of an array of 32 bit elements. Each entry holds the offset
> from the entry to the patched location.
> +
> +mfentry
> +Target Report Var(flag_fentry)
> +Emit profiling counter call at function entry before prologue. The compiled
> +code will require a 64-bit CPU and glibc 2.29 or newer to run.
> diff --git a/gcc/testsuite/gcc.target/s390/mfentry-m64.c b/gcc/testsuite/gcc.target/s390/mfentry-m64.c
> new file mode 100644
> index 00000000000..aa3fc81248f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/mfentry-m64.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile { target { lp64 } } } */
> +/* { dg-options "-pg -mfentry" } */
> +
> +void
> +profileme (void)
> +{
> + /* { dg-final { scan-assembler "brasl\t0,__fentry__" } } */
> +}
>