This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR70434
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 1 Apr 2016 09:10:30 +0200 (CEST)
- Subject: Re: [PATCH] Fix PR70434
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1603301358070 dot 13384 at t29 dot fhfr dot qr> <20160330121410 dot GZ3017 at tucnak dot redhat dot com>
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);