Hi Mike,

on 2024/1/6 06:18, Michael Meissner wrote:
> In looking at support for load vector pair and store vector pair for the
> PowerPC in GCC, I noticed that we were missing a print_operand output modifier
> if you are dealing with vector pairs to print the 2nd register in the vector
> pair.
> 
> If the instruction inside of the asm used the Altivec encoding, then we could
> use the %L<n> modifier:

It seems there is no Power specific documentation on operand modifiers like this
"%L"?

> 
>       __vector_pair *p, *q, *r;
>       // ...
>       __asm__ ("vaddudm %0,%1,%2\n\tvaddudm %L0,%L1,%L2"
>                : "=v" (*p)
>                : "v" (*q), "v" (*r));
> 
> Likewise if we know the value to be in a tradiational FPR register, %L<n> will
> work for instructions that use the VSX encoding:
> 
>       __vector_pair *p, *q, *r;
>       // ...
>       __asm__ ("xvadddp %x0,%x1,%x2\n\txvadddp %L0,%L1,%L2"
>                : "=f" (*p)
>                : "f" (*q), "f" (*r));
> 
> But if have a value that is in a traditional Altivec register, and the
> instruction uses the VSX encoding, %L<n> will a value between 0 and 31, when 
> it
> should give a value between 32 and 63.
> 
> This patch adds %S<n> that acts like %x<n>, except that it adds 1 to the
> register number.

Excepting for Peter's comments, since the existing "%L" has different handlings
on REG_P and MEM_P:

    case 'L':
      /* Write second word of DImode or DFmode reference.  Works on register
         or non-indexed memory only.  */
      if (REG_P (x))
        fputs (reg_names[REGNO (x) + 1], file);
      else if (MEM_P (x))
        ...

, maybe we can extend the existing '%X' for this similarly (as it's capital of
%x so easier to remember and it's only used for MEM_P now) instead of 
introducing
a new "%S".  But one argument can be a new character is more clear.  Thoughts?

BR,
Kewen

> 
> I have tested this on power10 and power9 little endian systems and on a power9
> big endian system.  There were no regressions in the patch.  Can I apply it to
> the trunk?
> 
> It would be nice if I could apply it to the open branches.  Can I backport it
> after a burn-in period?
> 
> 2024-01-04  Michael Meissner  <meiss...@linux.ibm.com>
> 
> gcc/
> 
>       PR target/112886
>       * config/rs6000/rs6000.cc (print_operand): Add %S<n> output modifier.
>       * doc/md.texi (Modifiers): Mention %S can be used like %x.
> 
> gcc/testsuite/
> 
>       PR target/112886
>       * /gcc.target/powerpc/pr112886.c: New test.
> ---
>  gcc/config/rs6000/rs6000.cc                 | 10 +++++++---
>  gcc/doc/md.texi                             |  5 +++--
>  gcc/testsuite/gcc.target/powerpc/pr112886.c | 19 +++++++++++++++++++
>  3 files changed, 29 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/pr112886.c
> 
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index 5a7e00b03d1..ba89377c9ec 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -14504,13 +14504,17 @@ print_operand (FILE *file, rtx x, int code)
>       print_operand (file, x, 0);
>        return;
>  
> +    case 'S':
>      case 'x':
> -      /* X is a FPR or Altivec register used in a VSX context.  */
> +      /* X is a FPR or Altivec register used in a VSX context.  %x<n> prints
> +      the VSX register number, %S<n> prints the 2nd register number for
> +      vector pair, decimal 128-bit floating and IBM 128-bit binary floating
> +      values.  */
>        if (!REG_P (x) || !VSX_REGNO_P (REGNO (x)))
> -     output_operand_lossage ("invalid %%x value");
> +     output_operand_lossage ("invalid %%%c value", (code == 'S' ? 'S' : 
> 'x'));
>        else
>       {
> -       int reg = REGNO (x);
> +       int reg = REGNO (x) + (code == 'S' ? 1 : 0);
>         int vsx_reg = (FP_REGNO_P (reg)
>                        ? reg - 32
>                        : reg - FIRST_ALTIVEC_REGNO + 32);
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index 47a87d6ceec..53ec957cb23 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -3386,8 +3386,9 @@ A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  
> This is either an
>  FPR (@code{vs0}@dots{}@code{vs31} are @code{f0}@dots{}@code{f31}) or a VR
>  (@code{vs32}@dots{}@code{vs63} are @code{v0}@dots{}@code{v31}).
>  
> -When using @code{wa}, you should use the @code{%x} output modifier, so that
> -the correct register number is printed.  For example:
> +When using @code{wa}, you should use either the @code{%x} or @code{%S}
> +output modifier, so that the correct register number is printed.  For
> +example:
>  
>  @smallexample
>  asm ("xvadddp %x0,%x1,%x2"
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr112886.c 
> b/gcc/testsuite/gcc.target/powerpc/pr112886.c
> new file mode 100644
> index 00000000000..07196bdc220
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr112886.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-mdejagnu-cpu=power10 -O2" } */
> +
> +/* PR target/112886: Test that print_operand %S<n> gives the correct register
> +   number for VSX registers (i.e. if the register is an Altivec register, the
> +   register number is 32..63 instead of 0..31.  */
> +
> +void
> +test (__vector_pair *p, __vector_pair *q, __vector_pair *r)
> +{
> +  __asm__ ("xvadddp %x0,%x1,%x2\n\txvadddp %S0,%S1,%S2"
> +        : "=v" (*p)
> +        : "v" (*q), "v" (*r));
> +}
> +
> +/* { dg-final { scan-assembler-times {\mxvadddp 
> (3[2-9]|[45][0-9]|6[0-3]),(3[2-9]|[45][0-9]|6[0-3]),(3[2-9]|[45][0-9]|6[0-3])\M}
>  2 } } */
> +/* { dg-final { scan-assembler-times {\mlxvp\M}  2 } } */
> +/* { dg-final { scan-assembler-times {\mstxvp\M} 1 } } */

Reply via email to