* [PATCH] Use signed boolean type for boolean vectors
@ 2015-10-28 13:33 Ilya Enkovich
2015-10-28 15:24 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Ilya Enkovich @ 2015-10-28 13:33 UTC (permalink / raw)
To: gcc-patches; +Cc: richard.guenther
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?
Thanks,
Ilya
--
gcc/
2015-10-28 Ilya Enkovich <enkovich.gnu@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
with precision greater than 1.
diff --git a/gcc/optabs.c b/gcc/optabs.c
index e1ac0b8..37a67f1 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -5373,7 +5373,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
{
@@ -5382,9 +5381,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 e77d4b8..712390f 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -1451,7 +1451,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;
@@ -8076,7 +8076,10 @@ build_nonstandard_boolean_type (unsigned HOST_WIDE_INT precision)
type = make_node (BOOLEAN_TYPE);
TYPE_PRECISION (type) = precision;
- fixup_unsigned_type (type);
+ if (precision > 1)
+ fixup_signed_type (type);
+ else
+ fixup_unsigned_type (type);
if (precision <= MAX_INT_CACHED_PREC)
nonstandard_boolean_type_cache[precision] = type;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Use signed boolean type for boolean vectors
2015-10-28 13:33 [PATCH] Use signed boolean type for boolean vectors Ilya Enkovich
@ 2015-10-28 15:24 ` Richard Biener
2015-10-28 15:31 ` Ilya Enkovich
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2015-10-28 15:24 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: GCC Patches
On Wed, Oct 28, 2015 at 2:13 PM, Ilya Enkovich <enkovich.gnu@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.
> Thanks,
> Ilya
> --
> gcc/
>
> 2015-10-28 Ilya Enkovich <enkovich.gnu@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
> with precision greater than 1.
>
>
> diff --git a/gcc/optabs.c b/gcc/optabs.c
> index e1ac0b8..37a67f1 100644
> --- a/gcc/optabs.c
> +++ b/gcc/optabs.c
> @@ -5373,7 +5373,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
> {
> @@ -5382,9 +5381,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 e77d4b8..712390f 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -1451,7 +1451,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;
>
> @@ -8076,7 +8076,10 @@ build_nonstandard_boolean_type (unsigned HOST_WIDE_INT precision)
>
> type = make_node (BOOLEAN_TYPE);
> TYPE_PRECISION (type) = precision;
> - fixup_unsigned_type (type);
> + if (precision > 1)
> + fixup_signed_type (type);
> + else
> + fixup_unsigned_type (type);
>
> if (precision <= MAX_INT_CACHED_PREC)
> nonstandard_boolean_type_cache[precision] = type;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Use signed boolean type for boolean vectors
2015-10-28 15:24 ` Richard Biener
@ 2015-10-28 15:31 ` Ilya Enkovich
2015-11-03 13:42 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Ilya Enkovich @ 2015-10-28 15:31 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
2015-10-28 18:21 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Oct 28, 2015 at 2:13 PM, Ilya Enkovich <enkovich.gnu@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.
Ilya
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Use signed boolean type for boolean vectors
2015-10-28 15:31 ` Ilya Enkovich
@ 2015-11-03 13:42 ` Richard Biener
2015-11-09 14:04 ` Ilya Enkovich
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2015-11-03 13:42 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: GCC Patches
On Wed, Oct 28, 2015 at 4:30 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2015-10-28 18:21 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>> On Wed, Oct 28, 2015 at 2:13 PM, Ilya Enkovich <enkovich.gnu@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.
>
> Ilya
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Use signed boolean type for boolean vectors
2015-11-03 13:42 ` Richard Biener
@ 2015-11-09 14:04 ` Ilya Enkovich
2015-11-09 14:27 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Ilya Enkovich @ 2015-11-09 14:04 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On 03 Nov 14:42, Richard Biener wrote:
> On Wed, Oct 28, 2015 at 4:30 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> > 2015-10-28 18:21 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
> >> On Wed, Oct 28, 2015 at 2:13 PM, Ilya Enkovich <enkovich.gnu@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?
Thanks,
Ilya
--
gcc/
2015-11-09 Ilya Enkovich <enkovich.gnu@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;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Use signed boolean type for boolean vectors
2015-11-09 14:04 ` Ilya Enkovich
@ 2015-11-09 14:27 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2015-11-09 14:27 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: GCC Patches
On Mon, Nov 9, 2015 at 3:03 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> On 03 Nov 14:42, Richard Biener wrote:
>> On Wed, Oct 28, 2015 at 4:30 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> > 2015-10-28 18:21 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>> >> On Wed, Oct 28, 2015 at 2:13 PM, Ilya Enkovich <enkovich.gnu@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.gnu@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;
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-09 14:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-28 13:33 [PATCH] Use signed boolean type for boolean vectors Ilya Enkovich
2015-10-28 15:24 ` Richard Biener
2015-10-28 15:31 ` Ilya Enkovich
2015-11-03 13:42 ` Richard Biener
2015-11-09 14:04 ` Ilya Enkovich
2015-11-09 14:27 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).