[1/3] S/390: Implement -mfentry

Message ID 20180716074807.43653-2-iii@linux.ibm.com
State New
Headers show
Series
  • [1/3] S/390: Implement -mfentry
Related show

Commit Message

Ilya Leoshkevich July 16, 2018, 7:48 a.m.
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.
---
 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

-- 
2.17.1

Comments

Andreas Krebbel July 16, 2018, 2:32 p.m. | #1
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__" } } */

> +}

>

Patch

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__" } } */
+}