- Revision
- 191395
- Author
- fpi...@apple.com
- Date
- 2015-10-21 11:45:48 -0700 (Wed, 21 Oct 2015)
Log Message
Failures in PutStackSinkingPhase should be less severe
https://bugs.webkit.org/show_bug.cgi?id=150400
Reviewed by Geoffrey Garen.
Source/_javascript_Core:
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.
Tools:
Add a test mode for no PutStackSinkingPhase.
* Scripts/run-jsc-stress-tests:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (191394 => 191395)
--- trunk/Source/_javascript_Core/ChangeLog 2015-10-21 18:44:44 UTC (rev 191394)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-10-21 18:45:48 UTC (rev 191395)
@@ -1,3 +1,18 @@
+2015-10-21 Filip Pizlo <fpi...@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 Saam barati <sbar...@apple.com>
The FTL should place the CallSiteIndex on the call frame for JS calls when it fills in the patchpoint
Modified: trunk/Source/_javascript_Core/dfg/DFGPlan.cpp (191394 => 191395)
--- trunk/Source/_javascript_Core/dfg/DFGPlan.cpp 2015-10-21 18:44:44 UTC (rev 191394)
+++ trunk/Source/_javascript_Core/dfg/DFGPlan.cpp 2015-10-21 18:45:48 UTC (rev 191395)
@@ -394,7 +394,8 @@
// Ideally, these would be run to fixpoint with the object allocation sinking phase.
performArgumentsElimination(dfg);
- performPutStackSinking(dfg);
+ if (Options::usePutStackSinking())
+ performPutStackSinking(dfg);
performConstantHoisting(dfg);
performGlobalCSE(dfg);
Modified: trunk/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp (191394 => 191395)
--- trunk/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp 2015-10-21 18:44:44 UTC (rev 191394)
+++ trunk/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp 2015-10-21 18:45:48 UTC (rev 191395)
@@ -233,10 +233,39 @@
dataLog("Deferred at ", node, ":", deferred, "\n");
if (node->op() == GetStack) {
- DFG_ASSERT(
- m_graph, node,
- deferred.operand(node->stackAccessData()->local) != ConflictingFlush);
+ // 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.
continue;
Modified: trunk/Source/_javascript_Core/runtime/Options.h (191394 => 191395)
--- trunk/Source/_javascript_Core/runtime/Options.h 2015-10-21 18:44:44 UTC (rev 191394)
+++ trunk/Source/_javascript_Core/runtime/Options.h 2015-10-21 18:45:48 UTC (rev 191395)
@@ -202,6 +202,7 @@
v(double, minimumCallToKnownRate, 0.51, nullptr) \
v(bool, createPreHeaders, true, nullptr) \
v(bool, useMovHintRemoval, true, nullptr) \
+ v(bool, usePutStackSinking, true, nullptr) \
v(bool, useObjectAllocationSinking, true, nullptr) \
v(bool, useCopyBarrierOptimization, true, nullptr) \
\
Modified: trunk/Tools/ChangeLog (191394 => 191395)
--- trunk/Tools/ChangeLog 2015-10-21 18:44:44 UTC (rev 191394)
+++ trunk/Tools/ChangeLog 2015-10-21 18:45:48 UTC (rev 191395)
@@ -1,3 +1,14 @@
+2015-10-21 Filip Pizlo <fpi...@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-21 Nan Wang <n_w...@apple.com>
AX: Expose table size and cell indexes on iOS
Modified: trunk/Tools/Scripts/run-jsc-stress-tests (191394 => 191395)
--- trunk/Tools/Scripts/run-jsc-stress-tests 2015-10-21 18:44:44 UTC (rev 191394)
+++ trunk/Tools/Scripts/run-jsc-stress-tests 2015-10-21 18:45:48 UTC (rev 191395)
@@ -746,6 +746,10 @@
run("ftl-no-cjit-validate", "--validateGraph=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS)) if $enableFTL
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
end
@@ -823,6 +827,7 @@
runDFGEagerNoCJITValidate
runDefaultFTL
runFTLNoCJITValidate
+ runFTLNoCJITNoPutStackValidate
runFTLNoCJITNoInlineValidate
runFTLEager
runFTLEagerNoCJITValidate
