diff mbox

Vector CONSTRUCTOR verifier

Message ID 20121002130138.GK1787@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Oct. 2, 2012, 1:01 p.m. UTC
Hi!

As discussed in the PR and on IRC, this patch verifies that vector
CONSTRUCTOR in GIMPLE is either empty CONSTRUCTOR, or contains scalar
elements of type compatible with vector element type (then the verification
is less strict, allows less than TYPE_VECTOR_SUBPARTS elements and allows
non-NULL indexes if they are consecutive (no holes); this is because
from FEs often CONSTRUCTORs with those properties leak into the IL, and
a change in the gimplifier to canonicalize them wasn't enough, they keep
leaking even from non-gimplified DECL_INITIAL values etc.), or
contains vector elements (element types must be compatible, the vector
elements must be of the same type and their number must fill the whole
wider vector - these are created/used by tree-vect-generic lowering if
HW supports only shorter vectors than what is requested in source).

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

2012-10-02  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/54713
	* expr.c (categorize_ctor_elements_1): Don't assume purpose is
	non-NULL.
	* tree-cfg.c (verify_gimple_assign_single): Add verification of
	vector CONSTRUCTORs.
	* tree-ssa-sccvn.c (vn_reference_lookup_3): For VECTOR_TYPE
	CONSTRUCTORs, don't do anything if element type is VECTOR_TYPE,
	and don't check index.
	* tree-vect-slp.c (vect_get_constant_vectors): VIEW_CONVERT_EXPR
	ctor elements first if their type isn't compatible with vector
	element type.


	Jakub

Comments

Richard Biener Oct. 2, 2012, 1:22 p.m. UTC | #1
On Tue, Oct 2, 2012 at 3:01 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> As discussed in the PR and on IRC, this patch verifies that vector
> CONSTRUCTOR in GIMPLE is either empty CONSTRUCTOR, or contains scalar
> elements of type compatible with vector element type (then the verification
> is less strict, allows less than TYPE_VECTOR_SUBPARTS elements and allows
> non-NULL indexes if they are consecutive (no holes); this is because
> from FEs often CONSTRUCTORs with those properties leak into the IL, and
> a change in the gimplifier to canonicalize them wasn't enough, they keep
> leaking even from non-gimplified DECL_INITIAL values etc.), or
> contains vector elements (element types must be compatible, the vector
> elements must be of the same type and their number must fill the whole
> wider vector - these are created/used by tree-vect-generic lowering if
> HW supports only shorter vectors than what is requested in source).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok with ...

