Re: [Qemu-devel] [PATCH 2/2] qemu-common.h: optimise muldiv64 for x86_64 architecture

[Frediano Ziglio]

2015-01-09 12:22 GMT+00:00 Peter Maydell <address@hidden>:
> On 9 January 2015 at 11:25, Frediano Ziglio <address@hidden> wrote:
>> As this platform can do multiply/divide using 128 bit precision use
>> these instructions to implement it.
>>
>> Signed-off-by: Frediano Ziglio <address@hidden>
>> ---
>>  include/qemu-common.h | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/qemu-common.h b/include/qemu-common.h
>> index f3033ae..880659d 100644
>> --- a/include/qemu-common.h
>> +++ b/include/qemu-common.h
>> @@ -370,11 +370,23 @@ static inline uint8_t from_bcd(uint8_t val)
>>  }
>>
>>  /* compute with 96 bit intermediate result: (a*b)/c */
>> -#ifdef CONFIG_INT128
>> +#if defined(CONFIG_INT128) && !defined(__x86_64__)
>>  static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
>>  {
>>      return (__int128)a * b / c;
>>  }
>> +#elif defined(__x86_64__)
>> +/* Optimised x64 version. This assume that a*b/c fits in 64 bit */
>
> This assumption isn't necessarily true, and this implementation
> will dump core on overflow. For instance:
>
> Inputs: a = 8000000000000000 b = 80000000 c = 1
> fn muldiv64 result 0
> fn muldiv64_with_int128 result 0
> fn muldiv64_with_uint128 result 0
> Floating point exception (core dumped)
>
> -- PMM

Yes, I know, it was meant to be a precondition, not a math rule. Doing
some grep I'm not really sure this is valid in all cases however I
would ask if the truncation is handled correctly. Surely in some cases
the call to this function is not needed like in "muldiv64(1, tks,
usb_bit_time);" or in "muldiv64((int16_t) s->rtc.comp, 1000, 0x8000);"

Frediano