On 9 January 2015 at 15:41, Frediano Ziglio <fredd...@gmail.com> wrote: > 2015-01-09 12:22 GMT+00:00 Peter Maydell <peter.mayd...@linaro.org>: >>> +/* 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.
> 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);" Sure, there are some cases where we know the calculation won't overflow, but there are also cases where we can't be so sure, and if the parameters can be controlled by the guest then we absolutely can't allow the guest to cause QEMU to SIGFPE. Incidentally, in the examples you quote gcc is generally able (because the function is inline) to do a better job because of some of the constants involved: for instance with "muldiv64((int16_t) s->rtc.comp, 1000, 0x8000);" it generates a completely inline insn sequence with one mul and no div insns. So I think at this point I come back to an earlier question: do you have profiling results showing this function to be a performance problem on a real workload? If not, it just doesn't seem to me to be worth the risk and the maintenance burden to add a native x86 assembly version. -- PMM