On 03/31/2016 07:00 AM, Andrew MacLeod wrote:
Looking through build_modify_expr and how it uses that last operand, it's unlikely that the goof in fix_builtin_array_notation_fn would cause problems. In fact, I'm pretty sure it'll never be used when called from either of those sites. A combination of factors come into play and we certainly can't guarantee those factors won't change over time, so we ought to go ahead and fix.Another potential buglet I stumbled across whilst testing the tree-type work: in c/c-array-notation.c::fix_builtin_array_notation_fn() <...> if (list_size > 1) { new_yes_ind = build_modify_expr (location, *new_var, TREE_TYPE (*new_var), NOP_EXPR, location, an_loop_info[0].var, TREE_TYPE (an_loop_info[0].var)); new_yes_expr = build_modify_expr (location, array_ind_value, TREE_TYPE (array_ind_value), NOP_EXPR, location, func_parm, TREE_TYPE ((*array_operand)[0])); } else { new_yes_ind = build_modify_expr (location, *new_var, TREE_TYPE (*new_var), NOP_EXPR, location, TREE_OPERAND (array_op0, 1), TREE_TYPE (TREE_OPERAND (array_op0, 1))); new_yes_expr = build_modify_expr (location, array_ind_value, TREE_TYPE (array_ind_value), NOP_EXPR, location, func_parm, TREE_OPERAND (array_op0, 1)); <<<-------- }'build_modify_expr' expects a type in that final parameter position. It triggered as showing a non-type being passed into a type parameter. I think the last operand on the last line ought to be wrapped in TREE_TYPE() just like it is in the first expression of the else.. Either that, or someone who understands the code needs to figure out whats really wanted there. There is a second occurrence later in the same routine. Patch number 1 makes this change and bootstraps/passes regression. If someone that understands the code wants to have a look, The test can be triggered in the cilk testsuite by adding an assert to build_modify_expr (thats patch number 2 for convenience) and running testcase : make RUNTESTFLAGS=cilk-plus.exp=sec_reduce_ind_same_value.c check-gcc I still have a couple from last fall that were discussed..I'm queuing these up for once stage 1 opens and we can discuss them again.
I'm going to cons up a ChangeLog and commit your patch. jeff
