This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix PR70434


On Wed, 30 Mar 2016, Jakub Jelinek wrote:

> On Wed, Mar 30, 2016 at 02:07:07PM +0200, Richard Biener wrote:
> > The patch for PR63764 (accepts-invalid / ICE) caused us to generate
> > worse code for rvalue vector indexing by forcing the vector to a
> > temporary.  It turns out this is not necessary if we alter the
> > way the C/C++ FE lower the vector to perform the indexing operation
> > from lowering to a pointer-to-element to lowering to an array
> > using a VIEW_CONVERT_EXPR.
> > 
> > The alternate lowering has the advantage that the vector is not
> > required to be addressable which should improve aliasing analysis.
> > Not lowering to indirect accesses should also improve optimizations
> > like value-numbering.
> > 
> > There's the fallout that we need to make sure to convert back
> > constant index array-refs to the canonical BIT_FIELD_REF form
> > we use for vectors (see the gimple-fold.c hunk pattern-matching this).
> > 
> > And there's the latent bug in update-address-taken which happily
> > re-wrote a vector array-ref base into SSA form.
> > 
> > Bootstrapped on x86_64-unknown-linux-gnu, testing still in progress.
> > 
> > I'll happily defer to stage1 if you think that's better as I don't
> > know any project that heavily makes use of GCCs vector extension
> > and I'm not sure about our own testsuite coverage.
> 
> I think we should defer it for stage1 at this point.

For reference, further changes were necessary to make the patch
regression-free - here they are, queued for stage1.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Richard.

2016-04-01  Richard Biener  <rguenther@suse.de>

	PR middle-end/70434
	c-family/
	* c-common.c (convert_vector_to_pointer_for_subscript): Use a
	VIEW_CONVERT_EXPR to an array type.

	cp/
	* expr.c (mark_exp_read): Handle VIEW_CONVERT_EXPR.
	* constexpr.c (cxx_eval_array_reference): Handle indexed
	vectors.

	c/
	* c-typeck.c (build_array_ref): Do not complain about indexing
	non-lvalue vectors.

	* tree-ssa.c (non_rewritable_mem_ref_base): Make sure to mark
	bases which are accessed with non-invariant indices.
	* gimple-fold.c (maybe_canonicalize_mem_ref_addr): Re-write
	constant index ARRAY_REFs of vectors into BIT_FIELD_REFs.

Index: gcc/c-family/c-common.c
===================================================================
*** gcc/c-family/c-common.c	(revision 234546)
--- gcc/c-family/c-common.c	(working copy)
*************** convert_vector_to_pointer_for_subscript
*** 12448,12501 ****
    if (VECTOR_TYPE_P (TREE_TYPE (*vecp)))
      {
        tree type = TREE_TYPE (*vecp);
-       tree type1;
  
        ret = !lvalue_p (*vecp);
        if (TREE_CODE (index) == INTEGER_CST)
          if (!tree_fits_uhwi_p (index)
              || tree_to_uhwi (index) >= TYPE_VECTOR_SUBPARTS (type))
            warning_at (loc, OPT_Warray_bounds, "index value is out of bound");
  
!       if (ret)
! 	{
! 	  tree tmp = create_tmp_var_raw (type);
! 	  DECL_SOURCE_LOCATION (tmp) = loc;
! 	  *vecp = c_save_expr (*vecp);
! 	  if (TREE_CODE (*vecp) == C_MAYBE_CONST_EXPR)
! 	    {
! 	      bool non_const = C_MAYBE_CONST_EXPR_NON_CONST (*vecp);
! 	      *vecp = C_MAYBE_CONST_EXPR_EXPR (*vecp);
! 	      *vecp
! 		= c_wrap_maybe_const (build4 (TARGET_EXPR, type, tmp,
! 					      *vecp, NULL_TREE, NULL_TREE),
! 				      non_const);
! 	    }
! 	  else
! 	    *vecp = build4 (TARGET_EXPR, type, tmp, *vecp,
! 			    NULL_TREE, NULL_TREE);
! 	  SET_EXPR_LOCATION (*vecp, loc);
! 	  c_common_mark_addressable_vec (tmp);
! 	}
!       else
! 	c_common_mark_addressable_vec (*vecp);
!       type = build_qualified_type (TREE_TYPE (type), TYPE_QUALS (type));
!       type1 = build_pointer_type (TREE_TYPE (*vecp));
!       bool ref_all = TYPE_REF_CAN_ALIAS_ALL (type1);
!       if (!ref_all
! 	  && !DECL_P (*vecp))
! 	{
! 	  /* If the original vector isn't declared may_alias and it
! 	     isn't a bare vector look if the subscripting would
! 	     alias the vector we subscript, and if not, force ref-all.  */
! 	  alias_set_type vecset = get_alias_set (*vecp);
! 	  alias_set_type sset = get_alias_set (type);
! 	  if (!alias_sets_must_conflict_p (sset, vecset)
! 	      && !alias_set_subset_of (sset, vecset))
! 	    ref_all = true;
! 	}
!       type = build_pointer_type_for_mode (type, ptr_mode, ref_all);
!       *vecp = build1 (ADDR_EXPR, type1, *vecp);
!       *vecp = convert (type, *vecp);
      }
    return ret;
  }
