[webkit-reviews] review denied: [Bug 232951] Prevent fused multiply add during ParseInt : [Attachment 443833] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has denied Mikhail R. Gadelha
<mikhail at igalia.com>'s request for review:
Bug 232951: Prevent fused multiply add during ParseInt
https://bugs.webkit.org/show_bug.cgi?id=232951

Attachment 443833: Patch

https://bugs.webkit.org/attachment.cgi?id=443833&action=review




--- Comment #3 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 443833
  --> https://bugs.webkit.org/attachment.cgi?id=443833
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=443833&action=review

Commented.

> Source/JavaScriptCore/ChangeLog:18
> +	   When parsing the string in parseInt, a compiler can wrongfully
generate
> +	   a fused multiply-add instruction, causing the conversion to be wrong
> +	   for some high values. An add followed by a multiply gives the
correct
> +	   result and it is the code generated most of the times.
> +
> +	   This patch adds a volatile qualifier to the number variable, so the
> +	   compiler doesn't try to optimize it, and enables a failing test on
> +	   mips.
> +
> +	   The issue was found when cross compiling to mips with gcc 8.4.0 and
> +	   options -ffp-contract=off -mmadd4.

This is GCC issue, and we should not use `volatile` here. We should disable
GCC's this optimization for this function via pragma etc.
https://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/Function-Specific-Option-Pragmas.h
tml#Function-Specific-Option-Pragmas
https://stackoverflow.com/questions/34436233/fused-multiply-add-and-default-rou
nding-modes