From: Markus Armbruster <armbru@redhat.com>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: Fangrui Song <i@maskray.me>,
qemu-devel@nongnu.org,
"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
Juan Quintela <quintela@redhat.com>
Subject: Re: [PATCH] Fix incorrect int->float conversions caught by clang -Wimplicit-int-float-conversion
Date: Thu, 21 Nov 2019 15:51:04 +0100 [thread overview]
Message-ID: <87mucpb7vr.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <f193df45-e4e7-808d-af20-cb98d8fcf96a@linaro.org> (Richard Henderson's message of "Thu, 21 Nov 2019 13:18:17 +0100")
Richard Henderson <richard.henderson@linaro.org> writes:
> On 11/20/19 6:30 PM, Fangrui Song wrote:
>> On 2019-11-20, Juan Quintela wrote:
>>> Markus Armbruster <armbru@redhat.com> wrote:
>>>> Fangrui Song <i@maskray.me> writes:
[...]
>>>>> diff --git a/util/cutils.c b/util/cutils.c
>>>>> index fd591cadf0..2b4484c015 100644
>>>>> --- a/util/cutils.c
>>>>> +++ b/util/cutils.c
>>>>> @@ -239,10 +239,10 @@ static int do_strtosz(const char *nptr, const char
>>>>> **end,
>>>>> goto out;
>>>>> }
>>>>> /*
>>>>> - * Values >= 0xfffffffffffffc00 overflow uint64_t after their trip
>>>>> + * Values > nextafter(0x1p64, 0) overflow uint64_t after their trip
>>>>> * through double (53 bits of precision).
>>>>> */
>>>>> - if ((val * mul >= 0xfffffffffffffc00) || val < 0) {
>>>>> + if ((val * mul > nextafter(0x1p64, 0)) || val < 0) {
>>>>> retval = -ERANGE;
>>>>> goto out;
>>>>> }
>>>
>>> This comment was really bad (it says the same that the code).
>>> On the other hand, I can *kind of* understand what does 0xffff<more
>>> f's here>.
>>>
>>> But I am at a complete loss about what value is:
>>>
>>> nextafter(0x1p64, 0).
>>>
>>> Can we put what value is that instead?
>>
>> It is a C99 hexadecimal floating-point literal.
>> 0x1p64 represents hex fraction 1.0 scaled by 2**64, that is 2**64.
>>
>> We can write this as `val * mul > 0xfffffffffffff800p0`, but I feel that
>> counting the number of f's is error-prone and is not fun.
>>
>> (We cannot use val * mul >= 0x1p64.
>> If FLT_EVAL_METHOD == 2, the intermediate computation val * mul will be
>> performed at long double precision, val * mul may not by representable
>> by a double and will overflow as (double)0x1p64.)
>
> I agree about not spelling out the f's, or the 0x800 at the end. That's
> something that the compiler can do for us, resolving this standard library
> function at compile-time.
>
> We just need a better comment. Perhaps:
>
> /*
> * Values near UINT64_MAX overflow to 2**64 when converting
> * to double precision. Compare against the maximum representable
> * double precision value below 2**64, computed as "the next value
> * after 2**64 (0x1p64) in the direction of 0".
> */
Yes, please.
next prev parent reply other threads:[~2019-11-21 14:51 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-16 1:07 [PATCH] Fix incorrect int->float conversions caught by clang -Wimplicit-int-float-conversion Fangrui Song
2019-11-19 15:14 ` Markus Armbruster
2019-11-20 11:15 ` Juan Quintela
2019-11-20 17:30 ` Fangrui Song
2019-11-21 12:18 ` Richard Henderson
2019-11-21 14:51 ` Markus Armbruster [this message]
2019-11-21 17:11 ` Fangrui Song
2019-11-19 20:49 ` Fangrui Song
2019-11-21 17:38 ` Eric Blake
2019-11-22 0:00 ` [PATCH v2] Fix incorrect integer->float " Fangrui Song
2019-11-22 8:06 ` Markus Armbruster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87mucpb7vr.fsf@dusky.pond.sub.org \
--to=armbru@redhat.com \
--cc=dgilbert@redhat.com \
--cc=i@maskray.me \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
--cc=richard.henderson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).