IBM zSystems: Fix function_ok_for_sibcall [PR106355]

For a parameter with BLKmode we cannot use REG_NREGS in order to
determine the number of consecutive registers.  Streamlined this with
the implementation of s390_function_arg.

Fix some indentation whitespace, too.

Assuming bootstrap and regtest are ok for mainline and gcc-{10,11,12},
ok to install for all of those?

PR target/106355

gcc/ChangeLog:

	* config/s390/s390.cc (s390_call_saved_register_used): For a
	parameter with BLKmode fix determining number of consecutive
	registers.

gcc/testsuite/ChangeLog:

	* gcc.target/s390/pr106355.h: Common code for new tests.
	* gcc.target/s390/pr106355-1.c: New test.
	* gcc.target/s390/pr106355-2.c: New test.
	* gcc.target/s390/pr106355-3.c: New test.
---
 gcc/config/s390/s390.cc                    | 47 +++++++++++-----------
 gcc/testsuite/gcc.target/s390/pr106355-1.c | 42 +++++++++++++++++++
 gcc/testsuite/gcc.target/s390/pr106355-2.c |  8 ++++
 gcc/testsuite/gcc.target/s390/pr106355-3.c |  8 ++++
 gcc/testsuite/gcc.target/s390/pr106355.h   | 18 +++++++++
 5 files changed, 100 insertions(+), 23 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/pr106355-1.c
 create mode 100644 gcc/testsuite/gcc.target/s390/pr106355-2.c
 create mode 100644 gcc/testsuite/gcc.target/s390/pr106355-3.c
 create mode 100644 gcc/testsuite/gcc.target/s390/pr106355.h

On Wed, Aug 17, 2022 at 1:57 PM Stefan Schulze Frielinghaus via
Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
> For a parameter with BLKmode we cannot use REG_NREGS in order to
> determine the number of consecutive registers.  Streamlined this with
> the implementation of s390_function_arg.
>
> Fix some indentation whitespace, too.
>
> Assuming bootstrap and regtest are ok for mainline and gcc-{10,11,12},

Note the GCC 12 branch is currently frozen.

Richard.

