RE: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]

[Kyrylo Tkachov via Gcc-patches]

> -----Original Message-----
> From: Jakub Jelinek <ja...@redhat.com>
> Sent: 19 November 2020 18:57
> To: Richard Earnshaw <richard.earns...@arm.com>; Ramana
> Radhakrishnan <ramana.radhakrish...@arm.com>; Kyrylo Tkachov
> <kyrylo.tkac...@arm.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: [PATCH] arm: Fix up neon_vector_mem_operand [PR97528]
> 
> Hi!
> 
> The documentation for POST_MODIFY says:
>    Currently, the compiler can only handle second operands of the
>    form (plus (reg) (reg)) and (plus (reg) (const_int)), where
>    the first operand of the PLUS has to be the same register as
>    the first operand of the *_MODIFY.
> The following testcase ICEs, because combine just attempts to simplify
> things and ends up with
> (post_modify (reg1) (plus (mult (reg2) (const_int 4)) (reg1))
> but the target predicates accept it, because they only verify
> that POST_MODIFY's second operand is PLUS and the second operand
> of the PLUS is a REG.
> 
> The following patch fixes this by performing further verification that
> the POST_MODIFY is in the form it should be.
> 
> Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk
> and release branches after a while?

Ok.
Thanks,
Kyrill

> 
> 2020-11-19  Jakub Jelinek  <ja...@redhat.com>
> 
>       PR target/97528
>       * config/arm/arm.c (neon_vector_mem_operand): For
> POST_MODIFY, require
>       first POST_MODIFY operand is a REG and is equal to the first operand
>       of PLUS.
> 
>       * gcc.target/arm/pr97528.c: New test.
> 
> --- gcc/config/arm/arm.c.jj   2020-11-13 19:00:46.729620560 +0100
> +++ gcc/config/arm/arm.c      2020-11-18 17:05:44.656867343 +0100
> @@ -13429,7 +13429,9 @@ neon_vector_mem_operand (rtx op, int typ
>    /* Allow post-increment by register for VLDn */
>    if (type == 2 && GET_CODE (ind) == POST_MODIFY
>        && GET_CODE (XEXP (ind, 1)) == PLUS
> -      && REG_P (XEXP (XEXP (ind, 1), 1)))
> +      && REG_P (XEXP (XEXP (ind, 1), 1))
> +      && REG_P (XEXP (ind, 0))
> +      && rtx_equal_p (XEXP (ind, 0), XEXP (XEXP (ind, 1), 0)))
>       return true;
> 
>    /* Match:
> --- gcc/testsuite/gcc.target/arm/pr97528.c.jj 2020-11-18
> 17:09:58.195053288 +0100
> +++ gcc/testsuite/gcc.target/arm/pr97528.c    2020-11-18
> 17:09:47.839168237 +0100
> @@ -0,0 +1,28 @@
> +/* PR target/97528 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm_neon_ok } */
> +/* { dg-options "-O1" }  */
> +/* { dg-add-options arm_neon } */
> +
> +#include <arm_neon.h>
> +
> +typedef __simd64_int16_t T;
> +typedef __simd64_uint16_t U;
> +unsigned short c;
> +int d;
> +U e;
> +
> +void
> +foo (void)
> +{
> +  unsigned short *dst = &c;
> +  int g = d, b = 4;
> +  U dc = e;
> +  for (int h = 0; h < b; h++)
> +    {
> +      unsigned short *i = dst;
> +      U j = dc;
> +      vst1_s16 ((int16_t *) i, (T) j);
> +      dst += g;
> +    }
> +}
> 
> 
>       Jakub