On 2019-11-21, Markus Armbruster wrote:
>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.

Thanks for the suggestion. Attached a new patch.