Title: [199406] releases/WebKitGTK/webkit-2.12/Source/_javascript_Core
Revision
199406
Author
carlo...@webkit.org
Date
2016-04-12 23:34:46 -0700 (Tue, 12 Apr 2016)

Log Message

Merge r198296 - ASSERTION FAILED: !edge->isPhantomAllocation() in regress/script-tests/sink-huge-activation.js.ftl-eager in debug mode
https://bugs.webkit.org/show_bug.cgi?id=153805

Reviewed by Mark Lam.

The object allocation sinking phase uses InferredValue::isStillValid() in the opposite
way from most clients: it will do an *extra* optimization if it returns false. The
phase will first compute sink candidates and then it will compute materialization
points. If something is a sink candidate then it is not a materialization point. A
NewFunction node may appear as not being a sink candidate during the first pass, so it's
not added to the set of things that will turn into PhantomNewFunction. But on the second
pass where we add materializations, we check isStillValid() again. Now this may become
false, so that second pass thinks that NewFunction is a sink candidate (even though it's
not in the sink candidates set) and so is not a materialization point.

This manifests as the NewFunction referring to a PhantomCreateActivation or whatever.

The solution is to have the phase cache results of calls to isStillValid(). It's OK if
we just remember the result of the first call and assume that it's not a sink candidate.
That's the worst that can happen.

No new tests since this is a super hard race and sink-huge-activation seemed to already
be catching it.

* dfg/DFGObjectAllocationSinkingPhase.cpp:

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ChangeLog (199405 => 199406)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ChangeLog	2016-04-13 06:26:15 UTC (rev 199405)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/ChangeLog	2016-04-13 06:34:46 UTC (rev 199406)
@@ -1,3 +1,31 @@
+2016-03-15  Filip Pizlo  <fpi...@apple.com>
+
+        ASSERTION FAILED: !edge->isPhantomAllocation() in regress/script-tests/sink-huge-activation.js.ftl-eager in debug mode
+        https://bugs.webkit.org/show_bug.cgi?id=153805
+
+        Reviewed by Mark Lam.
+
+        The object allocation sinking phase uses InferredValue::isStillValid() in the opposite
+        way from most clients: it will do an *extra* optimization if it returns false. The
+        phase will first compute sink candidates and then it will compute materialization
+        points. If something is a sink candidate then it is not a materialization point. A
+        NewFunction node may appear as not being a sink candidate during the first pass, so it's
+        not added to the set of things that will turn into PhantomNewFunction. But on the second
+        pass where we add materializations, we check isStillValid() again. Now this may become
+        false, so that second pass thinks that NewFunction is a sink candidate (even though it's
+        not in the sink candidates set) and so is not a materialization point.
+
+        This manifests as the NewFunction referring to a PhantomCreateActivation or whatever.
+
+        The solution is to have the phase cache results of calls to isStillValid(). It's OK if
+        we just remember the result of the first call and assume that it's not a sink candidate.
+        That's the worst that can happen.
+
+        No new tests since this is a super hard race and sink-huge-activation seemed to already
+        be catching it.
+
+        * dfg/DFGObjectAllocationSinkingPhase.cpp:
+
 2016-03-14  Julien Brianceau  <jbria...@cisco.com>
 
         [mips] Fix unaligned access in LLINT.

Modified: releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp (199405 => 199406)


--- releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp	2016-04-13 06:26:15 UTC (rev 199405)
+++ releases/WebKitGTK/webkit-2.12/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp	2016-04-13 06:34:46 UTC (rev 199406)
@@ -838,7 +838,7 @@
         case NewFunction:
         case NewArrowFunction:
         case NewGeneratorFunction: {
-            if (node->castOperand<FunctionExecutable*>()->singletonFunction()->isStillValid()) {
+            if (isStillValid(node->castOperand<FunctionExecutable*>()->singletonFunction())) {
                 m_heap.escape(node->child1().node());
                 break;
             }
@@ -855,7 +855,7 @@
         }
 
         case CreateActivation: {
-            if (node->castOperand<SymbolTable*>()->singletonScope()->isStillValid()) {
+            if (isStillValid(node->castOperand<SymbolTable*>()->singletonScope())) {
                 m_heap.escape(node->child1().node());
                 break;
             }
@@ -2173,6 +2173,15 @@
         }
     }
 
+    // This is a great way of asking value->isStillValid() without having to worry about getting
+    // different answers. It turns out that this analysis works OK regardless of what this
+    // returns but breaks badly if this changes its mind for any particular InferredValue. This
+    // method protects us from that.
+    bool isStillValid(InferredValue* value)
+    {
+        return m_validInferredValues.add(value, value->isStillValid()).iterator->value;
+    }
+
     SSACalculator m_pointerSSA;
     SSACalculator m_allocationSSA;
     HashSet<Node*> m_sinkCandidates;
@@ -2183,6 +2192,8 @@
     InsertionSet m_insertionSet;
     CombinedLiveness m_combinedLiveness;
 
+    HashMap<InferredValue*, bool> m_validInferredValues;
+
     HashMap<Node*, Node*> m_materializationToEscapee;
     HashMap<Node*, Vector<Node*>> m_materializationSiteToMaterializations;
     HashMap<Node*, Vector<PromotedHeapLocation>> m_materializationSiteToRecoveries;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to