--- 12448,12470 ----
    if (VECTOR_TYPE_P (TREE_TYPE (*vecp)))
      {
        tree type = TREE_TYPE (*vecp);
  
        ret = !lvalue_p (*vecp);
+ 
        if (TREE_CODE (index) == INTEGER_CST)
          if (!tree_fits_uhwi_p (index)
              || tree_to_uhwi (index) >= TYPE_VECTOR_SUBPARTS (type))
            warning_at (loc, OPT_Warray_bounds, "index value is out of bound");
  
!       /* We are building an ARRAY_REF so mark the vector as addressable
!          to not run into the gimplifiers premature setting of DECL_GIMPLE_REG_P
! 	 for function parameters.  */
!       c_common_mark_addressable_vec (*vecp);
! 
!       *vecp = build1 (VIEW_CONVERT_EXPR,
! 		      build_array_type_nelts (TREE_TYPE (type),
! 					      TYPE_VECTOR_SUBPARTS (type)),
! 		      *vecp);
      }
    return ret;
  }
Index: gcc/tree-ssa.c
===================================================================
*** gcc/tree-ssa.c	(revision 234546)
--- gcc/tree-ssa.c	(working copy)
*************** maybe_rewrite_mem_ref_base (tree *tp, bi
*** 1278,1291 ****
  static tree
  non_rewritable_mem_ref_base (tree ref)
  {
!   tree base = ref;
  
    /* A plain decl does not need it set.  */
    if (DECL_P (ref))
      return NULL_TREE;
  
!   while (handled_component_p (base))
!     base = TREE_OPERAND (base, 0);
  
    /* But watch out for MEM_REFs we cannot lower to a
       VIEW_CONVERT_EXPR or a BIT_FIELD_REF.  */
--- 1301,1319 ----
  static tree
  non_rewritable_mem_ref_base (tree ref)
  {
!   tree base;
  
    /* A plain decl does not need it set.  */
    if (DECL_P (ref))
      return NULL_TREE;
  
!   if (! (base = CONST_CAST_TREE (strip_invariant_refs (ref))))
!     {
!       base = get_base_address (ref);
!       if (DECL_P (base))
! 	return base;
!       return NULL_TREE;
!     }
  
    /* But watch out for MEM_REFs we cannot lower to a
       VIEW_CONVERT_EXPR or a BIT_FIELD_REF.  */
Index: gcc/cp/expr.c
===================================================================
*** gcc/cp/expr.c	(revision 234546)
--- gcc/cp/expr.c	(working copy)
*************** mark_exp_read (tree exp)
*** 145,150 ****
--- 145,151 ----
      case ADDR_EXPR:
      case INDIRECT_REF:
      case FLOAT_EXPR:
+     case VIEW_CONVERT_EXPR:
        mark_exp_read (TREE_OPERAND (exp, 0));
        break;
      case COMPOUND_EXPR:
Index: gcc/c/c-typeck.c
===================================================================
*** gcc/c/c-typeck.c	(revision 234546)
--- gcc/c/c-typeck.c	(working copy)
*************** build_array_ref (location_t loc, tree ar
*** 2581,2586 ****
--- 2581,2587 ----
  
    gcc_assert (TREE_CODE (TREE_TYPE (index)) == INTEGER_TYPE);
  
+   bool was_vector = VECTOR_TYPE_P (TREE_TYPE (array));
    bool non_lvalue
      = convert_vector_to_pointer_for_subscript (loc, &array, index);
  
*************** build_array_ref (location_t loc, tree ar
*** 2611,2617 ****
  	    return error_mark_node;
  	}
  
!       if (pedantic || warn_c90_c99_compat)
  	{
  	  tree foo = array;
  	  while (TREE_CODE (foo) == COMPONENT_REF)
--- 2612,2619 ----
  	    return error_mark_node;
  	}
  
!       if ((pedantic || warn_c90_c99_compat)
! 	  && ! was_vector)
  	{
  	  tree foo = array;
  	  while (TREE_CODE (foo) == COMPONENT_REF)
Index: gcc/cp/constexpr.c
===================================================================
--- gcc/cp/constexpr.c	(revision 234546)
+++ gcc/cp/constexpr.c	(working copy)
@@ -1866,6 +1866,10 @@ cxx_eval_array_reference (const constexp
   else if (lval)
     return build4 (ARRAY_REF, TREE_TYPE (t), ary, index, NULL, NULL);
   elem_type = TREE_TYPE (TREE_TYPE (ary));
+  if (TREE_CODE (ary) == VIEW_CONVERT_EXPR
+      && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (ary, 0)))
+      && TREE_TYPE (t) == TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0))))
+    ary = TREE_OPERAND (ary, 0);
   if (TREE_CODE (ary) == CONSTRUCTOR)
     len = CONSTRUCTOR_NELTS (ary);
   else if (TREE_CODE (ary) == STRING_CST)
@@ -1874,6 +1878,8 @@ cxx_eval_array_reference (const constexp
 		     / TYPE_PRECISION (char_type_node));
       len = (unsigned) TREE_STRING_LENGTH (ary) / elem_nchars;
     }
