Mark constant-sized objects as addressable if they have poly-int accesses

If SVE code is written for a specific vector length, it might load from
or store to fixed-sized objects.  This needs to work even without
-msve-vector-bits=N (which should never be needed for correctness).

There's no way of handling a direct poly-int sized reference to a
fixed-size register; it would have to go via memory.  And in that
case it's more efficient to mark the fixed-size object as
addressable from the outset, like we do for array references
with non-constant indices.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


2019-11-08  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* cfgexpand.c (discover_nonconstant_array_refs_r): If an access
	with POLY_INT_CST size is made to a fixed-size object, force the
	object to live in memory.

gcc/testsuite/
	* gcc.target/aarch64/sve/acle/general/deref_1.c: New test.

On Fri, Nov 8, 2019 at 10:40 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> If SVE code is written for a specific vector length, it might load from
> or store to fixed-sized objects.  This needs to work even without
> -msve-vector-bits=N (which should never be needed for correctness).
>
> There's no way of handling a direct poly-int sized reference to a
> fixed-size register; it would have to go via memory.  And in that
> case it's more efficient to mark the fixed-size object as
> addressable from the outset, like we do for array references
> with non-constant indices.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Hmm, shouldn't you somehow avoid walking subtrees again like
the array-ref cases?  Do "intermediate" types really matter here?
Thus if you have BIT_FIELDREF <VIEW_CONVERT <type-with-poly-size,
fixed-size-decl>,
select first element > do you really need it addressable?

It seems to me you want to check it only on the actual reference type.

Richard.

