[C/C++ PATCH] Fix ICEs with vector subscripting of register variables (PR middle-end/80162)

[C/C++ PATCH] Fix ICEs with vector subscripting of register variables (PR middle-end/80162)

[Jakub Jelinek]

Hi!

c*_mark_addressable doesn't look through VIEW_CONVERT_EXPRs that
vector subscripts are turned into, which means we don't diagnose
taking address of e.g. a vector element in a hard register.

On the other side, I think we want to support just normal vector
subscripting of vectors in hard registers, that can be expanded
into e.g. vector shift etc.

So, this patch handles specially c*_mark_addressable calls from
array_ref building, otherwise diagnoses taking addresses of
vector elements in hard registers.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-03-24  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/80162
c-common/
	* c-common.c (c_common_mark_addressable_vec): Don't set
	TREE_ADDRESSABLE on DECL_HARD_REGISTER.
c/
	* c-tree.h (c_mark_addressable): Add array_ref_p argument.
	* c-typeck.c (c_mark_addressable): Likewise.  Look through
	VIEW_CONVERT_EXPR unless array_ref_p and VCE is from VECTOR_TYPE
	to ARRAY_TYPE.
	(build_array_ref): Pass true as array_ref_p to c_mark_addressable.
cp/
	* cp-tree.h (cxx_mark_addressable): Add array_ref_p argument.
	* typeck.c (cxx_mark_addressable): Likewise.  Look through
	VIEW_CONVERT_EXPR unless array_ref_p and VCE is from VECTOR_TYPE
	to ARRAY_TYPE.
	(cp_build_array_ref): Pass true as array_ref_p to cxx_mark_addressable.
testsuite/
	* c-c++-common/pr80162-1.c: New test.
	* c-c++-common/pr80162-2.c: New test.
	* c-c++-common/pr80162-3.c: New test.

--- gcc/c-family/c-common.c.jj	2017-03-22 19:31:40.000000000 +0100
+++ gcc/c-family/c-common.c	2017-03-24 10:20:13.340088654 +0100
@@ -6542,7 +6542,8 @@ c_common_mark_addressable_vec (tree t)
       && TREE_CODE (t) != PARM_DECL
       && TREE_CODE (t) != COMPOUND_LITERAL_EXPR)
     return;
-  TREE_ADDRESSABLE (t) = 1;
+  if (!VAR_P (t) || !DECL_HARD_REGISTER (t))
+    TREE_ADDRESSABLE (t) = 1;
 }
 
 
