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 } } */
