Thanks Richard.
Uros,
Could you please review back-end part of this patch?
Thanks.
Yuri.
2016-01-28 16:26 GMT+03:00 Richard Biener <richard.guent...@gmail.com>:
> On Fri, Jan 22, 2016 at 3:29 PM, Yuri Rumyantsev <ysrum...@gmail.com> wrote:
>> Richard,
>>
>> I fixed all remarks pointed by you in vectorizer part of patch. Could
>> you take a look on modified patch.
>
> + if (is_gimple_call (stmt1))
> + lhs = gimple_call_lhs (stmt1);
> + else
> + if (gimple_code (stmt1) == GIMPLE_ASSIGN)
> + lhs = gimple_assign_lhs (stmt1);
> + else
> + break;
>
> use
>
> lhs = gimple_get_lhs (stmt1);
> if (!lhs)
> break;
>
> you forget to free bbs, I suggest to do it after seeding the worklist.
>
> Ok with those changes if the backend changes are approved.
>
> Sorry for the delay in reviewing this.
>
> Thanks,
> Richard.
>
>> Uros,
>>
>> Could you please review i386 part of patch related to support of
>> conditional branches with vector comparison.
>>
>> Bootstrap and regression testing did not show any new failures.
>> Is it OK for trunk?
>>
>> Thanks.
>> Yuri.
>>
>> ChangeLog:
>>
>> 2016-01-22 Yuri Rumyantsev <ysrum...@gmail.com>
>>
>> PR middle-end/68542
>> * config/i386/i386.c (ix86_expand_branch): Add support for conditional
>> brnach with vector comparison.
>> * config/i386/sse.md (define_expand "cbranch<mode>4): Add define-expand
>> for vector comparion with eq/ne only.
>> (optimize_mask_stores): New function.
>> * tree-vect-stmts.c (vectorizable_mask_load_store): Initialize
>> has_mask_store field of vect_info.
>> * tree-vectorizer.c (vectorize_loops): Invoke optimaze_mask_stores for
>> vectorized loops having masked stores after vec_info destroy.
>> * tree-vectorizer.h (loop_vec_info): Add new has_mask_store field and
>> correspondent macros.
>> (optimize_mask_stores): Add prototype.
>>
>> gcc/testsuite/ChangeLog:
>> * gcc.dg/vect/vect-mask-store-move-1.c: New test.
>> * testsuite/gcc.target/i386/avx2-vect-mask-store-move1.c: New test.
>>
>> 2016-01-20 15:24 GMT+03:00 Richard Biener <richard.guent...@gmail.com>:
>>> On Mon, Jan 18, 2016 at 3:50 PM, Yuri Rumyantsev <ysrum...@gmail.com> wrote:
>>>> Richard,
>>>>
>>>> Here is the second part of patch which really preforms mask stores and
>>>> all statements related to it to new basic block guarded by test on
>>>> zero mask. Hew test is also added.
>>>>
>>>> Is it OK for trunk?
>>>
>>> + /* Pick up all masked stores in loop if any. */
>>> + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>>> + {
>>> + stmt = gsi_stmt (gsi);
>>>
>>> you fail to iterate over all BBs of the loop here. Please follow
>>> other uses in the
>>> vectorizer.
>>>
>>> + while (!worklist.is_empty ())
>>> + {
>>> + gimple *last, *last_store;
>>> + edge e, efalse;
>>> + tree mask;
>>> + basic_block store_bb, join_bb;
>>> + gimple_stmt_iterator gsi_to;
>>> + /* tree arg3; */
>>>
>>> remove
>>>
>>> + tree vdef, new_vdef;
>>> + gphi *phi;
>>> + bool first_dump;
>>> + tree vectype;
>>> + tree zero;
>>> +
>>> + last = worklist.pop ();
>>> + mask = gimple_call_arg (last, 2);
>>> + /* Create new bb. */
>>>
>>> bb should be initialized from gimple_bb (last), not loop->header
>>>
>>> + e = split_block (bb, last);
>>>
>>> + gsi_from = gsi_for_stmt (stmt1);
>>> + gsi_to = gsi_start_bb (store_bb);
>>> + gsi_move_before (&gsi_from, &gsi_to);
>>> + update_stmt (stmt1);
>>>
>>> I think the update_stmt is redundant and you should be able to
>>> keep two gsis throughout the loop, from and to, no?
>>>
>>> + /* Put other masked stores with the same mask to STORE_BB. */
>>> + if (worklist.is_empty ()
>>> + || gimple_call_arg (worklist.last (), 2) != mask
>>> + || !is_valid_sink (worklist.last (), last_store))
>>>
>>> as I understand the code the last check is redundant with the invariant
>>> you track if you verify the stmt you breaked from the inner loop is
>>> actually equal to worklist.last () and you add a flag to track whether
>>> you did visit a load (vuse) in the sinking loop you didn't end up sinking.
>>>
>>> + /* Issue different messages depending on FIRST_DUMP. */
>>> + if (first_dump)
>>> + {
>>> + dump_printf_loc (MSG_NOTE, vect_location,
>>> + "Move MASK_STORE to new bb#%d\n",
>>> + store_bb->index);
>>> + first_dump = false;
>>> + }
>>> + else
>>> + dump_printf_loc (MSG_NOTE, vect_location,
>>> + "Move MASK_STORE to created bb\n");
>>>
>>> just add a separate dump when you create the BB, "Created new bb#%d for ..."
>>> to avoid this.
>>>
>>> Note that I can't comment on the x86 backend part so that will need to
>>> be reviewed by somebody
>>> else.
>>>
>>> Thanks,
>>> Richard.
>>>
>>>> Thanks.
>>>> Yuri.
>>>>
>>>> 2016-01-18 Yuri Rumyantsev <ysrum...@gmail.com>
>>>>
>>>> PR middle-end/68542
>>>> * config/i386/i386.c (ix86_expand_branch): Implement integral vector
>>>> comparison with boolean result.
>>>> * config/i386/sse.md (define_expand "cbranch<mode>4): Add define-expand
>>>> for vector comparion with eq/ne only.
>>>> * tree-vect-loop.c (is_valid_sink): New function.
>>>> (optimize_mask_stores): Likewise.
>>>> * tree-vect-stmts.c (vectorizable_mask_load_store): Initialize
>>>> has_mask_store field of vect_info.
>>>> * tree-vectorizer.c (vectorize_loops): Invoke optimaze_mask_stores for
>>>> vectorized loops having masked stores.
>>>> * tree-vectorizer.h (loop_vec_info): Add new has_mask_store field and
>>>> correspondent macros.
>>>> (optimize_mask_stores): Add prototype.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>> * gcc.dg/vect/vect-mask-store-move-1.c: New test.
>>>>
>>>> 2016-01-18 17:07 GMT+03:00 Richard Biener <richard.guent...@gmail.com>:
>>>>> On Mon, Jan 18, 2016 at 3:02 PM, Yuri Rumyantsev <ysrum...@gmail.com>
>>>>> wrote:
>>>>>> Thanks Richard.
>>>>>>
>>>>>> I changed the check on type as you proposed.
>>>>>>
>>>>>> What about the second back-end part of patch (it has been sent 08.12.15).
>>>>>
>>>>> Can't see it in my inbox - can you reply to the mail with a ping?
>>>>>
>>>>> Thanks,
>>>>> Richard.
>>>>>
>>>>>> Thanks.
>>>>>> Yuri.
>>>>>>
>>>>>> 2016-01-18 15:44 GMT+03:00 Richard Biener <richard.guent...@gmail.com>:
>>>>>>> On Mon, Jan 11, 2016 at 11:06 AM, Yuri Rumyantsev <ysrum...@gmail.com>
>>>>>>> wrote:
>>>>>>>> Hi Richard,
>>>>>>>>
>>>>>>>> Did you have anu chance to look at updated patch?
>>>>>>>
>>>>>>> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
>>>>>>> index acbb70b..208a752 100644
>>>>>>> --- a/gcc/tree-vrp.c
>>>>>>> +++ b/gcc/tree-vrp.c
>>>>>>> @@ -5771,6 +5771,10 @@ register_edge_assert_for (tree name, edge e,
>>>>>>> gimple_stmt_iterator si,
>>>>>>> &comp_code, &val))
>>>>>>> return;
>>>>>>>
>>>>>>> + /* VRP doesn't track ranges for vector types. */
>>>>>>> + if (TREE_CODE (TREE_TYPE (name)) == VECTOR_TYPE)
>>>>>>> + return;
>>>>>>> +
>>>>>>>
>>>>>>> please instead fix extract_code_and_val_from_cond_with_ops with
>>>>>>>
>>>>>>> Index: gcc/tree-vrp.c
>>>>>>> ===================================================================
>>>>>>> --- gcc/tree-vrp.c (revision 232506)
>>>>>>> +++ gcc/tree-vrp.c (working copy)
>>>>>>> @@ -5067,8 +5067,9 @@ extract_code_and_val_from_cond_with_ops
>>>>>>> if (invert)
>>>>>>> comp_code = invert_tree_comparison (comp_code, 0);
>>>>>>>
>>>>>>> - /* VRP does not handle float types. */
>>>>>>> - if (SCALAR_FLOAT_TYPE_P (TREE_TYPE (val)))
>>>>>>> + /* VRP only handles integral and pointer types. */
>>>>>>> + if (! INTEGRAL_TYPE_P (TREE_TYPE (val))
>>>>>>> + && ! POINTER_TYPE_P (TREE_TYPE (val)))
>>>>>>> return false;
>>>>>>>
>>>>>>> /* Do not register always-false predicates.
>>>>>>>
>>>>>>> Ok with that change.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Richard.
>>>>>>>
>>>>>>>> Thanks.
>>>>>>>> Yuri.
>>>>>>>>
>>>>>>>> 2015-12-18 13:20 GMT+03:00 Yuri Rumyantsev <ysrum...@gmail.com>:
>>>>>>>>> Hi Richard,
>>>>>>>>>
>>>>>>>>> Here is updated patch for middle-end part of the whole patch which
>>>>>>>>> fixes all your remarks I hope.
>>>>>>>>>
>>>>>>>>> Regression testing and bootstrapping did not show any new failures.
>>>>>>>>> Is it OK for trunk?
>>>>>>>>>
>>>>>>>>> Yuri.
>>>>>>>>>
>>>>>>>>> ChangeLog:
>>>>>>>>> 2015-12-18 Yuri Rumyantsev <ysrum...@gmail.com>
>>>>>>>>>
>>>>>>>>> PR middle-end/68542
>>>>>>>>> * fold-const.c (fold_binary_op_with_conditional_arg): Bail out for
>>>>>>>>> case
>>>>>>>>> of mixind vector and scalar types.
>>>>>>>>> (fold_relational_const): Add handling of vector
>>>>>>>>> comparison with boolean result.
>>>>>>>>> * tree-cfg.c (verify_gimple_comparison): Add argument CODE, allow
>>>>>>>>> comparison of vector operands with boolean result for EQ/NE only.
>>>>>>>>> (verify_gimple_assign_binary): Adjust call for
>>>>>>>>> verify_gimple_comparison.
>>>>>>>>> (verify_gimple_cond): Likewise.
>>>>>>>>> * tree-ssa-forwprop.c (combine_cond_expr_cond): Do not perform