> Richard
>
>
> 2019-11-08  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
>         * cfgexpand.c (discover_nonconstant_array_refs_r): If an access
>         with POLY_INT_CST size is made to a fixed-size object, force the
>         object to live in memory.
>
> gcc/testsuite/
>         * gcc.target/aarch64/sve/acle/general/deref_1.c: New test.
>
> Index: gcc/cfgexpand.c
> ===================================================================
> --- gcc/cfgexpand.c     2019-10-01 09:55:35.062089236 +0100
> +++ gcc/cfgexpand.c     2019-11-08 09:39:13.105130902 +0000
> @@ -6106,6 +6106,20 @@ discover_nonconstant_array_refs_r (tree
>  {
>    tree t = *tp;
>
> +  /* References of size POLY_INT_CST to a fixed-size object must go
> +     through memory.  It's more efficient to force that here than
> +     to create temporary slots on the fly.  */
> +  if (TYPE_SIZE (TREE_TYPE (t))
> +      && POLY_INT_CST_P (TYPE_SIZE (TREE_TYPE (t))))
> +    {
> +      t = get_base_address (t);
> +      if (t
> +         && DECL_P (t)
> +         && DECL_MODE (t) != BLKmode
> +         && GET_MODE_BITSIZE (DECL_MODE (t)).is_constant ())
> +       TREE_ADDRESSABLE (t) = 1;
> +    }
> +
>    if (IS_TYPE_OR_DECL_P (t))
>      *walk_subtrees = 0;
>    else if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF)
> Index: gcc/testsuite/gcc.target/aarch64/sve/acle/general/deref_1.c
> ===================================================================
> --- /dev/null   2019-09-17 11:41:18.176664108 +0100
> +++ gcc/testsuite/gcc.target/aarch64/sve/acle/general/deref_1.c 2019-11-08 09:39:13.105130902 +0000
> @@ -0,0 +1,13 @@
> +/* { dg-options "-O2" } */
> +
> +#include <arm_sve.h>
> +
> +uint64_t
> +f (int32_t *x, int32_t *y)
> +{
> +  union { uint64_t x; char c[8]; } u;
> +  svbool_t pg = svptrue_b32 ();
> +  *(svbool_t *)&u.c[0] = svcmpeq (pg, svld1 (pg, x), 0);
> +  *(svbool_t *)&u.c[4] = svcmpeq (pg, svld1 (pg, y), 1);
> +  return u.x;
> +}

[finally getting back to this]

Richard Biener <richard.guenther@gmail.com> writes:
> On Fri, Nov 8, 2019 at 10:40 AM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> If SVE code is written for a specific vector length, it might load from
>> or store to fixed-sized objects.  This needs to work even without
>> -msve-vector-bits=N (which should never be needed for correctness).
>>
>> There's no way of handling a direct poly-int sized reference to a
>> fixed-size register; it would have to go via memory.  And in that
>> case it's more efficient to mark the fixed-size object as
>> addressable from the outset, like we do for array references
>> with non-constant indices.
>>
>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>
> Hmm, shouldn't you somehow avoid walking subtrees again like
> the array-ref cases?  Do "intermediate" types really matter here?
> Thus if you have BIT_FIELDREF <VIEW_CONVERT <type-with-poly-size,
> fixed-size-decl>,
> select first element > do you really need it addressable?
>
> It seems to me you want to check it only on the actual reference type.

Yeah, I guess it was overly general.  I don't think it can happen
for anything other than the MEM_REF/TARGET_MEM_REF itself.

How about this version?  Tested as before.

Richard


2019-12-02  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* cfgexpand.c (discover_nonconstant_array_refs_r): If an access
	with POLY_INT_CST size is made to a fixed-size object, force the
	object to live in memory.

gcc/testsuite/
	* gcc.target/aarch64/sve/acle/general/deref_1.c: New test.

Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	2019-11-13 08:42:44.901373255 +0000
+++ gcc/cfgexpand.c	2019-12-02 16:27:45.157295080 +0000
@@ -6133,6 +6133,21 @@ discover_nonconstant_array_refs_r (tree
 
       *walk_subtrees = 0;
     }
+  /* References of size POLY_INT_CST to a fixed-size object must go
+     through memory.  It's more efficient to force that here than
+     to create temporary slots on the fly.  */
+  else if ((TREE_CODE (t) == MEM_REF || TREE_CODE (t) == TARGET_MEM_REF)
+	   && TYPE_SIZE (TREE_TYPE (t))
+	   && POLY_INT_CST_P (TYPE_SIZE (TREE_TYPE (t))))
+    {
+      tree base = get_base_address (t);
+      if (base
+	  && DECL_P (base)
+	  && DECL_MODE (base) != BLKmode
+	  && GET_MODE_SIZE (DECL_MODE (base)).is_constant ())
+	TREE_ADDRESSABLE (base) = 1;
+      *walk_subtrees = 0;
+    }
 
   return NULL_TREE;
 }
Index: gcc/testsuite/gcc.target/aarch64/sve/acle/general/deref_1.c
===================================================================
--- /dev/null	2019-09-17 11:41:18.176664108 +0100
+++ gcc/testsuite/gcc.target/aarch64/sve/acle/general/deref_1.c	2019-12-02 16:27:45.165295026 +0000
@@ -0,0 +1,25 @@
+/* { dg-options "-O2" } */
+
+#include <arm_sve.h>
+
+uint64_t
+f1 (int32_t *x, int32_t *y)
+{
+  union { uint64_t x; char c[8]; } u;
+  svbool_t pg = svptrue_b32 ();
+  *(svbool_t *)&u.c[0] = svcmpeq (pg, svld1 (pg, x), 0);
+  *(svbool_t *)&u.c[4] = svcmpeq (pg, svld1 (pg, y), 1);
+  return u.x;
+}
+
+typedef unsigned int v4si __attribute__((vector_size(16)));
+
+/* The aliasing is somewhat dubious here, but it must compile.  */
+
+v4si
+f2 (void)
+{
+  v4si res;
+  *(svuint32_t *) &res = svindex_u32 (0, 1);
+  return res;
+}