> 2012-10-02  Jakub Jelinek  <jakub@redhat.com>
>
>         PR tree-optimization/54713
>         * expr.c (categorize_ctor_elements_1): Don't assume purpose is
>         non-NULL.
>         * tree-cfg.c (verify_gimple_assign_single): Add verification of
>         vector CONSTRUCTORs.
>         * tree-ssa-sccvn.c (vn_reference_lookup_3): For VECTOR_TYPE
>         CONSTRUCTORs, don't do anything if element type is VECTOR_TYPE,
>         and don't check index.
>         * tree-vect-slp.c (vect_get_constant_vectors): VIEW_CONVERT_EXPR
>         ctor elements first if their type isn't compatible with vector
>         element type.
>
> --- gcc/expr.c.jj       2012-09-27 12:45:53.000000000 +0200
> +++ gcc/expr.c  2012-10-01 18:21:40.885122833 +0200
> @@ -5491,7 +5491,7 @@ categorize_ctor_elements_1 (const_tree c
>      {
>        HOST_WIDE_INT mult = 1;
>
> -      if (TREE_CODE (purpose) == RANGE_EXPR)
> +      if (purpose && TREE_CODE (purpose) == RANGE_EXPR)
>         {
>           tree lo_index = TREE_OPERAND (purpose, 0);
>           tree hi_index = TREE_OPERAND (purpose, 1);
> --- gcc/tree-cfg.c.jj   2012-10-01 17:28:17.469921927 +0200
> +++ gcc/tree-cfg.c      2012-10-02 11:24:11.686155889 +0200
> @@ -4000,6 +4000,80 @@ verify_gimple_assign_single (gimple stmt
>        return res;
>
>      case CONSTRUCTOR:
> +      if (TREE_CODE (rhs1_type) == VECTOR_TYPE)
> +       {
> +         unsigned int i;
> +         tree elt_i, elt_v, elt_t = NULL_TREE;
> +
> +         if (CONSTRUCTOR_NELTS (rhs1) == 0)
> +           return res;
> +         /* For vector CONSTRUCTORs we require that either it is empty
> +            CONSTRUCTOR, or it is a CONSTRUCTOR of smaller vector elements
> +            (then the element count must be correct to cover the whole
> +            outer vector and index must be NULL on all elements, or it is
> +            a CONSTRUCTOR of scalar elements, where we as an exception allow
> +            smaller number of elements (assuming zero filling) and
> +            consecutive indexes as compared to NULL indexes (such
> +            CONSTRUCTORs can appear in the IL from FEs).  */
> +         FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (rhs1), i, elt_i, elt_v)
> +           {
> +             if (elt_t == NULL_TREE)
> +               {
> +                 elt_t = TREE_TYPE (elt_v);
> +                 if (TREE_CODE (elt_t) == VECTOR_TYPE)
> +                   {
> +                     tree elt_t = TREE_TYPE (elt_v);
> +                     if (!useless_type_conversion_p (TREE_TYPE (rhs1_type),
> +                                                     TREE_TYPE (elt_t)))
> +                       {
> +                         error ("incorrect type of vector CONSTRUCTOR"
> +                                " elements");
> +                         debug_generic_stmt (rhs1);
> +                         return true;
> +                       }
> +                     else if (CONSTRUCTOR_NELTS (rhs1)
> +                              * TYPE_VECTOR_SUBPARTS (elt_t)
> +                              != TYPE_VECTOR_SUBPARTS (rhs1_type))
> +                       {
> +                         error ("incorrect number of vector CONSTRUCTOR"
> +                                " elements");
> +                         debug_generic_stmt (rhs1);
> +                         return true;
> +                       }
> +                   }
> +                 else if (!useless_type_conversion_p (TREE_TYPE (rhs1_type),
> +                                                      elt_t))
> +                   {
> +                     error ("incorrect type of vector CONSTRUCTOR elements");
> +                     debug_generic_stmt (rhs1);
> +                     return true;
> +                   }
> +                 else if (CONSTRUCTOR_NELTS (rhs1)
> +                          > TYPE_VECTOR_SUBPARTS (rhs1_type))
> +                   {
> +                     error ("incorrect number of vector CONSTRUCTOR elements");
> +                     debug_generic_stmt (rhs1);
> +                     return true;
> +                   }
> +               }
> +             else if (!useless_type_conversion_p (elt_t, TREE_TYPE (elt_v)))
> +               {
> +                 error ("incorrect type of vector CONSTRUCTOR elements");
> +                 debug_generic_stmt (rhs1);
> +                 return true;
> +               }
> +             if (elt_i != NULL_TREE
> +                 && (TREE_CODE (elt_t) == VECTOR_TYPE
> +                     || TREE_CODE (elt_i) != INTEGER_CST
> +                     || compare_tree_int (elt_i, i) != 0))
> +               {
> +                 error ("vector CONSTRUCTOR with non-NULL element index");
> +                 debug_generic_stmt (rhs1);
> +                 return true;
> +               }
> +           }
> +       }
> +      return res;
>      case OBJ_TYPE_REF:
>      case ASSERT_EXPR:
>      case WITH_SIZE_EXPR:
> --- gcc/tree-ssa-sccvn.c.jj     2012-09-25 11:59:43.000000000 +0200
> +++ gcc/tree-ssa-sccvn.c        2012-10-02 11:26:34.935326468 +0200
> @@ -1639,8 +1639,17 @@ vn_reference_lookup_3 (ao_ref *ref, tree
>                   if (i < CONSTRUCTOR_NELTS (ctor))
>                     {
>                       constructor_elt *elt = CONSTRUCTOR_ELT (ctor, i);
> -                     if (compare_tree_int (elt->index, i) == 0)
> -                       val = elt->value;
> +                     if (TREE_CODE (TREE_TYPE (rhs1)) == VECTOR_TYPE)
> +                       {
> +                         if (TREE_CODE (TREE_TYPE (elt->value))
> +                             != VECTOR_TYPE)
> +                           val = elt->value;
> +                       }
> +                     else if (elt->index)

This case removed.  We are only interested in VECTOR constructors
(the only constructors allowed in gimple).

Thanks,
Richard.

> +                       {
> +                         if (compare_tree_int (elt->index, i) == 0)
> +                           val = elt->value;
> +                       }
>                     }
>                 }
>               if (val)
> --- gcc/tree-vect-slp.c.jj      2012-10-01 17:28:17.215923346 +0200
> +++ gcc/tree-vect-slp.c 2012-10-01 17:59:49.423421284 +0200
> @@ -2345,6 +2345,7 @@ vect_get_constant_vectors (tree op, slp_
>    enum tree_code code = gimple_expr_code (stmt);
>    gimple def_stmt;
>    struct loop *loop;
> +  gimple_seq ctor_seq = NULL;
>
>    if (STMT_VINFO_DEF_TYPE (stmt_vinfo) == vect_reduction_def
>        && reduc_index != -1)
> @@ -2503,11 +2504,27 @@ vect_get_constant_vectors (tree op, slp_
>
>            /* Create 'vect_ = {op0,op1,...,opn}'.  */
>            number_of_places_left_in_vector--;
> -         if (constant_p
> -             && !types_compatible_p (TREE_TYPE (vector_type), TREE_TYPE (op)))
> +         if (!types_compatible_p (TREE_TYPE (vector_type), TREE_TYPE (op)))
>             {
> -             op = fold_unary (VIEW_CONVERT_EXPR, TREE_TYPE (vector_type), op);
> -             gcc_assert (op && CONSTANT_CLASS_P (op));
> +             if (constant_p)
> +               {
> +                 op = fold_unary (VIEW_CONVERT_EXPR,
> +                                  TREE_TYPE (vector_type), op);
> +                 gcc_assert (op && CONSTANT_CLASS_P (op));
> +               }
> +             else
> +               {
> +                 tree new_temp
> +                   = make_ssa_name (TREE_TYPE (vector_type), NULL);
> +                 gimple init_stmt;
> +                 op = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (vector_type),
> +                              op);
> +                 init_stmt
> +                   = gimple_build_assign_with_ops (VIEW_CONVERT_EXPR,
> +                                                   new_temp, op, NULL_TREE);
> +                 gimple_seq_add_stmt (&ctor_seq, init_stmt);
> +                 op = new_temp;
> +               }
>             }
>           elts[number_of_places_left_in_vector] = op;
>
> @@ -2529,6 +2546,15 @@ vect_get_constant_vectors (tree op, slp_
>                VEC_quick_push (tree, voprnds,
>                                vect_init_vector (stmt, vec_cst,
>                                                 vector_type, NULL));
> +             if (ctor_seq != NULL)
> +               {
> +                 gimple init_stmt
> +                   = SSA_NAME_DEF_STMT (VEC_last (tree, voprnds));
> +                 gimple_stmt_iterator gsi = gsi_for_stmt (init_stmt);
> +                 gsi_insert_seq_before_without_update (&gsi, ctor_seq,
> +                                                       GSI_SAME_STMT);
> +                 ctor_seq = NULL;
> +               }
>              }
>          }
>      }
>
>         Jakub
diff mbox

Patch

--- gcc/expr.c.jj	2012-09-27 12:45:53.000000000 +0200
+++ gcc/expr.c	2012-10-01 18:21:40.885122833 +0200
@@ -5491,7 +5491,7 @@  categorize_ctor_elements_1 (const_tree c
     {
       HOST_WIDE_INT mult = 1;
 
-      if (TREE_CODE (purpose) == RANGE_EXPR)
+      if (purpose && TREE_CODE (purpose) == RANGE_EXPR)
 	{
 	  tree lo_index = TREE_OPERAND (purpose, 0);
 	  tree hi_index = TREE_OPERAND (purpose, 1);
--- gcc/tree-cfg.c.jj	2012-10-01 17:28:17.469921927 +0200
+++ gcc/tree-cfg.c	2012-10-02 11:24:11.686155889 +0200
@@ -4000,6 +4000,80 @@  verify_gimple_assign_single (gimple stmt
       return res;
 
     case CONSTRUCTOR:
+      if (TREE_CODE (rhs1_type) == VECTOR_TYPE)
+	{
+	  unsigned int i;
+	  tree elt_i, elt_v, elt_t = NULL_TREE;
+
+	  if (CONSTRUCTOR_NELTS (rhs1) == 0)
+	    return res;
+	  /* For vector CONSTRUCTORs we require that either it is empty
+	     CONSTRUCTOR, or it is a CONSTRUCTOR of smaller vector elements
+	     (then the element count must be correct to cover the whole
+	     outer vector and index must be NULL on all elements, or it is
+	     a CONSTRUCTOR of scalar elements, where we as an exception allow
+	     smaller number of elements (assuming zero filling) and
+	     consecutive indexes as compared to NULL indexes (such
+	     CONSTRUCTORs can appear in the IL from FEs).  */
+	  FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (rhs1), i, elt_i, elt_v)
+	    {
+	      if (elt_t == NULL_TREE)
+		{
+		  elt_t = TREE_TYPE (elt_v);
+		  if (TREE_CODE (elt_t) == VECTOR_TYPE)
+		    {
+		      tree elt_t = TREE_TYPE (elt_v);
+		      if (!useless_type_conversion_p (TREE_TYPE (rhs1_type),
+						      TREE_TYPE (elt_t)))
+			{
+			  error ("incorrect type of vector CONSTRUCTOR"
+				 " elements");
+			  debug_generic_stmt (rhs1);
+			  return true;
+			}
+		      else if (CONSTRUCTOR_NELTS (rhs1)
+			       * TYPE_VECTOR_SUBPARTS (elt_t)
+			       != TYPE_VECTOR_SUBPARTS (rhs1_type))
+			{
+			  error ("incorrect number of vector CONSTRUCTOR"
+				 " elements");
+			  debug_generic_stmt (rhs1);
+			  return true;
+			}
+		    }
+		  else if (!useless_type_conversion_p (TREE_TYPE (rhs1_type),
+						       elt_t))
+		    {
+		      error ("incorrect type of vector CONSTRUCTOR elements");
+		      debug_generic_stmt (rhs1);
+		      return true;
+		    }
+		  else if (CONSTRUCTOR_NELTS (rhs1)
+			   > TYPE_VECTOR_SUBPARTS (rhs1_type))
+		    {
+		      error ("incorrect number of vector CONSTRUCTOR elements");
+		      debug_generic_stmt (rhs1);
+		      return true;
+		    }
+		}
+	      else if (!useless_type_conversion_p (elt_t, TREE_TYPE (elt_v)))
+		{
+		  error ("incorrect type of vector CONSTRUCTOR elements");
+		  debug_generic_stmt (rhs1);
+		  return true;
+		}
+	      if (elt_i != NULL_TREE
+		  && (TREE_CODE (elt_t) == VECTOR_TYPE
+		      || TREE_CODE (elt_i) != INTEGER_CST
+		      || compare_tree_int (elt_i, i) != 0))
+		{
+		  error ("vector CONSTRUCTOR with non-NULL element index");
+		  debug_generic_stmt (rhs1);
+		  return true;
+		}
+	    }
+	}
+      return res;
     case OBJ_TYPE_REF:
     case ASSERT_EXPR:
     case WITH_SIZE_EXPR:
--- gcc/tree-ssa-sccvn.c.jj	2012-09-25 11:59:43.000000000 +0200
+++ gcc/tree-ssa-sccvn.c	2012-10-02 11:26:34.935326468 +0200
@@ -1639,8 +1639,17 @@  vn_reference_lookup_3 (ao_ref *ref, tree
 		  if (i < CONSTRUCTOR_NELTS (ctor))
 		    {
 		      constructor_elt *elt = CONSTRUCTOR_ELT (ctor, i);
-		      if (compare_tree_int (elt->index, i) == 0)
-			val = elt->value;
+		      if (TREE_CODE (TREE_TYPE (rhs1)) == VECTOR_TYPE)
+			{
+			  if (TREE_CODE (TREE_TYPE (elt->value))
+			      != VECTOR_TYPE)
+			    val = elt->value;
+			}
+		      else if (elt->index)
+			{
+			  if (compare_tree_int (elt->index, i) == 0)
+			    val = elt->value;
+			}
 		    }
 		}
 	      if (val)
--- gcc/tree-vect-slp.c.jj	2012-10-01 17:28:17.215923346 +0200
+++ gcc/tree-vect-slp.c	2012-10-01 17:59:49.423421284 +0200
@@ -2345,6 +2345,7 @@  vect_get_constant_vectors (tree op, slp_
   enum tree_code code = gimple_expr_code (stmt);
   gimple def_stmt;
   struct loop *loop;
+  gimple_seq ctor_seq = NULL;
 
   if (STMT_VINFO_DEF_TYPE (stmt_vinfo) == vect_reduction_def
       && reduc_index != -1)
@@ -2503,11 +2504,27 @@  vect_get_constant_vectors (tree op, slp_
 
           /* Create 'vect_ = {op0,op1,...,opn}'.  */
           number_of_places_left_in_vector--;
-	  if (constant_p
-	      && !types_compatible_p (TREE_TYPE (vector_type), TREE_TYPE (op)))
+	  if (!types_compatible_p (TREE_TYPE (vector_type), TREE_TYPE (op)))
 	    {
-	      op = fold_unary (VIEW_CONVERT_EXPR, TREE_TYPE (vector_type), op);
-	      gcc_assert (op && CONSTANT_CLASS_P (op));
+	      if (constant_p)
+		{
+		  op = fold_unary (VIEW_CONVERT_EXPR,
+				   TREE_TYPE (vector_type), op);
+		  gcc_assert (op && CONSTANT_CLASS_P (op));
+		}
+	      else
+		{
+		  tree new_temp
+		    = make_ssa_name (TREE_TYPE (vector_type), NULL);
+		  gimple init_stmt;
+		  op = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (vector_type),
+			       op);		  
+		  init_stmt
+		    = gimple_build_assign_with_ops (VIEW_CONVERT_EXPR,
+						    new_temp, op, NULL_TREE);
+		  gimple_seq_add_stmt (&ctor_seq, init_stmt);
+		  op = new_temp;
+		}
 	    }
 	  elts[number_of_places_left_in_vector] = op;
 
@@ -2529,6 +2546,15 @@  vect_get_constant_vectors (tree op, slp_
               VEC_quick_push (tree, voprnds,
                               vect_init_vector (stmt, vec_cst,
 						vector_type, NULL));
+	      if (ctor_seq != NULL)
+		{
+		  gimple init_stmt
+		    = SSA_NAME_DEF_STMT (VEC_last (tree, voprnds));
+		  gimple_stmt_iterator gsi = gsi_for_stmt (init_stmt);
+		  gsi_insert_seq_before_without_update (&gsi, ctor_seq,
+							GSI_SAME_STMT);
+		  ctor_seq = NULL;
+		}
             }
         }
     }