Fix assertion in KnownCellUse inside SpeculativeJIT::speculate
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Nov 2018 03:39:54 +0000 (03:39 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Nov 2018 03:39:54 +0000 (03:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191895
<rdar://problem/46167406>

Reviewed by Mark Lam.

JSTests:

* stress/known-cell-use-needs-type-check-assertion.js: Added.
(foo):
(bar):

Source/JavaScriptCore:

We were asserting that the input edge should have type SpecCell but it should
really be SpecCellCheck since the type filter for KnownCellUse is SpecCellCheck.

This patch cleans up that assertion code by joining a bunch of cases into a
single function call which grabs the type filter for the edge UseKind and
asserts that the incoming edge meets the type filter criteria.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::speculate):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::speculate):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238436 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/known-cell-use-needs-type-check-assertion.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

index a4a0a16..2479b00 100644 (file)
@@ -1,3 +1,15 @@
+2018-11-21  Saam barati  <sbarati@apple.com>
+
+        Fix assertion in KnownCellUse inside SpeculativeJIT::speculate
+        https://bugs.webkit.org/show_bug.cgi?id=191895
+        <rdar://problem/46167406>
+
+        Reviewed by Mark Lam.
+
+        * stress/known-cell-use-needs-type-check-assertion.js: Added.
+        (foo):
+        (bar):
+
 2018-11-21  Mark Lam  <mark.lam@apple.com>
 
         Creating a wasm memory that is bigger than the ArrayBuffer limit but smaller than the spec limit should throw OOME not RangeError.
diff --git a/JSTests/stress/known-cell-use-needs-type-check-assertion.js b/JSTests/stress/known-cell-use-needs-type-check-assertion.js
new file mode 100644 (file)
index 0000000..652cf89
--- /dev/null
@@ -0,0 +1,14 @@
+//@ runDefault("--useTypeProfiler=1")
+
+function foo(z) {
+    bar(z);
+}
+function bar(o) {
+    o.x = 0;
+}
+let p = 0;
+let k = {};
+for (var i = 0; i < 100000; ++i) {
+    bar(p);
+    foo(k);
+}
index f7d9899..75b693c 100644 (file)
@@ -1,3 +1,23 @@
+2018-11-21  Saam barati  <sbarati@apple.com>
+
+        Fix assertion in KnownCellUse inside SpeculativeJIT::speculate
+        https://bugs.webkit.org/show_bug.cgi?id=191895
+        <rdar://problem/46167406>
+
+        Reviewed by Mark Lam.
+
+        We were asserting that the input edge should have type SpecCell but it should
+        really be SpecCellCheck since the type filter for KnownCellUse is SpecCellCheck.
+        
+        This patch cleans up that assertion code by joining a bunch of cases into a
+        single function call which grabs the type filter for the edge UseKind and
+        asserts that the incoming edge meets the type filter criteria.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::speculate):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::speculate):
+
 2018-11-21  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
         [JSC] Use ProtoCallFrame::numberOfRegisters instead of raw number `4`
index fb4c5d7..ccf0166 100644 (file)
@@ -10286,23 +10286,15 @@ void SpeculativeJIT::speculate(Node*, Edge edge)
     switch (edge.useKind()) {
     case UntypedUse:
         break;
-    case KnownInt32Use:
-        ASSERT(!needsTypeCheck(edge, SpecInt32Only));
-        break;
     case DoubleRepUse:
-        ASSERT(!needsTypeCheck(edge, SpecFullDouble));
-        break;
     case Int52RepUse:
-        ASSERT(!needsTypeCheck(edge, SpecAnyInt));
-        break;
+    case KnownInt32Use:
     case KnownCellUse:
-        ASSERT(!needsTypeCheck(edge, SpecCell));
-        break;
     case KnownStringUse:
-        ASSERT(!needsTypeCheck(edge, SpecString));
-        break;
     case KnownPrimitiveUse:
-        ASSERT(!needsTypeCheck(edge, SpecHeapTop & ~SpecObject));
+    case KnownOtherUse:
+    case KnownBooleanUse:
+        ASSERT(!m_interpreter.needsTypeCheck(edge));
         break;
     case Int32Use:
         speculateInt32(edge);
@@ -10327,9 +10319,6 @@ void SpeculativeJIT::speculate(Node*, Edge edge)
     case BooleanUse:
         speculateBoolean(edge);
         break;
-    case KnownBooleanUse:
-        ASSERT(!needsTypeCheck(edge, SpecBoolean));
-        break;
     case CellUse:
         speculateCell(edge);
         break;
@@ -10405,9 +10394,6 @@ void SpeculativeJIT::speculate(Node*, Edge edge)
     case NotCellUse:
         speculateNotCell(edge);
         break;
-    case KnownOtherUse:
-        ASSERT(!needsTypeCheck(edge, SpecOther));
-        break;
     case OtherUse:
         speculateOther(edge);
         break;
index a538990..027a2dc 100644 (file)
@@ -15339,6 +15339,8 @@ private:
         case KnownOtherUse:
         case DoubleRepUse:
         case Int52RepUse:
+        case KnownCellUse:
+        case KnownBooleanUse:
             ASSERT(!m_interpreter.needsTypeCheck(edge));
             break;
         case Int32Use:
@@ -15350,9 +15352,6 @@ private:
         case CellOrOtherUse:
             speculateCellOrOther(edge);
             break;
-        case KnownCellUse:
-            ASSERT(!m_interpreter.needsTypeCheck(edge));
-            break;
         case AnyIntUse:
             speculateAnyInt(edge);
             break;