Re: [PATCH] Vector CONSTRUCTOR verifier

[Richard Guenther]

On Tue, Oct 2, 2012 at 3:01 PM, Jakub Jelinek  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  
>
> 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.0 +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");
> + 

[PATCH] Vector CONSTRUCTOR verifier

[Jakub Jelinek]

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  

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.0 +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 C