On Fri, Sep 2, 2011 at 4:41 PM, Joseph S. Myers <jos...@codesourcery.com> wrote: > On Fri, 2 Sep 2011, Artem Shinkarov wrote: > >> + /* Avoid C_MAYBE_CONST_EXPRs inside VEC_SHUFFLE_EXPR. */ >> + tmp = c_fully_fold (v0, false, &maybe_const); >> + v0 = save_expr (tmp); >> + wrap &= maybe_const; > > I suppose you need this save_expr because of the two-argument case, but > shouldn't need it otherwise. > >> + if (!two_arguments) >> + { >> + tmp = c_fully_fold (v1, false, &maybe_const); >> + v1 = save_expr (tmp); > > And you shouldn't need this save_expr at all. > >> + tmp = c_fully_fold (mask, false, &maybe_const); >> + mask = save_expr (tmp); > > Or this one.
Joseph, I don't understand this comment. I have 2 or 3 arguments in the VEC_SHUFFLE_EXPR and any of them can be C_MAYBE_CONST_EXPR, so I need to wrap mask (the last argument) to avoid the following failure: #define vector(elcount, type) \ __attribute__((vector_size((elcount)*sizeof(type)))) type extern int p, q, v, r; int main () { vector (4, int) i0 = {argc, 1,2,3}; vector (4, int) i1 = {argc, 1, argc, 3}; vector (4, int) imask = {0,3,2,1}; vector (4, int) extmask = {p,q,r,v}; i2 = __builtin_shuffle (i0, (p,q)? imask:extmask); return 0; } and the same failure would happen if __builtin_shuffle expression will be in the following form: i2 = __builtin_shuffle (i0, (p,q)? imask:extmask, i2); All the rest -- agreed, and is fixed already. Thanks, Artem. >> +/* Helper function to read arguments of builtins which are interfaces >> + for the middle-end nodes like COMPLEX_EXPR, VEC_SHUFLE_EXPR and > > Spelling of SHUFFLE. > >> + others. The name of the builtin is passed using BNAME parameter. > > Two spaces after ".". > >> + Function returns true if there were no errors while parsing and >> + stores the arguments in EXPR_LIST*/ > > ". " at end of comment. > >> +static bool >> +c_parser_get_builtin_args (c_parser * parser, const char * bname, >> + VEC(tree,gc) ** expr_list) > > No spaces after "*". > >> + if (c_parser_next_token_is_not (parser, CPP_OPEN_PAREN)) >> + { >> + error_at (loc, "cannot take address of %<%s%>", bname); > > %qs is a simpler form of %<%s%>. > >> @@ -6461,6 +6500,35 @@ c_parser_postfix_expression (c_parser *p > > Should also convert __builtin_choose_expr and __builtin_complex to use the > new helper. > >> + if (! c_parser_get_builtin_args (parser, > > No space after "!". > >> + { >> + error_at (loc, "%<__builtin_shuffle%> wrong number of >> arguments"); > > "wrong number of arguments to %<__builtin_shuffle%>". > > -- > Joseph S. Myers > jos...@codesourcery.com >