--- gcc/c/c-tree.h.jj	2017-03-23 15:49:55.000000000 +0100
+++ gcc/c/c-tree.h	2017-03-24 11:12:19.877403779 +0100
@@ -615,7 +615,7 @@ extern int same_translation_unit_p (cons
 extern int comptypes (tree, tree);
 extern int comptypes_check_different_types (tree, tree, bool *);
 extern bool c_vla_type_p (const_tree);
-extern bool c_mark_addressable (tree);
+extern bool c_mark_addressable (tree, bool = false);
 extern void c_incomplete_type_error (location_t, const_tree, const_tree);
 extern tree c_type_promotes_to (tree);
 extern struct c_expr default_function_array_conversion (location_t,
--- gcc/c/c-typeck.c.jj	2017-03-21 07:55:51.000000000 +0100
+++ gcc/c/c-typeck.c	2017-03-24 11:16:16.725391687 +0100
@@ -2654,7 +2654,7 @@ build_array_ref (location_t loc, tree ar
 	  || (COMPLETE_TYPE_P (TREE_TYPE (TREE_TYPE (array)))
 	      && TREE_CODE (TYPE_SIZE (TREE_TYPE (TREE_TYPE (array)))) != INTEGER_CST))
 	{
-	  if (!c_mark_addressable (array))
+	  if (!c_mark_addressable (array, true))
 	    return error_mark_node;
 	}
       /* An array that is indexed by a constant value which is not within
@@ -4755,16 +4755,26 @@ lvalue_or_else (location_t loc, const_tr
 \f
 /* Mark EXP saying that we need to be able to take the
    address of it; it should not be allocated in a register.
-   Returns true if successful.  */
+   Returns true if successful.  ARRAY_REF_P is true if this
+   is for ARRAY_REF construction - in that case we don't want
+   to look through VIEW_CONVERT_EXPR from VECTOR_TYPE to ARRAY_TYPE,
+   it is fine to use ARRAY_REFs for vector subscripts on vector
+   register variables.  */
 
 bool
-c_mark_addressable (tree exp)
+c_mark_addressable (tree exp, bool array_ref_p)
 {
   tree x = exp;
 
   while (1)
     switch (TREE_CODE (x))
       {
+      case VIEW_CONVERT_EXPR:
+	if (array_ref_p
+	    && TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE
+	    && TREE_CODE (TREE_TYPE (TREE_OPERAND (x, 0))) == VECTOR_TYPE)
+	  return true;
+	/* FALLTHRU */
       case COMPONENT_REF:
       case ADDR_EXPR:
       case ARRAY_REF:
--- gcc/cp/cp-tree.h.jj	2017-03-23 15:49:56.000000000 +0100
+++ gcc/cp/cp-tree.h	2017-03-24 11:23:23.828878855 +0100
@@ -6715,7 +6715,7 @@ extern void cxx_print_error_function		(d
 						 struct diagnostic_info *);
 
 /* in typeck.c */
-extern bool cxx_mark_addressable		(tree);
+extern bool cxx_mark_addressable		(tree, bool = false);
 extern int string_conv_p			(const_tree, const_tree, int);
 extern tree cp_truthvalue_conversion		(tree);
 extern tree condition_conversion		(tree);
--- gcc/cp/typeck.c.jj	2017-03-19 11:57:14.000000000 +0100
+++ gcc/cp/typeck.c	2017-03-24 11:25:51.997956658 +0100
@@ -3217,7 +3217,7 @@ cp_build_array_ref (location_t loc, tree
 	      && (TREE_CODE (TYPE_SIZE (TREE_TYPE (TREE_TYPE (array))))
 		  != INTEGER_CST)))
 	{
-	  if (!cxx_mark_addressable (array))
+	  if (!cxx_mark_addressable (array, true))
 	    return error_mark_node;
 	}
 
@@ -6269,18 +6269,28 @@ unary_complex_lvalue (enum tree_code cod
 \f
 /* Mark EXP saying that we need to be able to take the
    address of it; it should not be allocated in a register.
-   Value is true if successful.
+   Value is true if successful.  ARRAY_REF_P is true if this
+   is for ARRAY_REF construction - in that case we don't want
+   to look through VIEW_CONVERT_EXPR from VECTOR_TYPE to ARRAY_TYPE,
+   it is fine to use ARRAY_REFs for vector subscripts on vector
+   register variables.
 
    C++: we do not allow `current_class_ptr' to be addressable.  */
 
 bool
-cxx_mark_addressable (tree exp)
+cxx_mark_addressable (tree exp, bool array_ref_p)
 {
   tree x = exp;
 
   while (1)
     switch (TREE_CODE (x))
       {
+      case VIEW_CONVERT_EXPR:
+	if (array_ref_p
+	    && TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE
+	    && TREE_CODE (TREE_TYPE (TREE_OPERAND (x, 0))) == VECTOR_TYPE)
+	  return true;
+	/* FALLTHRU */
       case ADDR_EXPR:
       case COMPONENT_REF:
       case ARRAY_REF:
--- gcc/testsuite/c-c++-common/pr80162-1.c.jj	2017-03-24 09:50:24.831288336 +0100
+++ gcc/testsuite/c-c++-common/pr80162-1.c	2017-03-24 11:30:56.973023007 +0100
@@ -0,0 +1,13 @@
+/* PR middle-end/80162 */
+/* { dg-do compile { target { { i?86-*-* x86_64-*-* } && lp64 } } } */
+/* { dg-options "-msse2 -ffixed-xmm7" } */
+
+typedef int v8 __attribute__ ((vector_size (8)));
+struct U { v8 a; v8 b; };
+register struct U u asm ("xmm7");
+
+int *
+foo (int i)
+{
+  return &u.a[i];	/* { dg-error "address of \[^ \n\r]* register variable" } */
+}
--- gcc/testsuite/c-c++-common/pr80162-2.c.jj	2017-03-24 11:28:26.139963810 +0100
+++ gcc/testsuite/c-c++-common/pr80162-2.c	2017-03-24 11:28:58.729544473 +0100
@@ -0,0 +1,18 @@
+/* PR middle-end/80162 */
+/* { dg-do compile { target { { i?86-*-* x86_64-*-* } && lp64 } } } */
+/* { dg-options "-mavx2 -ffixed-xmm7" } */
+
+typedef int V __attribute__ ((vector_size (32)));
+register V u asm ("xmm7");
+
+int
+foo (int i)
+{
+  return u[i];
+}
+
+int
+bar (void)
+{
+  return u[5];
+}
--- gcc/testsuite/c-c++-common/pr80162-3.c.jj	2017-03-24 11:29:26.082192520 +0100
+++ gcc/testsuite/c-c++-common/pr80162-3.c	2017-03-24 11:31:08.667872527 +0100
@@ -0,0 +1,18 @@
+/* PR middle-end/80162 */
+/* { dg-do compile { target { { i?86-*-* x86_64-*-* } && lp64 } } } */
+/* { dg-options "-mavx2 -ffixed-xmm7" } */
+
+typedef int V __attribute__ ((vector_size (32)));
+register V u asm ("xmm7");
+
+int *
+foo (int i)
+{
+  return &u[i];		/* { dg-error "address of \[^ \n\r]* register variable" } */
+}
+
+int *
+bar (void)
+{
+  return &u[5];		/* { dg-error "address of \[^ \n\r]* register variable" } */
+}

	Jakub

Re: [C/C++ PATCH] Fix ICEs with vector subscripting of register variables (PR middle-end/80162)

[Jason Merrill]

On Fri, Mar 24, 2017 at 2:55 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> c*_mark_addressable doesn't look through VIEW_CONVERT_EXPRs that
> vector subscripts are turned into, which means we don't diagnose
> taking address of e.g. a vector element in a hard register.
>
> On the other side, I think we want to support just normal vector
> subscripting of vectors in hard registers, that can be expanded
> into e.g. vector shift etc.
>
> So, this patch handles specially c*_mark_addressable calls from
> array_ref building, otherwise diagnoses taking addresses of
> vector elements in hard registers.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK if C doesn't object.

Jason

Re: [C/C++ PATCH] Fix ICEs with vector subscripting of register variables (PR middle-end/80162)

[Marek Polacek]

On Mon, Mar 27, 2017 at 12:06:03PM -0400, Jason Merrill wrote:
> On Fri, Mar 24, 2017 at 2:55 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > c*_mark_addressable doesn't look through VIEW_CONVERT_EXPRs that
> > vector subscripts are turned into, which means we don't diagnose
> > taking address of e.g. a vector element in a hard register.
> >
> > On the other side, I think we want to support just normal vector
> > subscripting of vectors in hard registers, that can be expanded
> > into e.g. vector shift etc.
> >
> > So, this patch handles specially c*_mark_addressable calls from
> > array_ref building, otherwise diagnoses taking addresses of
> > vector elements in hard registers.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> OK if C doesn't object.

I'm fine with the C parts but I think you should use VECTOR_TYPE_P
(two spots).

	Marek