Remove an invalid assertion in DFG::SpeculativeJIT::nonSpeculativeNonPeepholeCompareN...
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 23:34:31 +0000 (23:34 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 23:34:31 +0000 (23:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196116
<rdar://problem/48976951>

Reviewed by Filip Pizlo.

JSTests:

* stress/dfg-compare-eq-via-nonSpeculativeNonPeepholeCompareNullOrUndefined.js: Added.

Source/JavaScriptCore:

The DFG backend should not make assumptions about what optimizations the front end
will or will not do.  The assertion asserts that the operand cannot be known to be
a cell.  However, it is not guaranteed that the front end will fold away this case.
Also, the DFG backend is perfectly capable of generating code to handle the case
where the operand is a cell.

The attached test case demonstrates a case where the operand can be a known cell.
The test needs to be run with the concurrent JIT and GC, and is racy.  It used to
trip up this assertion about once every 10 runs or so.

* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::nonSpeculativeNonPeepholeCompareNullOrUndefined):

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

JSTests/ChangeLog
JSTests/stress/dfg-compare-eq-via-nonSpeculativeNonPeepholeCompareNullOrUndefined.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

index af69f22..56f39d4 100644 (file)
@@ -1,3 +1,13 @@
+2019-03-21  Mark Lam  <mark.lam@apple.com>
+
+        Remove an invalid assertion in DFG::SpeculativeJIT::nonSpeculativeNonPeepholeCompareNullOrUndefined().
+        https://bugs.webkit.org/show_bug.cgi?id=196116
+        <rdar://problem/48976951>
+
+        Reviewed by Filip Pizlo.
+
+        * stress/dfg-compare-eq-via-nonSpeculativeNonPeepholeCompareNullOrUndefined.js: Added.
+
 2019-03-21  Tadeu Zagallo  <tzagallo@apple.com>
 
         JSObject::putDirectIndexSlowOrBeyondVectorLength should check if indexIsSufficientlyBeyondLengthForSparseMap
diff --git a/JSTests/stress/dfg-compare-eq-via-nonSpeculativeNonPeepholeCompareNullOrUndefined.js b/JSTests/stress/dfg-compare-eq-via-nonSpeculativeNonPeepholeCompareNullOrUndefined.js
new file mode 100644 (file)
index 0000000..61ffa55
--- /dev/null
@@ -0,0 +1,7 @@
+//@ runDefault("--collectContinuously=true", "--collectContinuouslyPeriodMS=0.15", "--useMaximalFlushInsertionPhase=true", "--useLLInt=false", "--useFTLJIT=false", "--jitPolicyScale=0")
+
+// This test exercises DFG::SpeculativeJIT::nonSpeculativeNonPeepholeCompareNullOrUndefined().
+
+for (let i = 0; i < 25; i++)
+    'a'.match(/a/);
+
index d940ae3..ed85169 100644 (file)
@@ -1,3 +1,24 @@
+2019-03-21  Mark Lam  <mark.lam@apple.com>
+
+        Remove an invalid assertion in DFG::SpeculativeJIT::nonSpeculativeNonPeepholeCompareNullOrUndefined().
+        https://bugs.webkit.org/show_bug.cgi?id=196116
+        <rdar://problem/48976951>
+
+        Reviewed by Filip Pizlo.
+
+        The DFG backend should not make assumptions about what optimizations the front end
+        will or will not do.  The assertion asserts that the operand cannot be known to be
+        a cell.  However, it is not guaranteed that the front end will fold away this case.
+        Also, the DFG backend is perfectly capable of generating code to handle the case
+        where the operand is a cell.
+
+        The attached test case demonstrates a case where the operand can be a known cell.
+        The test needs to be run with the concurrent JIT and GC, and is racy.  It used to
+        trip up this assertion about once every 10 runs or so.
+
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::nonSpeculativeNonPeepholeCompareNullOrUndefined):
+
 2019-03-21  Tadeu Zagallo  <tzagallo@apple.com>
 
         JSC::createError should clear exception thrown by errorDescriptionForValue
index 892cd3f..3afcc74 100644 (file)
@@ -217,8 +217,6 @@ void SpeculativeJIT::cachedGetByIdWithThis(CodeOrigin codeOrigin, GPRReg baseGPR
 
 void SpeculativeJIT::nonSpeculativeNonPeepholeCompareNullOrUndefined(Edge operand)
 {
-    ASSERT_WITH_MESSAGE(!masqueradesAsUndefinedWatchpointIsStillValid() || !isKnownCell(operand.node()), "The Compare should have been eliminated, it is known to be always false.");
-
     JSValueOperand arg(this, operand, ManualOperandSpeculation);
     GPRReg argGPR = arg.gpr();