Re: [PATCH] Fix ICE for boolean comparison

[Ilya Enkovich]

2015-11-13 14:28 GMT+03:00 Richard Biener :
> On Fri, Nov 13, 2015 at 11:52 AM, Ilya Enkovich  
> wrote:
>> 2015-11-13 13:38 GMT+03:00 Richard Biener :
>>> On Thu, Nov 12, 2015 at 4:44 PM, Ilya Enkovich  
>>> wrote:
 Hi,

 Currently compiler may ICE when loaded boolean is compared with vector 
 invariant or another boolean value.  This is because we don't detect mix 
 of bool and non-bool vectypes and incorrectly determine vectype for 
 boolean loop invariant for comparison.  This was fixed for COND_EXP before 
 but also needs to be fixed for comparison.  This patch was bootstrapped 
 and tested on x86_64-unknown-linux-gnu.  OK for trunk?
>>>
>>> Hmm, so this disables vectorization in these cases.  Isn't this a
>>> regression?  Shouldn't we simply "materialize"
>>> the non-bool vector from the boolean one say, with
>>>
>>>  vec = boolvec ? {-1, -1 ... } : {0, 0, 0 ...}
>>
>> We may do this using patterns, but still should catch cases when
>> patterns don't catch it. Patterns don't have vectypes computed and
>> therefore may miss such cases. Thus stability fix is still valid.
>>
>> I don't think we have a compiler version which can vectorize
>> simd-bool-comparison-2.cc, thus technically it is not a regression.
>> There are also other similar cases, e.g. store of comparison result or
>> use loaded boolean as a predicate. I was going to support
>> vectorization for such cases later (seems I don't hit stage1 for them
>> and not sure if it will be OK for stage3).
>
> I still think those checks show that there is an issue we should fix
> differently.  We're accumulating more mess into the already messy
> vectorizer :(

Right. Earlier vectype computation would let to reveal such cases more easily.

>
> Ok.

Thanks!
Ilya

>
> Thanks,
> Richard.
>
>> Ilya
>>
>>>
>>> ?
>>>
>>> Thanks,
>>> Richard.
>>>
 Thanks,
 Ilya

Re: [PATCH] Fix ICE for boolean comparison

[Richard Biener]

On Fri, Nov 13, 2015 at 11:52 AM, Ilya Enkovich  wrote:
> 2015-11-13 13:38 GMT+03:00 Richard Biener :
>> On Thu, Nov 12, 2015 at 4:44 PM, Ilya Enkovich  
>> wrote:
>>> Hi,
>>>
>>> Currently compiler may ICE when loaded boolean is compared with vector 
>>> invariant or another boolean value.  This is because we don't detect mix of 
>>> bool and non-bool vectypes and incorrectly determine vectype for boolean 
>>> loop invariant for comparison.  This was fixed for COND_EXP before but also 
>>> needs to be fixed for comparison.  This patch was bootstrapped and tested 
>>> on x86_64-unknown-linux-gnu.  OK for trunk?
>>
>> Hmm, so this disables vectorization in these cases.  Isn't this a
>> regression?  Shouldn't we simply "materialize"
>> the non-bool vector from the boolean one say, with
>>
>>  vec = boolvec ? {-1, -1 ... } : {0, 0, 0 ...}
>
> We may do this using patterns, but still should catch cases when
> patterns don't catch it. Patterns don't have vectypes computed and
> therefore may miss such cases. Thus stability fix is still valid.
>
> I don't think we have a compiler version which can vectorize
> simd-bool-comparison-2.cc, thus technically it is not a regression.
> There are also other similar cases, e.g. store of comparison result or
> use loaded boolean as a predicate. I was going to support
> vectorization for such cases later (seems I don't hit stage1 for them
> and not sure if it will be OK for stage3).

I still think those checks show that there is an issue we should fix
differently.  We're accumulating more mess into the already messy
vectorizer :(

Ok.

Thanks,
Richard.

> Ilya
>
>>
>> ?
>>
>> Thanks,
>> Richard.
>>
>>> Thanks,
>>> Ilya

Re: [PATCH] Fix ICE for boolean comparison

[Ilya Enkovich]

2015-11-13 13:38 GMT+03:00 Richard Biener :
> On Thu, Nov 12, 2015 at 4:44 PM, Ilya Enkovich  wrote:
>> Hi,
>>
>> Currently compiler may ICE when loaded boolean is compared with vector 
>> invariant or another boolean value.  This is because we don't detect mix of 
>> bool and non-bool vectypes and incorrectly determine vectype for boolean 
>> loop invariant for comparison.  This was fixed for COND_EXP before but also 
>> needs to be fixed for comparison.  This patch was bootstrapped and tested on 
>> x86_64-unknown-linux-gnu.  OK for trunk?
>
> Hmm, so this disables vectorization in these cases.  Isn't this a
> regression?  Shouldn't we simply "materialize"
> the non-bool vector from the boolean one say, with
>
>  vec = boolvec ? {-1, -1 ... } : {0, 0, 0 ...}

We may do this using patterns, but still should catch cases when
patterns don't catch it. Patterns don't have vectypes computed and
therefore may miss such cases. Thus stability fix is still valid.

I don't think we have a compiler version which can vectorize
simd-bool-comparison-2.cc, thus technically it is not a regression.
There are also other similar cases, e.g. store of comparison result or
use loaded boolean as a predicate. I was going to support
vectorization for such cases later (seems I don't hit stage1 for them
and not sure if it will be OK for stage3).

Ilya

>
> ?
>
> Thanks,
> Richard.
>
>> Thanks,
>> Ilya

Re: [PATCH] Fix ICE for boolean comparison

[Richard Biener]

On Thu, Nov 12, 2015 at 4:44 PM, Ilya Enkovich  wrote:
> Hi,
>
> Currently compiler may ICE when loaded boolean is compared with vector 
> invariant or another boolean value.  This is because we don't detect mix of 
> bool and non-bool vectypes and incorrectly determine vectype for boolean loop 
> invariant for comparison.  This was fixed for COND_EXP before but also needs 
> to be fixed for comparison.  This patch was bootstrapped and tested on 
> x86_64-unknown-linux-gnu.  OK for trunk?

Hmm, so this disables vectorization in these cases.  Isn't this a
regression?  Shouldn't we simply "materialize"
the non-bool vector from the boolean one say, with

 vec = boolvec ? {-1, -1 ... } : {0, 0, 0 ...}

?

Thanks,
Richard.

> Thanks,
> Ilya
> --
> gcc/
>
> 2015-11-12  Ilya Enkovich  
>
> * tree-vect-loop.c (vect_determine_vectorization_factor): Check
> mix of boolean and integer vectors in a single statement.
> * tree-vect-slp.c (vect_mask_constant_operand_p): New.
> (vect_get_constant_vectors): Use vect_mask_constant_operand_p to
> determine constant type.
> * tree-vect-stmts.c (vectorizable_comparison): Provide vectype
> for loop invariants.
>
> gcc/testsuite/
>
> 2015-11-12  Ilya Enkovich  
>
> * g++.dg/vect/simd-bool-comparison-1.cc: New test.
> * g++.dg/vect/simd-bool-comparison-2.cc: New test.
>
>
> diff --git a/gcc/testsuite/g++.dg/vect/simd-bool-comparison-1.cc 
> b/gcc/testsuite/g++.dg/vect/simd-bool-comparison-1.cc
> new file mode 100644
> index 000..a08362f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/vect/simd-bool-comparison-1.cc
> @@ -0,0 +1,21 @@
> +// { dg-do compile }
> +// { dg-additional-options "-mavx512bw -mavx512dq" { target { i?86-*-* 
> x86_64-*-* } } }
> +
> +#define N 1024
> +
> +double a[N];
> +bool b[N];
> +bool c;
> +
> +void test ()
> +{
> +  int i;
> +
> +  for (i = 0; i < N; i++)
> +if (b[i] != c)
> +  a[i] = 0.0;
> +else
> +  a[i] = 1.0;
> +}
> +
> +// { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target 
> { i?86-*-* x86_64-*-* } } } }
> diff --git a/gcc/testsuite/g++.dg/vect/simd-bool-comparison-2.cc 
> b/gcc/testsuite/g++.dg/vect/simd-bool-comparison-2.cc
> new file mode 100644
> index 000..4accf56
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/vect/simd-bool-comparison-2.cc
> @@ -0,0 +1,20 @@
> +// { dg-do compile }
> +// { dg-additional-options "-mavx512bw -mavx512dq" { target { i?86-*-* 
> x86_64-*-* } } }
> +
> +#define N 1024
> +
> +double a[N];
> +bool b[N];
> +char c[N];
> +
> +void test ()
> +{
> +  int i;
> +
> +  #pragma omp simd
> +  for (i = 0; i < N; i++)
> +if ((c[i] > 0) && b[i])
> +  a[i] = 0.0;
> +else
> +  a[i] = 1.0;
> +}
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index 55e5309..6b78b55 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -649,7 +649,32 @@ vect_determine_vectorization_factor (loop_vec_info 
> loop_vinfo)
> }
>   return false;
> }
> + else if (VECTOR_BOOLEAN_TYPE_P (mask_type)
> +  != VECTOR_BOOLEAN_TYPE_P (vectype))
> +   {
> + if (dump_enabled_p ())
> +   {
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> +  "not vectorized: mixed mask and "
> +  "nonmask vector types in statement, ");
> + dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM,
> +mask_type);
> + dump_printf (MSG_MISSED_OPTIMIZATION, " and ");
> + dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM,
> +vectype);
> + dump_printf (MSG_MISSED_OPTIMIZATION, "\n");
> +   }
> + return false;
> +   }
> }
> +
> + /* We may compare boolean value loaded as vector of integers.
> +Fix mask_type in such case.  */
> + if (mask_type
> + && !VECTOR_BOOLEAN_TYPE_P (mask_type)
> + && gimple_code (stmt) == GIMPLE_ASSIGN
> + && TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) == 
> tcc_comparison)
> +   mask_type = build_same_sized_truth_vector_type (mask_type);
> }
>
>/* No mask_type should mean loop invariant predicate.
> diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
> index 9d97140..f3acb04 100644
> --- a/gcc/tree-vect-slp.c
> +++ b/gcc/tree-vect-slp.c
> @@ -2589,6 +2589,57 @@ vect_slp_bb (basic_block bb)
>  }
>
>
> +/* Return 1 if vector type of boolean constant which is OPNUM
> +   operand in statement STMT is a boolean vector.  */
> +
> +static bool
> +vect_mask_constant_operand_p (gimple *stmt, int opnum)
> +{
> +