Re: [PATCH 07/11] aarch64: Fix up printing of ldp/stp with -msve-vector-bits=128

[Richard Sandiford]

Alex Coplan <alex.cop...@arm.com> writes:
> Later patches allow using SVE modes in ldp/stp with -msve-vector-bits=128,
> so we need to make sure that we don't use SVE addressing modes when
> printing the address for the ldp/stp.
>
> This patch does that.
>
> Bootstrapped/regtested as a series on aarch64-linux-gnu, OK for trunk?
>
> gcc/ChangeLog:
>
>       * config/aarch64/aarch64.cc (aarch64_print_address_internal): Handle SVE
>       modes when printing ldp/stp addresses.
> ---
>  gcc/config/aarch64/aarch64.cc | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index abd029887e5..4820fac67a1 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -12661,6 +12661,9 @@ aarch64_print_address_internal (FILE *f, machine_mode 
> mode, rtx x,
>        return false;
>      }
>  
> +  const bool load_store_pair_p = (type == ADDR_QUERY_LDP_STP
> +                               || type == ADDR_QUERY_LDP_STP_N);
> +
>    if (aarch64_classify_address (&addr, x, mode, true, type))
>      switch (addr.type)
>        {
> @@ -12672,7 +12675,15 @@ aarch64_print_address_internal (FILE *f, 
> machine_mode mode, rtx x,
>         }
>  
>       vec_flags = aarch64_classify_vector_mode (mode);
> -     if (vec_flags & VEC_ANY_SVE)
> +     if ((vec_flags & VEC_ANY_SVE)
> +         && load_store_pair_p
> +         && !addr.const_offset.is_constant ())
> +       {
> +         output_operand_lossage ("poly offset in ldp/stp address");
> +         return false;
> +       }
> +
> +     if ((vec_flags & VEC_ANY_SVE) && !load_store_pair_p)
>         {
>           HOST_WIDE_INT vnum
>             = exact_div (addr.const_offset,

It should be possible to exercise the error form of the output_operand_lossage
with a test like:

#include <arm_sve.h>

void f(int32_t *ptr) {
  asm volatile ("%z0" :: "m" (*(svint32_t*)(ptr + svcntw())));
}

But the "poly offset" won't mean anything to users.

It's up to the caller to report the error on a false return, so I think
this code should just return with its own error.

Also, IMO it'd be neater to do the tests the other way around:


        if ((vec_flags & VEC_ANY_SVE) && !load_store_pair_p)
          ...

        if (!CONST_INT_P (addr.offset))
          return false;

        asm_fprintf (f, "[%s, %wd]", reg_names[REGNO (addr.base)],
                     INTVAL (addr.offset));
        return true;

OK with those changes, and with the test above if it works.

Thanks,
Richard