Fix expand_expr_real_1 handling of BLKmode bitfield references

Message ID 87bmcxabec.fsf@linaro.org
State New
Headers show
Series
  • Fix expand_expr_real_1 handling of BLKmode bitfield references
Related show

Commit Message

Richard Sandiford May 30, 2018, 6:46 a.m.
The handling of bitfield references in expand_expr_real_1 includes:

            machine_mode ext_mode = mode;

            if (ext_mode == BLKmode
                && ! (target != 0 && MEM_P (op0)
                      && MEM_P (target)
                      && multiple_p (bitpos, BITS_PER_UNIT)))
              ext_mode = int_mode_for_size (bitsize, 1).else_blk ();

            if (ext_mode == BLKmode)
              {
                [...]
                gcc_assert (MEM_P (op0)

Here "mode" is the TYPE_MODE of the result, so when mode == BLKmode,
the target must be a MEM if nonnull, since no other rtl objects can
have BLKmode.  But there's no guarantee that the source value op0 is also
BLKmode and thus also a MEM: we can reach the assert for any source if
the bitsize being extracted is larger than the largest integer mode
(or larger than MAX_FIXED_MODE_SIZE).

This triggered for SVE with -msve-vector-bits=512, where we could
sometimes try to extract a BLKmode value from a 512-bit vector,
and where int_mode_for_size would rightly fail for large bitsizes.

The patch reuses the existing:

	/* Otherwise, if this is a constant or the object is not in memory
	   and need be, put it there.  */
	else if (CONSTANT_P (op0) || (!MEM_P (op0) && must_force_mem))
	  {
	    memloc = assign_temp (TREE_TYPE (tem), 1, 1);
	    emit_move_insn (memloc, op0);
	    op0 = memloc;
	    clear_mem_expr = true;
	  }

to handle this case.

Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf and
x86_64-linux-gnu.  OK to install?

Richard


2018-05-30  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	* expr.c (expand_expr_real_1): Force the operand into memory if
	its TYPE_MODE is BLKmode and if there is no integer mode for
	the number of bits being extracted.

gcc/testsuite/
	* gcc.target/aarch64/sve/extract_5.c: New test.

Comments

Richard Biener June 4, 2018, 1:09 p.m. | #1
On Wed, May 30, 2018 at 8:46 AM Richard Sandiford
<richard.sandiford@linaro.org> wrote:>
> The handling of bitfield references in expand_expr_real_1 includes:

>

>             machine_mode ext_mode = mode;

>

>             if (ext_mode == BLKmode

>                 && ! (target != 0 && MEM_P (op0)

>                       && MEM_P (target)

>                       && multiple_p (bitpos, BITS_PER_UNIT)))

>               ext_mode = int_mode_for_size (bitsize, 1).else_blk ();

>

>             if (ext_mode == BLKmode)

>               {

>                 [...]

>                 gcc_assert (MEM_P (op0)

>

> Here "mode" is the TYPE_MODE of the result, so when mode == BLKmode,

> the target must be a MEM if nonnull, since no other rtl objects can

> have BLKmode.  But there's no guarantee that the source value op0 is also

> BLKmode and thus also a MEM: we can reach the assert for any source if

> the bitsize being extracted is larger than the largest integer mode

> (or larger than MAX_FIXED_MODE_SIZE).

>

> This triggered for SVE with -msve-vector-bits=512, where we could

> sometimes try to extract a BLKmode value from a 512-bit vector,

> and where int_mode_for_size would rightly fail for large bitsizes.

>

> The patch reuses the existing:

>

>         /* Otherwise, if this is a constant or the object is not in memory

>            and need be, put it there.  */

>         else if (CONSTANT_P (op0) || (!MEM_P (op0) && must_force_mem))

>           {

>             memloc = assign_temp (TREE_TYPE (tem), 1, 1);

>             emit_move_insn (memloc, op0);

>             op0 = memloc;

>             clear_mem_expr = true;

>           }

>

> to handle this case.

>

> Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf and

> x86_64-linux-gnu.  OK to install?


Ok.

Richard.

> Richard

>

>

> 2018-05-30  Richard Sandiford  <richard.sandiford@linaro.org>

>

> gcc/

>         * expr.c (expand_expr_real_1): Force the operand into memory if

>         its TYPE_MODE is BLKmode and if there is no integer mode for

>         the number of bits being extracted.

>

> gcc/testsuite/

>         * gcc.target/aarch64/sve/extract_5.c: New test.

>

> Index: gcc/expr.c

> ===================================================================

> --- gcc/expr.c  2018-05-30 07:33:11.652009370 +0100

> +++ gcc/expr.c  2018-05-30 07:44:31.856060230 +0100

> @@ -10582,6 +10582,8 @@ expand_expr_real_1 (tree exp, rtx target

>            to a larger size.  */

>         must_force_mem = (offset

>                           || mode1 == BLKmode

> +                         || (mode == BLKmode

> +                             && !int_mode_for_size (bitsize, 1).exists ())

>                           || maybe_gt (bitpos + bitsize,

>                                        GET_MODE_BITSIZE (mode2)));

>

> Index: gcc/testsuite/gcc.target/aarch64/sve/extract_5.c

> ===================================================================

> --- /dev/null   2018-04-20 16:19:46.369131350 +0100

> +++ gcc/testsuite/gcc.target/aarch64/sve/extract_5.c    2018-05-30 07:44:31.857060190 +0100

> @@ -0,0 +1,71 @@

> +/* Originally from gcc.dg/vect/vect-alias-check-10.c.  */

> +/* { dg-do compile } */

> +/* { dg-options "-O2 -ftree-vectorize -msve-vector-bits=512" } */

> +

> +#define N 87

> +#define M 6

> +

> +typedef signed char sc;

> +typedef unsigned char uc;

> +typedef signed short ss;

> +typedef unsigned short us;

> +typedef int si;

> +typedef unsigned int ui;

> +typedef signed long long sll;

> +typedef unsigned long long ull;

> +

> +#define FOR_EACH_TYPE(M) \

> +  M (sc) M (uc) \

> +  M (ss) M (us) \

> +  M (si) M (ui) \

> +  M (sll) M (ull) \

> +  M (float) M (double)

> +

> +#define TEST_VALUE(I) ((I) * 5 / 2)

> +

> +#define ADD_TEST(TYPE)                         \

> +  void __attribute__((noinline, noclone))      \

> +  test_##TYPE (TYPE *a, int step)              \

> +  {                                            \

> +    for (int i = 0; i < N; ++i)                        \

> +      {                                                \

> +       a[i * step + 0] = a[i * step + 0] + 1;  \

> +       a[i * step + 1] = a[i * step + 1] + 2;  \

> +       a[i * step + 2] = a[i * step + 2] + 4;  \

> +       a[i * step + 3] = a[i * step + 3] + 8;  \

> +      }                                                \

> +  }                                            \

> +  void __attribute__((noinline, noclone))      \

> +  ref_##TYPE (TYPE *a, int step)               \

> +  {                                            \

> +    for (int i = 0; i < N; ++i)                        \

> +      {                                                \

> +       a[i * step + 0] = a[i * step + 0] + 1;  \

> +       a[i * step + 1] = a[i * step + 1] + 2;  \

> +       a[i * step + 2] = a[i * step + 2] + 4;  \

> +       a[i * step + 3] = a[i * step + 3] + 8;  \

> +       asm volatile ("");                      \

> +      }                                                \

> +  }

> +

> +#define DO_TEST(TYPE)                                  \

> +  for (int j = -M; j <= M; ++j)                                \

> +    {                                                  \

> +      TYPE a[N * M], b[N * M];                         \

> +      for (int i = 0; i < N * M; ++i)                  \

> +       a[i] = b[i] = TEST_VALUE (i);                   \

> +      int offset = (j < 0 ? N * M - 4 : 0);            \

> +      test_##TYPE (a + offset, j);                     \

> +      ref_##TYPE (b + offset, j);                      \

> +      if (__builtin_memcmp (a, b, sizeof (a)) != 0)    \

> +       __builtin_abort ();                             \

> +    }

> +

> +FOR_EACH_TYPE (ADD_TEST)

> +

> +int

> +main (void)

> +{

> +  FOR_EACH_TYPE (DO_TEST)

> +  return 0;

> +}

Patch

Index: gcc/expr.c
===================================================================
--- gcc/expr.c	2018-05-30 07:33:11.652009370 +0100
+++ gcc/expr.c	2018-05-30 07:44:31.856060230 +0100
@@ -10582,6 +10582,8 @@  expand_expr_real_1 (tree exp, rtx target
 	   to a larger size.  */
 	must_force_mem = (offset
 			  || mode1 == BLKmode
+			  || (mode == BLKmode
+			      && !int_mode_for_size (bitsize, 1).exists ())
 			  || maybe_gt (bitpos + bitsize,
 				       GET_MODE_BITSIZE (mode2)));
 
Index: gcc/testsuite/gcc.target/aarch64/sve/extract_5.c
===================================================================
--- /dev/null	2018-04-20 16:19:46.369131350 +0100
+++ gcc/testsuite/gcc.target/aarch64/sve/extract_5.c	2018-05-30 07:44:31.857060190 +0100
@@ -0,0 +1,71 @@ 
+/* Originally from gcc.dg/vect/vect-alias-check-10.c.  */
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-vectorize -msve-vector-bits=512" } */
+
+#define N 87
+#define M 6
+
+typedef signed char sc;
+typedef unsigned char uc;
+typedef signed short ss;
+typedef unsigned short us;
+typedef int si;
+typedef unsigned int ui;
+typedef signed long long sll;
+typedef unsigned long long ull;
+
+#define FOR_EACH_TYPE(M) \
+  M (sc) M (uc) \
+  M (ss) M (us) \
+  M (si) M (ui) \
+  M (sll) M (ull) \
+  M (float) M (double)
+
+#define TEST_VALUE(I) ((I) * 5 / 2)
+
+#define ADD_TEST(TYPE)				\
+  void __attribute__((noinline, noclone))	\
+  test_##TYPE (TYPE *a, int step)		\
+  {						\
+    for (int i = 0; i < N; ++i)			\
+      {						\
+	a[i * step + 0] = a[i * step + 0] + 1;	\
+	a[i * step + 1] = a[i * step + 1] + 2;	\
+	a[i * step + 2] = a[i * step + 2] + 4;	\
+	a[i * step + 3] = a[i * step + 3] + 8;	\
+      }						\
+  }						\
+  void __attribute__((noinline, noclone))	\
+  ref_##TYPE (TYPE *a, int step)		\
+  {						\
+    for (int i = 0; i < N; ++i)			\
+      {						\
+	a[i * step + 0] = a[i * step + 0] + 1;	\
+	a[i * step + 1] = a[i * step + 1] + 2;	\
+	a[i * step + 2] = a[i * step + 2] + 4;	\
+	a[i * step + 3] = a[i * step + 3] + 8;	\
+	asm volatile ("");			\
+      }						\
+  }
+
+#define DO_TEST(TYPE)					\
+  for (int j = -M; j <= M; ++j)				\
+    {							\
+      TYPE a[N * M], b[N * M];				\
+      for (int i = 0; i < N * M; ++i)			\
+	a[i] = b[i] = TEST_VALUE (i);			\
+      int offset = (j < 0 ? N * M - 4 : 0);		\
+      test_##TYPE (a + offset, j);			\
+      ref_##TYPE (b + offset, j);			\
+      if (__builtin_memcmp (a, b, sizeof (a)) != 0)	\
+	__builtin_abort ();				\
+    }
+
+FOR_EACH_TYPE (ADD_TEST)
+
+int
+main (void)
+{
+  FOR_EACH_TYPE (DO_TEST)
+  return 0;
+}