Title: [227236] trunk
- Revision
- 227236
- Author
- [email protected]
- Date
- 2018-01-19 14:30:45 -0800 (Fri, 19 Jan 2018)
Log Message
Kill ArithNegate's ArithProfile assert inside BytecodeParser
https://bugs.webkit.org/show_bug.cgi?id=181877
<rdar://problem/36630552>
Reviewed by Mark Lam.
JSTests:
* stress/arith-profile-for-negate-can-see-non-number-due-to-dfg-osr-exit-profiling.js: Added.
(runNearStackLimit):
(f1):
(f2):
(f3):
(i.catch):
(i.try.runNearStackLimit):
(catch):
Source/_javascript_Core:
Before this patch, we used to assert that op_negate's result ArithProfile
only produces number. It's logically true that negate only produces a number.
However, the DFG may incorrectly pick this ArithProfile when doing OSR exit
profiling. So we'll end up profiling something that's likely the input to
negate. This patch removes the assert. We cede to the fact that Graph::methodOfGettingAValueProfileFor
is entirely heuristic based, potentially leading to profiling results being imprecise.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::makeSafe):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (227235 => 227236)
--- trunk/JSTests/ChangeLog 2018-01-19 22:16:21 UTC (rev 227235)
+++ trunk/JSTests/ChangeLog 2018-01-19 22:30:45 UTC (rev 227236)
@@ -1,5 +1,22 @@
2018-01-19 Saam Barati <[email protected]>
+ Kill ArithNegate's ArithProfile assert inside BytecodeParser
+ https://bugs.webkit.org/show_bug.cgi?id=181877
+ <rdar://problem/36630552>
+
+ Reviewed by Mark Lam.
+
+ * stress/arith-profile-for-negate-can-see-non-number-due-to-dfg-osr-exit-profiling.js: Added.
+ (runNearStackLimit):
+ (f1):
+ (f2):
+ (f3):
+ (i.catch):
+ (i.try.runNearStackLimit):
+ (catch):
+
+2018-01-19 Saam Barati <[email protected]>
+
Spread's effects are modeled incorrectly both in AI and in Clobberize
https://bugs.webkit.org/show_bug.cgi?id=181867
<rdar://problem/36290415>
Added: trunk/JSTests/stress/arith-profile-for-negate-can-see-non-number-due-to-dfg-osr-exit-profiling.js (0 => 227236)
--- trunk/JSTests/stress/arith-profile-for-negate-can-see-non-number-due-to-dfg-osr-exit-profiling.js (rev 0)
+++ trunk/JSTests/stress/arith-profile-for-negate-can-see-non-number-due-to-dfg-osr-exit-profiling.js 2018-01-19 22:30:45 UTC (rev 227236)
@@ -0,0 +1,32 @@
+function runNearStackLimit(f) {
+ try {
+ return t();
+ } catch (e) {
+ return f();
+ }
+}
+let flag = false;
+function f1() {
+ return flag ? {} : 10;
+}
+noInline(f1);
+
+function f2() {
+}
+
+function f3(arg) {
+ let r = -(arg ? f1() : f2());
+}
+
+for (let i = 0; i < 100000; ++i) {
+ try {
+ f3(!!(i % 2));
+ } catch (e) {}
+}
+
+flag = true;
+for (let i = 0; i < 100000; ++i) try {
+ runNearStackLimit(() => {
+ return f3(!!(i % 2));
+ });
+} catch (e) {}
Modified: trunk/Source/_javascript_Core/ChangeLog (227235 => 227236)
--- trunk/Source/_javascript_Core/ChangeLog 2018-01-19 22:16:21 UTC (rev 227235)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-01-19 22:30:45 UTC (rev 227236)
@@ -1,3 +1,21 @@
+2018-01-19 Saam Barati <[email protected]>
+
+ Kill ArithNegate's ArithProfile assert inside BytecodeParser
+ https://bugs.webkit.org/show_bug.cgi?id=181877
+ <rdar://problem/36630552>
+
+ Reviewed by Mark Lam.
+
+ Before this patch, we used to assert that op_negate's result ArithProfile
+ only produces number. It's logically true that negate only produces a number.
+ However, the DFG may incorrectly pick this ArithProfile when doing OSR exit
+ profiling. So we'll end up profiling something that's likely the input to
+ negate. This patch removes the assert. We cede to the fact that Graph::methodOfGettingAValueProfileFor
+ is entirely heuristic based, potentially leading to profiling results being imprecise.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::makeSafe):
+
2018-01-19 David Kilzer <[email protected]>
oss-fuzz jsc build is broken: StringImpl.h:27:10: fatal error: 'unicode/ustring.h' file not found
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (227235 => 227236)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-19 22:16:21 UTC (rev 227235)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-19 22:30:45 UTC (rev 227236)
@@ -940,8 +940,10 @@
break;
}
case ArithNegate: {
- ASSERT_WITH_MESSAGE(!arithProfile->didObserveNonNumber(), "op_negate starts with a toNumber() on the argument, it should only produce numbers.");
-
+ // We'd like to assert here that the arith profile for the result of negate never
+ // sees a non-number, but we can't. It's true that negate never produces a non-number.
+ // But sometimes we'll end up grabbing the wrong ArithProfile during OSR exit, and
+ // profiling the wrong value, leading the ArithProfile to think it observed a non-number result.
if (arithProfile->lhsObservedType().sawNumber() || arithProfile->didObserveDouble())
node->mergeFlags(NodeMayHaveDoubleResult);
if (arithProfile->didObserveNegZeroDouble() || m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, NegativeZero))
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