+  else if (TREE_CODE (ary) == VECTOR_CST)
+    len = VECTOR_CST_NELTS (ary);
   else
     {
       /* We can't do anything with other tree codes, so use
@@ -1891,7 +1897,14 @@ cxx_eval_array_reference (const constexp
       return t;
     }
 
-  tree nelts = array_type_nelts_top (TREE_TYPE (ary));
+  tree nelts;
+  if (TREE_CODE (TREE_TYPE (ary)) == ARRAY_TYPE)
+    nelts = array_type_nelts_top (TREE_TYPE (ary));
+  else if (VECTOR_TYPE_P (TREE_TYPE (ary)))
+    nelts = size_int (TYPE_VECTOR_SUBPARTS (TREE_TYPE (ary)));
+  else
+    gcc_unreachable ();
+
   /* For VLAs, the number of elements won't be an integer constant.  */
   nelts = cxx_eval_constant_expression (ctx, nelts, false, non_constant_p,
 					overflow_p);
@@ -1938,6 +1951,8 @@ cxx_eval_array_reference (const constexp
 
   if (TREE_CODE (ary) == CONSTRUCTOR)
     return (*CONSTRUCTOR_ELTS (ary))[i].value;
+  else if (TREE_CODE (ary) == VECTOR_CST)
+    return VECTOR_CST_ELT (ary, i);
   else if (elem_nchars == 1)
     return build_int_cst (cv_unqualified (TREE_TYPE (TREE_TYPE (ary))),
 			  TREE_STRING_POINTER (ary)[i]);
Index: gcc/gimple-fold.c
===================================================================
*** gcc/gimple-fold.c	(revision 234546)
--- gcc/gimple-fold.c	(working copy)
*************** maybe_canonicalize_mem_ref_addr (tree *t
*** 3445,3450 ****
--- 3445,3488 ----
    if (TREE_CODE (*t) == ADDR_EXPR)
      t = &TREE_OPERAND (*t, 0);
  
+   /* The C and C++ frontends use an ARRAY_REF for indexing with their
+      generic vector extension.  The actual vector referenced is
+      view-converted to an array type for this purpose.  If the index
+      is constant the canonical representation in the middle-end is a
+      BIT_FIELD_REF so re-write the former to the latter here.  */
+   if (TREE_CODE (*t) == ARRAY_REF
+       && TREE_CODE (TREE_OPERAND (*t, 0)) == VIEW_CONVERT_EXPR
+       && TREE_CODE (TREE_OPERAND (*t, 1)) == INTEGER_CST
+       && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (TREE_OPERAND (*t, 0), 0))))
+     {
+       tree vtype = TREE_TYPE (TREE_OPERAND (TREE_OPERAND (*t, 0), 0));
+       if (VECTOR_TYPE_P (vtype))
+ 	{
+ 	  tree low = array_ref_low_bound (*t);
+ 	  if (TREE_CODE (low) == INTEGER_CST)
+ 	    {
+ 	      if (tree_int_cst_le (low, TREE_OPERAND (*t, 1)))
+ 		{
+ 		  widest_int idx = wi::sub (wi::to_widest (TREE_OPERAND (*t, 1)),
+ 					    wi::to_widest (low));
+ 		  idx = wi::mul (idx, wi::to_widest
+ 					 (TYPE_SIZE (TREE_TYPE (*t))));
+ 		  widest_int ext
+ 		    = wi::add (idx, wi::to_widest (TYPE_SIZE (TREE_TYPE (*t))));
+ 		  if (wi::les_p (ext, wi::to_widest (TYPE_SIZE (vtype))))
+ 		    {
+ 		      *t = build3_loc (EXPR_LOCATION (*t), BIT_FIELD_REF,
+ 				       TREE_TYPE (*t),
+ 				       TREE_OPERAND (TREE_OPERAND (*t, 0), 0),
+ 				       TYPE_SIZE (TREE_TYPE (*t)),
+ 				       wide_int_to_tree (sizetype, idx));
+ 		      res = true;
+ 		    }
+ 		}
+ 	    }
+ 	}
+     }
+ 
    while (handled_component_p (*t))
      t = &TREE_OPERAND (*t, 0);
  


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]