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 jos...@codesourcery.com