Title: [285788] trunk
- Revision
- 285788
- Author
- commit-qu...@webkit.org
- Date
- 2021-11-14 00:45:44 -0800 (Sun, 14 Nov 2021)
Log Message
Prevent fused multiply add during ParseInt
https://bugs.webkit.org/show_bug.cgi?id=232951
Patch by Mikhail R. Gadelha <mikh...@igalia.com> on 2021-11-14
Reviewed by Yusuke Suzuki.
When parsing the string in parseInt, gcc 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.
Alternative solutions that I tried but gcc seems to ignore: #pragma
STDC FP_CONTRACT OFF, compiling with -ffp-contract=off, and setting function
attributes __attribute__((optimize("fp-contract=off"))) and
__attribute__((optimize("-ffp-contract=off"))), so volative seems to be
a good compromise.
The issue was found when cross compiling to mips with gcc 8.4.0 and
options -ffp-contract=off -mmadd4.
JSTests:
* ChakraCore.yaml:
Source/_javascript_Core:
* runtime/ParseInt.h:
(JSC::parseInt):
Modified Paths
Diff
Modified: trunk/JSTests/ChakraCore.yaml (285787 => 285788)
--- trunk/JSTests/ChakraCore.yaml 2021-11-14 07:37:36 UTC (rev 285787)
+++ trunk/JSTests/ChakraCore.yaml 2021-11-14 08:45:44 UTC (rev 285788)
@@ -696,12 +696,7 @@
end
- path: ChakraCore/test/GlobalFunctions/ParseInt1.js
# Currently fails on the Loongson 3A4000 (in 32-bits mode).
- cmd: |
- if $architecture == "mips"
- skip
- else
- runChakra :baseline, "NoException", "ParseInt1.baseline", []
- end
+ cmd: runChakra :baseline, "NoException", "ParseInt1.baseline", []
- path: ChakraCore/test/GlobalFunctions/toString.js
cmd: runChakra :baseline, "NoException", "toString.baseline", []
- path: ChakraCore/test/InlineCaches/t0.js
Modified: trunk/JSTests/ChangeLog (285787 => 285788)
--- trunk/JSTests/ChangeLog 2021-11-14 07:37:36 UTC (rev 285787)
+++ trunk/JSTests/ChangeLog 2021-11-14 08:45:44 UTC (rev 285788)
@@ -1,3 +1,30 @@
+2021-11-14 Mikhail R. Gadelha <mikh...@igalia.com>
+
+ Prevent fused multiply add during ParseInt
+ https://bugs.webkit.org/show_bug.cgi?id=232951
+
+ Reviewed by Yusuke Suzuki.
+
+ When parsing the string in parseInt, gcc 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.
+
+ Alternative solutions that I tried but gcc seems to ignore: #pragma
+ STDC FP_CONTRACT OFF, compiling with -ffp-contract=off, and setting function
+ attributes __attribute__((optimize("fp-contract=off"))) and
+ __attribute__((optimize("-ffp-contract=off"))), so volative seems to be
+ a good compromise.
+
+ The issue was found when cross compiling to mips with gcc 8.4.0 and
+ options -ffp-contract=off -mmadd4.
+
+ * ChakraCore.yaml:
+
2021-11-12 Joseph Griego <jgri...@igalia.com>
[JSC] update test262
Modified: trunk/Source/_javascript_Core/ChangeLog (285787 => 285788)
--- trunk/Source/_javascript_Core/ChangeLog 2021-11-14 07:37:36 UTC (rev 285787)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-11-14 08:45:44 UTC (rev 285788)
@@ -1,3 +1,31 @@
+2021-11-14 Mikhail R. Gadelha <mikh...@igalia.com>
+
+ Prevent fused multiply add during ParseInt
+ https://bugs.webkit.org/show_bug.cgi?id=232951
+
+ Reviewed by Yusuke Suzuki.
+
+ When parsing the string in parseInt, gcc 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.
+
+ Alternative solutions that I tried but gcc seems to ignore: #pragma
+ STDC FP_CONTRACT OFF, compiling with -ffp-contract=off, and setting function
+ attributes __attribute__((optimize("fp-contract=off"))) and
+ __attribute__((optimize("-ffp-contract=off"))), so volative seems to be
+ a good compromise.
+
+ The issue was found when cross compiling to mips with gcc 8.4.0 and
+ options -ffp-contract=off -mmadd4.
+
+ * runtime/ParseInt.h:
+ (JSC::parseInt):
+
2021-11-12 Darin Adler <da...@apple.com>
Make sort-Xcode-project-file idempotent
Modified: trunk/Source/_javascript_Core/runtime/ParseInt.h (285787 => 285788)
--- trunk/Source/_javascript_Core/runtime/ParseInt.h 2021-11-14 07:37:36 UTC (rev 285787)
+++ trunk/Source/_javascript_Core/runtime/ParseInt.h 2021-11-14 08:45:44 UTC (rev 285788)
@@ -162,7 +162,15 @@
// 14. Let number be the Number value for mathInt.
int firstDigitPosition = p;
bool sawDigit = false;
+#if COMPILER(GCC)
+ // Due to a bug found in GCC v8.4.0, a wrong fused multiply-add optimization can be inserted when calculating the final number,
+ // in number *= radix; number += digit;, so add volatile to prevent optimizations.
+ // GCC v8.4.0 also seems to ignore #pragma STDC FP_CONTRACT OFF, compiling with -ffp-contract=off, and setting function attributes
+ // __attribute__((optimize("fp-contract=off"))) and __attribute__((optimize("-ffp-contract=off"))).
+ volatile double number = 0;
+#else
double number = 0;
+#endif
while (p < length) {
int digit = parseDigit(data[p], radix);
if (digit == -1)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
