Re: [RFC] Try vector<bool> as a new representation for vector masks

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Sep 15, 2015 at 3:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> On 08 Sep 15:37, Ilya Enkovich wrote:
>> 2015-09-04 23:42 GMT+03:00 Jeff Law <law@redhat.com>:
>> >
>> > So do we have enough confidence in this representation that we want to go
>> > ahead and commit to it?
>>
>> I think new representation fits nice mostly. There are some places
>> where I have to make some exceptions for vector of bools to make it
>> work. This is mostly to avoid target modifications. I'd like to avoid
>> necessity to change all targets currently supporting vec_cond. It
>> makes me add some special handling of vec<bool> in GIMPLE, e.g. I add
>> a special code in vect_init_vector to build vec<bool> invariants with
>> proper casting to int. Otherwise I'd need to do it on a target side.
>>
>> I made several fixes and current patch (still allowing integer vector
>> result for vector comparison and applying bool patterns) passes
>> bootstrap and regression testing on x86_64. Now I'll try to fully
>> switch to vec<bool> and see how it goes.
>>
>> Thanks,
>> Ilya
>>
>
> Hi,
>
> I made a step forward forcing vector comparisons have a mask (vec<bool>) result and disabling bool patterns in case vector comparison is supported by target.  Several issues were met.
>
>  - c/c++ front-ends generate vector comparison with integer vector result.  I had to make some modifications to use vec_cond instead.  Don't know if there are other front-ends producing vector comparisons.
>  - vector lowering fails to expand vector masks due to mismatch of type and mode sizes.  I fixed vector type size computation to match mode size and added a special handling of mask expand.
>  - I disabled canonical type creation for vector mask because we can't layout it with VOID mode. I don't know why we may need a canonical type here.  But get_mask_mode call may be moved into type layout to get it.
>  - Expand of vec<bool> constants/contstructors requires special handling.  Common case should require target hooks/optabs to expand vector into required mode.  But I suppose we want to have a generic code to handle vector of int mode case to avoid modification of multiple targets which use default vec<bool> modes.

One complication you might run into currently is that at the moment we
require the comparison result to be
of the same size as the comparison operands.  This means that
vector<bool> with, say, 4 elements has
to support different modes for v4si < v4si vs. v4df < v4df (if you
think of x86 with its multiple vector sizes).
That's for the "fallback" non-mask vector<bool> only of course.  Does
that mean we have to use different
bool types with different modes here?

So the other possibility is to never expose the fallback vector<bool>
anywhere but make sure to lower to
vector<int> via VEC_COND_EXPRs.  After all it's only the vectorizer
that should create stmts with
vector<bool> LHS and the vectorizer is already careful to only
generate code supported by the target.

