Changeset 191463 in webkit

Timestamp:

Oct 22, 2015 11:36:28 AM (9 years ago)

Author:

matthew_hanson@apple.com

Message:

Merge r191395. rdar://problem/22846455

Location:

branches/safari-601.1.46-branch

Files:

• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGPlan.cpp (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGPutStackSinkingPhase.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/Options.h (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/Scripts/run-jsc-stress-tests (modified) (2 diffs)

branches/safari-601.1.46-branch/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-10-21  Matthew Hanson  <matthew_hanson@apple.com>
+
+        Merge r191395. rdar://problem/22846455
+
+    2015-10-21  Filip Pizlo  <fpizlo@apple.com>
+
+            Failures in PutStackSinkingPhase should be less severe
+            https://bugs.webkit.org/show_bug.cgi?id=150400
+
+            Reviewed by Geoffrey Garen.
+
+            Make the PutStackSinkingPhase abort instead of asserting. To test that it's OK to not have
+            PutStackSinkingPhase run, this adds a test mode where we run without PutStackSinkingPhase.
+
+            * dfg/DFGPlan.cpp: Make it possible to not run PutStackSinkingPhase for tests.
+            (JSC::DFG::Plan::compileInThreadImpl):
+            * dfg/DFGPutStackSinkingPhase.cpp: PutStackSinkingPhase should abort instead of asserting, except when validation is enabled.
+            * runtime/Options.h: Add an option for disabling PutStackSinkingPhase.
+
 2015-10-20  Matthew Hanson  <matthew_hanson@apple.com>
 

branches/safari-601.1.46-branch/Source/JavaScriptCore/dfg/DFGPlan.cpp

@@ -355 +355 @@
         // Ideally, these would be run to fixpoint with the object allocation sinking phase.
         performArgumentsElimination(dfg);
-        performPutStackSinking(dfg);
+        if (Options::usePutStackSinking())
+            performPutStackSinking(dfg);
         
         performConstantHoisting(dfg);

branches/safari-601.1.46-branch/Source/JavaScriptCore/dfg/DFGPutStackSinkingPhase.cpp

@@ -231 +231 @@
                     
                     if (node->op() == GetStack) {
+                        // Handle the case that the input doesn't match our requirements. This is
+                        // really a bug, but it's a benign one if we simply don't run this phase.
+                        // It usually arises because of patterns like:
+                        //
+                        // if (thing)
+                        //     PutStack()
+                        // ...
+                        // if (thing)
+                        //     GetStack()
+                        //
+                        // Or:
+                        //
+                        // if (never happens)
+                        //     GetStack()
+                        //
+                        // Because this phase runs early in SSA, it should be sensible to enforce
+                        // that no such code pattern has arisen yet. So, when validation is
+                        // enabled, we assert that we aren't seeing this. But with validation
+                        // disabled we silently let this fly and we just abort this phase.
+                        // FIXME: Get rid of all remaining cases of conflicting GetStacks.
+                        // https://bugs.webkit.org/show_bug.cgi?id=150398
+
+                        bool isConflicting =
+                            deferred.operand(node->stackAccessData()->local) == ConflictingFlush;
+                        
+                        if (validationEnabled())
+                            DFG_ASSERT(m_graph, node, !isConflicting);
+
+                        if (isConflicting) {
+                            // Oh noes! Abort!!
+                            return false;
+                        }
+
                         // A GetStack doesn't affect anything, since we know which local we are reading
                         // from.

branches/safari-601.1.46-branch/Source/JavaScriptCore/runtime/Options.h

@@ -191 +191 @@
     v(bool, enableMovHintRemoval, true, nullptr) \
     v(bool, enableObjectAllocationSinking, true, nullptr) \
+    v(bool, usePutStackSinking, true, nullptr) \
     \
     v(bool, enableConcurrentJIT, true, "allows the DFG / FTL compilation in threads other than the executing JS thread") \

branches/safari-601.1.46-branch/Tools/ChangeLog

@@ +1 @@
+2015-10-21  Matthew Hanson  <matthew_hanson@apple.com>
+
+        Merge r191395. rdar://problem/22846455
+
+    2015-10-21  Filip Pizlo  <fpizlo@apple.com>
+
+            Failures in PutStackSinkingPhase should be less severe
+            https://bugs.webkit.org/show_bug.cgi?id=150400
+
+            Reviewed by Geoffrey Garen.
+
+            Add a test mode for no PutStackSinkingPhase.
+
+            * Scripts/run-jsc-stress-tests:
+
 2015-10-20  Matthew Hanson  <matthew_hanson@apple.com>
 

branches/safari-601.1.46-branch/Tools/Scripts/run-jsc-stress-tests

@@ -711 +711 @@
 end
 
+def runFTLNoCJITNoPutStackValidate
+    run("ftl-no-cjit-no-put-stack-validate", "--validateGraph=true", "--usePutStackSinking=false", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
+end
+
 def runFTLNoCJITNoInlineValidate
     run("ftl-no-cjit-no-inline-validate", "--validateGraph=true", "--maximumInliningDepth=1", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
@@ -774 +778 @@
         runFTLNoCJITValidate
         runFTLNoCJITNoInlineValidate
+        runFTLNoCJITNoPutStackValidate
         runFTLEager
         runFTLEagerNoCJITValidate