On Mon, Dec 2, 2019 at 5:30 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> [finally getting back to this]
>
> Richard Biener <richard.guenther@gmail.com> writes:
> > On Fri, Nov 8, 2019 at 10:40 AM Richard Sandiford
> > <richard.sandiford@arm.com> wrote:
> >>
> >> If SVE code is written for a specific vector length, it might load from
> >> or store to fixed-sized objects.  This needs to work even without
> >> -msve-vector-bits=N (which should never be needed for correctness).
> >>
> >> There's no way of handling a direct poly-int sized reference to a
> >> fixed-size register; it would have to go via memory.  And in that
> >> case it's more efficient to mark the fixed-size object as
> >> addressable from the outset, like we do for array references
> >> with non-constant indices.
> >>
> >> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
> >
> > Hmm, shouldn't you somehow avoid walking subtrees again like
> > the array-ref cases?  Do "intermediate" types really matter here?
> > Thus if you have BIT_FIELDREF <VIEW_CONVERT <type-with-poly-size,
> > fixed-size-decl>,
> > select first element > do you really need it addressable?
> >
> > It seems to me you want to check it only on the actual reference type.
>
> Yeah, I guess it was overly general.  I don't think it can happen
> for anything other than the MEM_REF/TARGET_MEM_REF itself.
>
> How about this version?  Tested as before.

OK.

Thanks,
Richard.

> Richard
>
>
> 2019-12-02  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
>         * cfgexpand.c (discover_nonconstant_array_refs_r): If an access
>         with POLY_INT_CST size is made to a fixed-size object, force the
>         object to live in memory.
>
> gcc/testsuite/
>         * gcc.target/aarch64/sve/acle/general/deref_1.c: New test.
>
> Index: gcc/cfgexpand.c
> ===================================================================
> --- gcc/cfgexpand.c     2019-11-13 08:42:44.901373255 +0000
> +++ gcc/cfgexpand.c     2019-12-02 16:27:45.157295080 +0000
> @@ -6133,6 +6133,21 @@ discover_nonconstant_array_refs_r (tree
>
>        *walk_subtrees = 0;
>      }
> +  /* References of size POLY_INT_CST to a fixed-size object must go
> +     through memory.  It's more efficient to force that here than
> +     to create temporary slots on the fly.  */
> +  else if ((TREE_CODE (t) == MEM_REF || TREE_CODE (t) == TARGET_MEM_REF)
> +          && TYPE_SIZE (TREE_TYPE (t))
> +          && POLY_INT_CST_P (TYPE_SIZE (TREE_TYPE (t))))
> +    {
> +      tree base = get_base_address (t);
> +      if (base
> +         && DECL_P (base)
> +         && DECL_MODE (base) != BLKmode
> +         && GET_MODE_SIZE (DECL_MODE (base)).is_constant ())
> +       TREE_ADDRESSABLE (base) = 1;
> +      *walk_subtrees = 0;
> +    }
>
>    return NULL_TREE;
>  }
> Index: gcc/testsuite/gcc.target/aarch64/sve/acle/general/deref_1.c
> ===================================================================
> --- /dev/null   2019-09-17 11:41:18.176664108 +0100
> +++ gcc/testsuite/gcc.target/aarch64/sve/acle/general/deref_1.c 2019-12-02 16:27:45.165295026 +0000
> @@ -0,0 +1,25 @@
> +/* { dg-options "-O2" } */
> +
> +#include <arm_sve.h>
> +
> +uint64_t
> +f1 (int32_t *x, int32_t *y)
> +{
> +  union { uint64_t x; char c[8]; } u;
> +  svbool_t pg = svptrue_b32 ();
> +  *(svbool_t *)&u.c[0] = svcmpeq (pg, svld1 (pg, x), 0);
> +  *(svbool_t *)&u.c[4] = svcmpeq (pg, svld1 (pg, y), 1);
> +  return u.x;
> +}
> +
> +typedef unsigned int v4si __attribute__((vector_size(16)));
> +
> +/* The aliasing is somewhat dubious here, but it must compile.  */
> +
> +v4si
> +f2 (void)
> +{
> +  v4si res;
> +  *(svuint32_t *) &res = svindex_u32 (0, 1);
> +  return res;
> +}