> ok to install for all of those?
>
> PR target/106355
>
> gcc/ChangeLog:
>
>         * config/s390/s390.cc (s390_call_saved_register_used): For a
>         parameter with BLKmode fix determining number of consecutive
>         registers.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/s390/pr106355.h: Common code for new tests.
>         * gcc.target/s390/pr106355-1.c: New test.
>         * gcc.target/s390/pr106355-2.c: New test.
>         * gcc.target/s390/pr106355-3.c: New test.
> ---
>  gcc/config/s390/s390.cc                    | 47 +++++++++++-----------
>  gcc/testsuite/gcc.target/s390/pr106355-1.c | 42 +++++++++++++++++++
>  gcc/testsuite/gcc.target/s390/pr106355-2.c |  8 ++++
>  gcc/testsuite/gcc.target/s390/pr106355-3.c |  8 ++++
>  gcc/testsuite/gcc.target/s390/pr106355.h   | 18 +++++++++
>  5 files changed, 100 insertions(+), 23 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/s390/pr106355-1.c
>  create mode 100644 gcc/testsuite/gcc.target/s390/pr106355-2.c
>  create mode 100644 gcc/testsuite/gcc.target/s390/pr106355-3.c
>  create mode 100644 gcc/testsuite/gcc.target/s390/pr106355.h
>
> diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc
> index 5aaf76a9490..85e5b2cb2a2 100644
> --- a/gcc/config/s390/s390.cc
> +++ b/gcc/config/s390/s390.cc
> @@ -13712,36 +13712,37 @@ s390_call_saved_register_used (tree call_expr)
>        function_arg_info arg (TREE_TYPE (parameter), /*named=*/true);
>        apply_pass_by_reference_rules (&cum_v, arg);
>
> -       parm_rtx = s390_function_arg (cum, arg);
> +      parm_rtx = s390_function_arg (cum, arg);
>
> -       s390_function_arg_advance (cum, arg);
> +      s390_function_arg_advance (cum, arg);
>
> -       if (!parm_rtx)
> -        continue;
> -
> -       if (REG_P (parm_rtx))
> -        {
> -          for (reg = 0; reg < REG_NREGS (parm_rtx); reg++)
> -            if (!call_used_or_fixed_reg_p (reg + REGNO (parm_rtx)))
> -              return true;
> -        }
> +      if (!parm_rtx)
> +       continue;
>
> -       if (GET_CODE (parm_rtx) == PARALLEL)
> -        {
> -          int i;
> +      if (REG_P (parm_rtx))
> +       {
> +         int size = s390_function_arg_size (arg.mode, arg.type);
> +         int nregs = (size + UNITS_PER_LONG - 1) / UNITS_PER_LONG;
>
> -          for (i = 0; i < XVECLEN (parm_rtx, 0); i++)
> -            {
> -              rtx r = XEXP (XVECEXP (parm_rtx, 0, i), 0);
> +         for (reg = 0; reg < nregs; reg++)
> +           if (!call_used_or_fixed_reg_p (reg + REGNO (parm_rtx)))
> +             return true;
> +       }
> +      else if (GET_CODE (parm_rtx) == PARALLEL)
> +       {
> +         int i;
>
> -              gcc_assert (REG_P (r));
> +         for (i = 0; i < XVECLEN (parm_rtx, 0); i++)
> +           {
> +             rtx r = XEXP (XVECEXP (parm_rtx, 0, i), 0);
>
> -              for (reg = 0; reg < REG_NREGS (r); reg++)
> -                if (!call_used_or_fixed_reg_p (reg + REGNO (r)))
> -                  return true;
> -            }
> -        }
> +             gcc_assert (REG_P (r));
> +             gcc_assert (REG_NREGS (r) == 1);
>
> +             if (!call_used_or_fixed_reg_p (REGNO (r)))
> +               return true;
> +           }
> +       }
>      }
>    return false;
>  }
> diff --git a/gcc/testsuite/gcc.target/s390/pr106355-1.c b/gcc/testsuite/gcc.target/s390/pr106355-1.c
> new file mode 100644
> index 00000000000..1ec0f6b25ac
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/pr106355-1.c
> @@ -0,0 +1,42 @@
> +/* { dg-do compile } */
> +/* { dg-options "-foptimize-sibling-calls" } */
> +/* { dg-final { scan-assembler {brasl\t%r\d+,bar4} } } */
> +/* { dg-final { scan-assembler {brasl\t%r\d+,bar8} } } */
> +
> +/* Parameter E is passed in GPR 6 which is call-saved which prohibits
> +   sibling call optimization.  This must hold true also if the mode of the
> +   parameter is BLKmode.  */
> +
> +/* 4 byte */
> +
> +typedef struct
> +{
> +  char x;
> +  char y[3];
> +} t4;
> +
> +extern t4 e4;
> +
> +extern void bar4 (int a, int b, int c, int d, t4 e4);
> +
> +void foo4 (int a, int b, int c, int d)
> +{
> +  bar4 (a, b, c, d, e4);
> +}
> +
> +/* 8 byte */
> +
> +typedef struct
> +{
> +  short x;
> +  char y[6];
> +} t8;
> +
> +extern t8 e8;
> +
> +extern void bar8 (int a, int b, int c, int d, t8 e8);
> +
> +void foo8 (int a, int b, int c, int d)
> +{
> +  bar8 (a, b, c, d, e8);
> +}
> diff --git a/gcc/testsuite/gcc.target/s390/pr106355-2.c b/gcc/testsuite/gcc.target/s390/pr106355-2.c
> new file mode 100644
> index 00000000000..ddbdba5d278
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/pr106355-2.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile { target { s390-*-* } } } */
> +/* { dg-options "-foptimize-sibling-calls -mzarch" } */
> +/* { dg-final { scan-assembler {brasl\t%r\d+,bar} } } */
> +
> +/* This tests function s390_call_saved_register_used where
> +   GET_CODE (parm_rtx) == PARALLEL holds.  */
> +
> +#include "pr106355.h"
> diff --git a/gcc/testsuite/gcc.target/s390/pr106355-3.c b/gcc/testsuite/gcc.target/s390/pr106355-3.c
> new file mode 100644
> index 00000000000..39daea44fc4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/pr106355-3.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile { target { s390-*-* } } } */
> +/* { dg-options "-foptimize-sibling-calls -mesa" } */
> +/* { dg-final { scan-assembler {brasl\t%r\d+,bar} } } */
> +
> +/* This tests function s390_call_saved_register_used where
> +   REG_P (parm_rtx) and nregs == 2 holds.  */
> +
> +#include "pr106355.h"
> diff --git a/gcc/testsuite/gcc.target/s390/pr106355.h b/gcc/testsuite/gcc.target/s390/pr106355.h
> new file mode 100644
> index 00000000000..362908e5913
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/pr106355.h
> @@ -0,0 +1,18 @@
> +/* For the S/390 ABI parameter D is passed in GPR 5 and 6 and the latter is
> +   call-saved which prohibits sibling call optimization.  This must hold true
> +   also if the mode of the parameter is BLKmode.  */
> +
> +typedef struct
> +{
> +  short x;
> +  char y[6];
> +} t;
> +
> +extern t d;
> +
> +extern void bar (int a, int b, int c, t d);
> +
> +void foo (int a, int b, int c)
> +{
> +  bar (a, b, c, d);
> +}
> --
> 2.35.3
>

On Wed, Aug 17, 2022 at 01:50:45PM +0200, Stefan Schulze Frielinghaus wrote:
> For a parameter with BLKmode we cannot use REG_NREGS in order to
> determine the number of consecutive registers.  Streamlined this with
> the implementation of s390_function_arg.
> 
> Fix some indentation whitespace, too.
> 
> Assuming bootstrap and regtest are ok for mainline and gcc-{10,11,12},
> ok to install for all of those?

Meanwhile bootstrap and regtest ran successfully for all branches.

On 8/17/22 13:50, Stefan Schulze Frielinghaus wrote:
> For a parameter with BLKmode we cannot use REG_NREGS in order to
> determine the number of consecutive registers.  Streamlined this with
> the implementation of s390_function_arg.
> 
> Fix some indentation whitespace, too.
> 
> Assuming bootstrap and regtest are ok for mainline and gcc-{10,11,12},
> ok to install for all of those?
> 
> PR target/106355
> 
> gcc/ChangeLog:
> 
> 	* config/s390/s390.cc (s390_call_saved_register_used): For a
> 	parameter with BLKmode fix determining number of consecutive
> 	registers.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/s390/pr106355.h: Common code for new tests.
> 	* gcc.target/s390/pr106355-1.c: New test.
> 	* gcc.target/s390/pr106355-2.c: New test.
> 	* gcc.target/s390/pr106355-3.c: New test.

Ok for all those branches. Please check if the branches are currently open before committing. GCC 11
and 12 appear to be but I'm not sure if GCC 10 has been re-opened again. There should be a final
10.5 release some day though.

Thanks!

Andreas