On 03/31/2016 07:00 AM, Andrew MacLeod wrote:
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.
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.
I'm going to cons up a ChangeLog and commit your patch.
jeff