On Mon, Nov 9, 2015 at 3:03 PM, Ilya Enkovich <enkovich....@gmail.com> wrote: > On 03 Nov 14:42, Richard Biener wrote: >> On Wed, Oct 28, 2015 at 4:30 PM, Ilya Enkovich <enkovich....@gmail.com> >> wrote: >> > 2015-10-28 18:21 GMT+03:00 Richard Biener <richard.guent...@gmail.com>: >> >> On Wed, Oct 28, 2015 at 2:13 PM, Ilya Enkovich <enkovich....@gmail.com> >> >> wrote: >> >>> Hi, >> >>> >> >>> Testing boolean vector conversions I found several runtime regressions >> >>> and investigation showed it's due to incorrect conversion caused by >> >>> unsigned boolean type. When boolean vector is represented as an >> >>> integer vector on target it's a signed integer actually. Unsigned >> >>> boolean type was chosen due to possible single bit values, but for >> >>> multiple bit values it causes wrong casting. The easiest way to fix >> >>> it is to use signed boolean value. The following patch does this and >> >>> fixes my problems with conversion. Bootstrapped and tested on >> >>> x86_64-unknown-linux-gnu. Is it OK? >> >> >> >> Hmm. Actually formally the "boolean" vectors were always 0 or -1 >> >> (all bits set). That is also true for a signed boolean with precision 1 >> >> but with higher precision what makes sure to sign-extend 'true'? >> >> >> >> So it's far from an obvious change, esp as you don't change the >> >> precision == 1 case. [I still think we should have precision == 1 >> >> for all boolean types] >> >> >> >> Richard. >> >> >> > >> > For 1 bit precision signed type value 1 is out of range, right? This might >> > break >> > in many place due to used 1 as true value. >> >> For vectors -1 is true. Did you try whether it breaks many places? >> build_int_cst (type, 1) should still work fine. >> >> Richard. >> > > I tried it and didn't find any new failures. So looks I was wrong assuming > it should cause many failures. Testing is not complete because many SPEC > benchmarks are failing to compile on -O3 for AVX-512 on trunk. But I think > we may proceed with signed type and fix constant generation issues if any > revealed. This patch was bootstrapped and regtested on > x86_64-unknown-linux-gnu. OK for trunk?
Ok. Richard. > Thanks, > Ilya > -- > gcc/ > > 2015-11-09 Ilya Enkovich <enkovich....@gmail.com> > > * optabs.c (expand_vec_cond_expr): Always get sign from type. > * tree.c (wide_int_to_tree): Support negative values for boolean. > (build_nonstandard_boolean_type): Use signed type for booleans. > > > diff --git a/gcc/optabs.c b/gcc/optabs.c > index fdcdc6a..44971ad 100644 > --- a/gcc/optabs.c > +++ b/gcc/optabs.c > @@ -5365,7 +5365,6 @@ expand_vec_cond_expr (tree vec_cond_type, tree op0, > tree op1, tree op2, > op0a = TREE_OPERAND (op0, 0); > op0b = TREE_OPERAND (op0, 1); > tcode = TREE_CODE (op0); > - unsignedp = TYPE_UNSIGNED (TREE_TYPE (op0a)); > } > else > { > @@ -5374,9 +5373,9 @@ expand_vec_cond_expr (tree vec_cond_type, tree op0, > tree op1, tree op2, > op0a = op0; > op0b = build_zero_cst (TREE_TYPE (op0)); > tcode = LT_EXPR; > - unsignedp = false; > } > cmp_op_mode = TYPE_MODE (TREE_TYPE (op0a)); > + unsignedp = TYPE_UNSIGNED (TREE_TYPE (op0a)); > > > gcc_assert (GET_MODE_SIZE (mode) == GET_MODE_SIZE (cmp_op_mode) > diff --git a/gcc/tree.c b/gcc/tree.c > index 18d6544..6fb4c09 100644 > --- a/gcc/tree.c > +++ b/gcc/tree.c > @@ -1437,7 +1437,7 @@ wide_int_to_tree (tree type, const wide_int_ref &pcst) > case BOOLEAN_TYPE: > /* Cache false or true. */ > limit = 2; > - if (hwi < 2) > + if (IN_RANGE (hwi, 0, 1)) > ix = hwi; > break; > > @@ -8069,7 +8069,7 @@ build_nonstandard_boolean_type (unsigned HOST_WIDE_INT > precision) > > type = make_node (BOOLEAN_TYPE); > TYPE_PRECISION (type) = precision; > - fixup_unsigned_type (type); > + fixup_signed_type (type); > > if (precision <= MAX_INT_CACHED_PREC) > nonstandard_boolean_type_cache[precision] = type;