[PATCH] Retain TYPE_MODE more often for BIT_FIELD_REFs in get_inner_referece

[Richard Biener]

On Thu, 26 Sep 2019, Eric Botcazou wrote:

> > I see.  So I misremember seeing aggregate typed BIT_FIELD_REFs
> > (that was probably VIEW_CONVERTs then...).  Still the GIMPLE verifier
> > only has
> > 
> >           else if (!INTEGRAL_TYPE_P (TREE_TYPE (expr))
> >                    && TYPE_MODE (TREE_TYPE (expr)) != BLKmode
> >                    && maybe_ne (GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE
> > (expr))),
> >                                 size))
> >             {
> >               error ("mode size of non-integral result does not "
> >                      "match field size of %qs",
> >                      code_name);
> >               return true;
> >             }
> > 
> > it doesn't verify that besides integral typed expressions only
> > vector typed expressions are allowed.
> 
> I think that the !INTEGRAL_TYPE_P is simply the opposite of the first case:
> 
> 	  if (INTEGRAL_TYPE_P (TREE_TYPE (t))
> 	      && maybe_ne (TYPE_PRECISION (TREE_TYPE (t)), size))
> 	    {
> 	      error ("integral result type precision does not match "
> 		     "field size of BIT_FIELD_REF");
> 	      return t;
> 	    }
> 
> > Anyhow - the original patch succeeded bootstrapping and testing.
> > The way I proposed it:
> > 
> >       /* For vector types, with the correct size of access, use the mode
> > of
> >          inner type.  */
> >       if (((TREE_CODE (TREE_TYPE (TREE_OPERAND (exp, 0))) == VECTOR_TYPE
> >             && TREE_TYPE (exp) == TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp,
> > 0))))
> > 
> >            || !INTEGRAL_TYPE_P (TREE_TYPE (exp)))
> > 
> >           && tree_int_cst_equal (size_tree, TYPE_SIZE (TREE_TYPE (exp))))
> >         mode = TYPE_MODE (TREE_TYPE (exp));
> > 
> > matches in sofar that we only restrict integer types (not modes) and
> > for integer types allow extracts from vectors (the preexisting
> > check for a matching component type is a bit too strict I guess).
> 
> IMO if the !VECTOR_TYPE case cannot be covered, changes in this delicate area 
> ought to be restricted to VECTOR_TYPE.

OK, so I'm testing the following then, simply adding the VECTOR_TYPE_P
case in addition to the existing one plus adjusting the comment
accordingly.

Bootstrap / regtest running on x86_64-unknown-linux-gnu, OK if that 
passes?

Thanks,
Richard.

2019-09-26  Richard Biener  <rguenther@suse.de>

	PR middle-end/91897
	* expr.c (get_inner_reference): For BIT_FIELD_REF with
	vector type retain the original mode.

	* gcc.target/i386/pr91897.c: New testcase.

Index: gcc/testsuite/gcc.target/i386/pr91897.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr91897.c	(nonexistent)
+++ gcc/testsuite/gcc.target/i386/pr91897.c	(working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mavx" } */
+
+typedef double Double16 __attribute__((vector_size(8*16)));
+
+void mult(Double16 *res, const Double16 *v1, const Double16 *v2)
+{
+  *res = *v1 * *v2;
+}
+
+/* We want 4 ymm loads and 4 ymm stores.  */
+/* { dg-final { scan-assembler-times "movapd" 8 } } */
Index: gcc/expr.c
===================================================================
--- gcc/expr.c	(revision 276147)
+++ gcc/expr.c	(working copy)
@@ -7230,12 +7230,13 @@ get_inner_reference (tree exp, poly_int6
       *punsignedp = (! INTEGRAL_TYPE_P (TREE_TYPE (exp))
 		     || TYPE_UNSIGNED (TREE_TYPE (exp)));
 
-      /* For vector types, with the correct size of access, use the mode of
-	 inner type.  */
-      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (exp, 0))) == VECTOR_TYPE
-	  && TREE_TYPE (exp) == TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0)))
-	  && tree_int_cst_equal (size_tree, TYPE_SIZE (TREE_TYPE (exp))))
-        mode = TYPE_MODE (TREE_TYPE (exp));
+      /* For vector element types with the correct size of access or for
+         vector typed accesses use the mode of the access type.  */
+      if ((TREE_CODE (TREE_TYPE (TREE_OPERAND (exp, 0))) == VECTOR_TYPE
+	   && TREE_TYPE (exp) == TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0)))
+	   && tree_int_cst_equal (size_tree, TYPE_SIZE (TREE_TYPE (exp))))
+	  || VECTOR_TYPE_P (TREE_TYPE (exp)))
+	mode = TYPE_MODE (TREE_TYPE (exp));
     }
   else
     {