> Currently 'make check' shows two types of regression.
>   - missed vector expression pattern recongnition (MIN, MAX, ABX, VEC_COND).  This must be due to my front-end changes.  Hope it will be easy to fix.
>   - missed vectorization. All of them appear due to bool patterns disabling.  I didn't look into all of them but it seems the main problem is in mixed type sizes.  With bool patterns and integer vector masks we just put int->(other sized int) conversion for masks and it gives us required mask transformation.  With boolean mask we don't have a proper scalar statements to do that.  I think mask widening/narrowing may be directly supported in masked statements vectorization.  Going to look into it.
>
> I attach what I currently have for a prototype.  It grows bigger so I split into several parts.
>
> Thanks,
> Ilya
> --
> * avx512-vec-bool-01-add-truth-vector.ChangeLog
>
> 2015-09-15  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * doc/tm.texi: Regenerated.
>         * doc/tm.texi.in (TARGET_VECTORIZE_GET_MASK_MODE): New.
>         * stor-layout.c (layout_type): Use mode to get vector mask size.
>         (vector_type_mode): Likewise.
>         * target.def (get_mask_mode): New.
>         * targhooks.c (default_vector_alignment): Use mode alignment
>         for vector masks.
>         (default_get_mask_mode): New.
>         * targhooks.h (default_get_mask_mode): New.
>         * tree.c (make_vector_type): Vector mask has no canonical type.
>         (build_truth_vector_type): New.
>         (build_same_sized_truth_vector_type): New.
>         (truth_type_for): Support vector masks.
>         * tree.h (VECTOR_MASK_TYPE_P): New.
>         (build_truth_vector_type): New.
>         (build_same_sized_truth_vector_type): New.
>
> * avx512-vec-bool-02-no-int-vec-cmp.ChangeLog
>
> gcc/
>
> 2015-09-15  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * tree-cfg.c (verify_gimple_comparison) Require vector mask
>         type for vector comparison.
>         (verify_gimple_assign_ternary): Likewise.
>
> gcc/c
>
> 2015-09-15  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * c-typeck.c (build_conditional_expr): Use vector mask
>         type for vector comparison.
>         (build_vec_cmp): New.
>         (build_binary_op): Use build_vec_cmp for comparison.
>
> gcc/cp
>
> 2015-09-15  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * call.c (build_conditional_expr_1): Use vector mask
>         type for vector comparison.
>         * typeck.c (build_vec_cmp): New.
>         (cp_build_binary_op): Use build_vec_cmp for comparison.
>
> * avx512-vec-bool-03-vec-lower.ChangeLog
>
> 2015-09-15  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * tree-vect-generic.c (tree_vec_extract): Use additional
>         comparison when extracting boolean value.
>         (do_bool_compare): New.
>         (expand_vector_comparison): Add casts for vector mask.
>         (expand_vector_divmod): Use vector mask type for vector
>         comparison.
>         (expand_vector_operations_1) Skip scalar mode mask statements.
>
> * avx512-vec-bool-04-vectorize.ChangeLog
>
> gcc/
>
> 2015-09-15  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * expr.c (do_store_flag): Use expand_vec_cmp_expr for mask results.
>         (const_vector_mask_from_tree): New.
>         (const_vector_from_tree): Use const_vector_mask_from_tree for vector
>         masks.
>         * internal-fn.c (expand_MASK_LOAD): Adjust to optab changes.
>         (expand_MASK_STORE): Likewise.
>         * optabs.c (vector_compare_rtx): Add OPNO arg.
>         (expand_vec_cond_expr): Adjust to vector_compare_rtx change.
>         (get_vec_cmp_icode): New.
>         (expand_vec_cmp_expr_p): New.
>         (expand_vec_cmp_expr): New.
>         (can_vec_mask_load_store_p): Add MASK_MODE arg.
>         * optabs.def (vec_cmp_optab): New.
>         (vec_cmpu_optab): New.
>         (maskload_optab): Transform into convert optab.
>         (maskstore_optab): Likewise.
>         * optabs.h (expand_vec_cmp_expr_p): New.
>         (expand_vec_cmp_expr): New.
>         (can_vec_mask_load_store_p): Add MASK_MODE arg.
>         * tree-if-conv.c (ifcvt_can_use_mask_load_store): Adjust to
>         can_vec_mask_load_store_p signature change.
>         (predicate_mem_writes): Use boolean mask.
>         * tree-vect-data-refs.c (vect_get_new_vect_var): Support vect_mask_var.
>         (vect_create_destination_var): Likewise.
>         * tree-vect-loop.c (vect_determine_vectorization_factor): Ignore mask
>         operations for VF.  Add mask type computation.
>         * tree-vect-stmts.c (vect_init_vector): Support mask invariants.
>         (vect_get_vec_def_for_operand): Support mask constant.
>         (vectorizable_mask_load_store): Adjust to can_vec_mask_load_store_p
>         signature change.
>         (vectorizable_condition): Use vector mask type for vector comparison.
>         (vectorizable_comparison): New.
>         (vect_analyze_stmt): Add vectorizable_comparison.
>         (vect_transform_stmt): Likewise.
>         (get_mask_type_for_scalar_type): New.
>         * tree-vectorizer.h (enum stmt_vec_info_type): Add vect_mask_var
>         (enum stmt_vec_info_type): Add comparison_vec_info_type.
>         (get_mask_type_for_scalar_type): New.
>
> * avx512-vec-bool-05-bool-patterns.ChangeLog
>
> 2015-09-15  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * tree-vect-patterns.c (check_bool_pattern): Check fails
>         if we can vectorize comparison directly.
>         (search_type_for_mask): New.
>         (vect_recog_bool_pattern): Support cases when bool pattern
>         check fails.
>
> * avx512-vec-bool-06-i386.ChangeLog
>
> 2015-09-15  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * config/i386/i386-protos.h (ix86_expand_mask_vec_cmp): New.
>         (ix86_expand_int_vec_cmp): New.
>         (ix86_expand_fp_vec_cmp): New.
>         * config/i386/i386.c (ix86_expand_sse_cmp): Allow NULL for
>         op_true and op_false.
>         (ix86_int_cmp_code_to_pcmp_immediate): New.
>         (ix86_fp_cmp_code_to_pcmp_immediate): New.
>         (ix86_cmp_code_to_pcmp_immediate): New.
>         (ix86_expand_mask_vec_cmp): New.
>         (ix86_expand_fp_vec_cmp): New.
>         (ix86_expand_int_sse_cmp): New.
>         (ix86_expand_int_vcond): Use ix86_expand_int_sse_cmp.
>         (ix86_expand_int_vec_cmp): New.
>         (ix86_get_mask_mode): New.
>         (TARGET_VECTORIZE_GET_MASK_MODE): New.
>         * config/i386/sse.md (avx512fmaskmodelower): New.
>         (vec_cmp<mode><avx512fmaskmodelower>): New.
>         (vec_cmp<mode><sseintvecmodelower>): New.
>         (vec_cmpv2div2di): New.
>         (vec_cmpu<mode><avx512fmaskmodelower>): New.
>         (vec_cmpu<mode><sseintvecmodelower>): New.
>         (vec_cmpuv2div2di): New.
>         (maskload<mode>): Rename to ...
>         (maskload<mode><sseintvecmodelower>): ... this.
>         (maskstore<mode>): Rename to ...
>         (maskstore<mode><sseintvecmodelower>): ... this.
>         (maskload<mode><avx512fmaskmodelower>): New.
>         (maskstore<mode><avx512fmaskmodelower>): New.