Changeset 191514 in webkit

Timestamp:

Oct 23, 2015, 3:45:34 PM (10 years ago)

Author:

matthew_hanson@apple.com

Message:

Rollout 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>
+
+        Rollout 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-21  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);
-        if (Options::usePutStackSinking())
-            performPutStackSinking(dfg);
+        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>
+
+        Rollout 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-23  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
@@ -778 +774 @@
         runFTLNoCJITValidate
         runFTLNoCJITNoInlineValidate
-        runFTLNoCJITNoPutStackValidate
         runFTLEager
         runFTLEagerNoCJITValidate