This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Vector shuffling
On Sat, 3 Sep 2011, Artem Shinkarov wrote:
> > No. You need to fold it (c_fully_fold) to eliminate any
> > C_MAYBE_CONST_EXPR it contains, but you shouldn't need to wrap the result
> > of folding in a SAVE_EXPR.
>
> Ok, Now I get it, thanks.
>
> In the attachment there is a new version of the patch that removes save-exprs.
> +res = __builtin_shuffle2 (a, b, mask2); /* res is @{1,5,3,6@} */
Elsewhere it's __builtin_shuffle for the variants with both numbers of
arguments.
> Index: gcc/c-family/c-common.h
> ===================================================================
> --- gcc/c-family/c-common.h (revision 178354)
> +++ gcc/c-family/c-common.h (working copy)
> @@ -898,6 +898,7 @@ extern tree build_function_call (locatio
>
> extern tree build_function_call_vec (location_t, tree,
> VEC(tree,gc) *, VEC(tree,gc) *);
> +extern tree c_build_vec_shuffle_expr (location_t, tree, tree, tree);
Since this function is actually defined in c-typeck.c, not in c-family
code, the declaration should go in c-tree.h not c-common.h.
> +/* Return true if VEC_SHUFF_EXPR can be expanded using SIMD extensions
VEC_SHUFFLE_EXPR
> Index: gcc/c-typeck.c
> ===================================================================
> +/* Build a VEC_SHUFFLE_EXPR if V0, V1 and MASK are not error_mark_nodes
> + and have vector types, V0 has the same type as V1, and the number of
> + elements of V0, V1, MASK is the same. */
> +tree
> +c_build_vec_shuffle_expr (location_t loc, tree v0, tree v1, tree mask)
> +{
> + tree vec_shuffle, tmp;
> + bool wrap = true;
> + bool maybe_const = false;
> + bool two_arguments = v0 == v1;
Relying on pointer comparison to determine the number of arguments seems
error-prone; a convention of passing NULL_TREE for v1 in the two-argument
case would be better. Consider the case where v0 and v1 both refer to the
same volatile DECL (so at present would have the same tree), which is a
three-argument case where the variable should be read from twice.
The documentation seems to suggest that in the two-argument case the mask
values must be within a single vector whereas the implementation treats
the two-argument case as if the first argument is passed twice (so twice
the range of mask values) but only evaluated once. If the twice-the-range
is intended semantics, it should be documented; otherwise there should be
a comment noting that it's an implementation accident and not guaranteed
semantics for users.
> + if (TREE_TYPE (v0) != TREE_TYPE (v1))
> + {
> + error_at (loc, "__builtin_shuffle argument vectors must be of "
> + "the same type");
> + return error_mark_node;
> + }
What if one is const-qualified or a typedef, and the other isn't? That
should still be allowed, and I don't see any testcases for it. You may
need to use TYPE_MAIN_VARIANT.
> @@ -6120,7 +6201,14 @@ digest_init (location_t init_loc, tree t
> tree value;
> bool constant_p = true;
>
> - /* Iterate through elements and check if all constructor
> + /* If constructor has less elements than the vector type. */
> + if (CONSTRUCTOR_NELTS (inside_init)
> + < TYPE_VECTOR_SUBPARTS (TREE_TYPE (inside_init)))
> + warning_at (init_loc, 0, "vector length does not match "
> + "initializer length, zero elements "
> + "will be inserted");
> +
This looks unrelated to the other changes and should be submitted
separately with its own testcase and rationale if you wish to propose it
as a patch.
> case RID_CHOOSE_EXPR:
> - expr = integer_zerop (c) ? e3 : e2;
> + expr.value = integer_zerop (c) ? e3value : e2value;
I think you should preserve the original_type value of the operand as well
(meaning you need to use the p_orig_types operand to c_parser_expr_list).
This is only relevant to __builtin_choose_expr, not the other
pseudo-builtins.
--
Joseph S. Myers
joseph@codesourcery.com