Martin Liška <mli...@suse.cz> writes:
> On 6/26/19 1:39 PM, Jakub Jelinek wrote:
>> On Wed, Jun 26, 2019 at 12:57:15PM +0200, Martin Liška wrote:
>>> --- a/gcc/tree-vect-stmts.c
>>> +++ b/gcc/tree-vect-stmts.c
>>> @@ -3483,8 +3483,7 @@ vectorizable_call (stmt_vec_info stmt_info,
>>> gimple_stmt_iterator *gsi,
>>> = gimple_build_call_internal_vec (ifn, vargs);
>>> gimple_call_set_lhs (call, half_res);
>>> gimple_call_set_nothrow (call, true);
>>> - new_stmt_info
>>> - = vect_finish_stmt_generation (stmt_info, call, gsi);
>>> + vect_finish_stmt_generation (stmt_info, call, gsi);
>>> if ((i & 1) == 0)
>>> {
>>> prev_res = half_res;
>>> @@ -3583,8 +3582,7 @@ vectorizable_call (stmt_vec_info stmt_info,
>>> gimple_stmt_iterator *gsi,
>>> gcall *call = gimple_build_call_internal_vec (ifn, vargs);
>>> gimple_call_set_lhs (call, half_res);
>>> gimple_call_set_nothrow (call, true);
>>> - new_stmt_info
>>> - = vect_finish_stmt_generation (stmt_info, call, gsi);
>>> + vect_finish_stmt_generation (stmt_info, call, gsi);
>>> if ((j & 1) == 0)
>>> {
>>> prev_res = half_res;
>>
>> This looks wrong.
>> There should have been
>> SLP_TREE_VEC_STMTS (slp_node).quick_push (new_stmt_info);
>> in between for slp_node, or the usual code like:
>> if (cond_for_first)
>> STMT_VINFO_VEC_STMT (stmt_info) = *vec_stmt = new_stmt_info;
>> else
>> STMT_VINFO_RELATED_STMT (prev_stmt_info) = new_stmt_info;
>> prev_stmt_info = new_stmt_info;
>> otherwise. In any case, I think this should be dealt with separately.
>
> Likewise here:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91017
I think this part of the patch is OK. SLP_TREE_VEC_STMTS and
STMT_VINFO_VEC_STMT should only receive the final stmt for each vector
result, whereas these stmts are producing intermediate results.
The assignments were introduced as part of a mechanical patch to avoid
using vinfo_for_stmt to get stmt_infos that we'd just created. I'd
missed that in these cases we didn't actually use vinfo_for_stmt.
Off-hand, I'm not sure whether we actually need to create the
stmt_info here as things stand, or whether we're really just using
vect_finish_stmt_generation to insert the stmt in the right way.
I did have some WIP patches that would need the stmt_info to be
created though.
Thanks,
Richard
