* wide int knowledge/bug?
@ 2020-10-27 17:18 Andrew MacLeod
2020-10-27 17:35 ` Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Andrew MacLeod @ 2020-10-27 17:18 UTC (permalink / raw)
To: GCC
I was looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97596
and the ranger is constructing some 128 bit constants and calling
wide_int_to_tree to turn them into trees.
In particular, it starts with the value
p r.lower_bound(0)
{<wide_int_storage> = {val = {-65535, 9223372036854775807,
140737488257608}, len = 2, precision = 128}, static is_sign_extended = true}
p r.lower_bound(0).dump()
[0x7fffffffffffffff,0xffffffffffff0001], precision = 128
and proceeds to call
wide_int new_lb = wi::set_bit (r.lower_bound (0), 127)
and creates the value:
p new_lb
{<wide_int_storage> = {val = {-65535, -1, 0}, len = 2, precision = 128},
static is_sign_extended = true}
p new_lb.dump()
[0xffffffffffffffff,0xffffffffffff0001], precision = 128
but when we try to call wide_int_to_tree() on this value, it dies in the
second assert (this is the trap in the PR) :
/* Verify that everything is canonical. */
int l = pcst.get_len ();
if (l > 1)
{
if (pcst.elt (l - 1) == 0)
gcc_checking_assert (pcst.elt (l - 2) < 0);
if (pcst.elt (l - 1) == HOST_WIDE_INT_M1)
gcc_checking_assert (pcst.elt (l - 2) >= 0);
<<<--- here.
}
If i look at the value, its:
p pcst.elt (l - 2)
$18 = -65535
which is clearly not >= 0... it seems to be treating it as a signed
value? So did the set_bit routine do something wrong when creating the
new value?
I cant really read this, but something seems amok...
any ideas?
Andrew
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: wide int knowledge/bug?
2020-10-27 17:18 wide int knowledge/bug? Andrew MacLeod
@ 2020-10-27 17:35 ` Jakub Jelinek
2020-10-27 17:46 ` Richard Sandiford
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2020-10-27 17:35 UTC (permalink / raw)
To: Andrew MacLeod; +Cc: GCC
On Tue, Oct 27, 2020 at 01:18:03PM -0400, Andrew MacLeod via Gcc wrote:
> I was looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97596
>
> and the ranger is constructing some 128 bit constants and calling
> wide_int_to_tree to turn them into trees.
>
> In particular, it starts with the value
>
> p r.lower_bound(0)
> {<wide_int_storage> = {val = {-65535, 9223372036854775807, 140737488257608},
> len = 2, precision = 128}, static is_sign_extended = true}
>
> p r.lower_bound(0).dump()
> [0x7fffffffffffffff,0xffffffffffff0001], precision = 128
>
>
> Â and proceeds to call
>
> wide_int new_lb = wi::set_bit (r.lower_bound (0), 127)
>
> and creates the value:
>
> p new_lb
> {<wide_int_storage> = {val = {-65535, -1, 0}, len = 2, precision = 128},
> static is_sign_extended = true}
This is non-canonical and so invalid, if the low HWI has the MSB set
and the high HWI is -1, it should have been just
val = {-65535}, len = 1, precision = 128}
I guess the bug is that wi::set_bit_large doesn't call canonize.
So perhaps totally untested:
--- gcc/wide-int.cc.jj 2020-10-19 18:42:41.134426398 +0200
+++ gcc/wide-int.cc 2020-10-27 18:33:38.546703763 +0100
@@ -702,8 +702,11 @@ wi::set_bit_large (HOST_WIDE_INT *val, c
/* If the bit we just set is at the msb of the block, make sure
that any higher bits are zeros. */
if (bit + 1 < precision && subbit == HOST_BITS_PER_WIDE_INT - 1)
- val[len++] = 0;
- return len;
+ {
+ val[len++] = 0;
+ return len;
+ }
+ return canonize (val, len, precision);
}
else
{
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: wide int knowledge/bug?
2020-10-27 17:35 ` Jakub Jelinek
@ 2020-10-27 17:46 ` Richard Sandiford
2020-10-27 18:02 ` Andrew MacLeod
0 siblings, 1 reply; 4+ messages in thread
From: Richard Sandiford @ 2020-10-27 17:46 UTC (permalink / raw)
To: Jakub Jelinek via Gcc; +Cc: Andrew MacLeod, Jakub Jelinek
Jakub Jelinek via Gcc <gcc@gcc.gnu.org> writes:
> On Tue, Oct 27, 2020 at 01:18:03PM -0400, Andrew MacLeod via Gcc wrote:
>> I was looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97596
>>
>> and the ranger is constructing some 128 bit constants and calling
>> wide_int_to_tree to turn them into trees.
>>
>> In particular, it starts with the value
>>
>> p r.lower_bound(0)
>> {<wide_int_storage> = {val = {-65535, 9223372036854775807, 140737488257608},
>> len = 2, precision = 128}, static is_sign_extended = true}
>>
>> p r.lower_bound(0).dump()
>> [0x7fffffffffffffff,0xffffffffffff0001], precision = 128
>>
>>
>> Â and proceeds to call
>>
>> wide_int new_lb = wi::set_bit (r.lower_bound (0), 127)
>>
>> and creates the value:
>>
>> p new_lb
>> {<wide_int_storage> = {val = {-65535, -1, 0}, len = 2, precision = 128},
>> static is_sign_extended = true}
>
> This is non-canonical and so invalid, if the low HWI has the MSB set
> and the high HWI is -1, it should have been just
> val = {-65535}, len = 1, precision = 128}
>
> I guess the bug is that wi::set_bit_large doesn't call canonize.
Yeah, looks like a micro-optimisation gone wrong.
> So perhaps totally untested:
>
> --- gcc/wide-int.cc.jj 2020-10-19 18:42:41.134426398 +0200
> +++ gcc/wide-int.cc 2020-10-27 18:33:38.546703763 +0100
> @@ -702,8 +702,11 @@ wi::set_bit_large (HOST_WIDE_INT *val, c
> /* If the bit we just set is at the msb of the block, make sure
> that any higher bits are zeros. */
> if (bit + 1 < precision && subbit == HOST_BITS_PER_WIDE_INT - 1)
> - val[len++] = 0;
> - return len;
> + {
> + val[len++] = 0;
> + return len;
> + }
> + return canonize (val, len, precision);
> }
> else
> {
LGTM, thanks.
Richard
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: wide int knowledge/bug?
2020-10-27 17:46 ` Richard Sandiford
@ 2020-10-27 18:02 ` Andrew MacLeod
0 siblings, 0 replies; 4+ messages in thread
From: Andrew MacLeod @ 2020-10-27 18:02 UTC (permalink / raw)
To: Jakub Jelinek via Gcc, Jakub Jelinek, richard.sandiford
On 10/27/20 1:46 PM, Richard Sandiford wrote:
> Jakub Jelinek via Gcc <gcc@gcc.gnu.org> writes:
>> On Tue, Oct 27, 2020 at 01:18:03PM -0400, Andrew MacLeod via Gcc wrote:
>>> I was looking at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97596
>>>
>>> and the ranger is constructing some 128 bit constants and calling
>>> wide_int_to_tree to turn them into trees.
>>>
>>> In particular, it starts with the value
>>>
>>> p r.lower_bound(0)
>>> {<wide_int_storage> = {val = {-65535, 9223372036854775807, 140737488257608},
>>> len = 2, precision = 128}, static is_sign_extended = true}
>>>
>>> p r.lower_bound(0).dump()
>>> [0x7fffffffffffffff,0xffffffffffff0001], precision = 128
>>>
>>>
>>> Â and proceeds to call
>>>
>>> wide_int new_lb = wi::set_bit (r.lower_bound (0), 127)
>>>
>>> and creates the value:
>>>
>>> p new_lb
>>> {<wide_int_storage> = {val = {-65535, -1, 0}, len = 2, precision = 128},
>>> static is_sign_extended = true}
>> This is non-canonical and so invalid, if the low HWI has the MSB set
>> and the high HWI is -1, it should have been just
>> val = {-65535}, len = 1, precision = 128}
>>
>> I guess the bug is that wi::set_bit_large doesn't call canonize.
> Yeah, looks like a micro-optimisation gone wrong.
>
>> So perhaps totally untested:
>>
>> --- gcc/wide-int.cc.jj 2020-10-19 18:42:41.134426398 +0200
>> +++ gcc/wide-int.cc 2020-10-27 18:33:38.546703763 +0100
>> @@ -702,8 +702,11 @@ wi::set_bit_large (HOST_WIDE_INT *val, c
>> /* If the bit we just set is at the msb of the block, make sure
>> that any higher bits are zeros. */
>> if (bit + 1 < precision && subbit == HOST_BITS_PER_WIDE_INT - 1)
>> - val[len++] = 0;
>> - return len;
>> + {
>> + val[len++] = 0;
>> + return len;
>> + }
>> + return canonize (val, len, precision);
>> }
>> else
>> {
> LGTM, thanks.
>
> Richard
>
Seems to resolve my problem.
want me to run this thru the tests and apply it to this PR, or is that
already underway? :-)
Andrew
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-27 18:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-27 17:18 wide int knowledge/bug? Andrew MacLeod
2020-10-27 17:35 ` Jakub Jelinek
2020-10-27 17:46 ` Richard Sandiford
2020-10-27 18:02 ` Andrew MacLeod
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).