On Tue, Oct 2, 2012 at 3:01 PM, Jakub Jelinek <ja...@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 <ja...@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
