Title: [166281] trunk/Source/_javascript_Core
- Revision
- 166281
- Author
- fpi...@apple.com
- Date
- 2014-03-25 21:08:52 -0700 (Tue, 25 Mar 2014)
Log Message
Arguments simplification phase should be fine with marking the arguments local itself as an arguments alias
https://bugs.webkit.org/show_bug.cgi?id=130764
<rdar://problem/16304788>
Reviewed by Sam Weinig.
Being an arguments alias just means that your OSR exit recovery should attempt arguments
creation. This is true of arguments locals. We had special cases that tried to make it not
true of arguments locals. The only consequence of those special cases was to cause crashes
in case of arguments that are also captured variables (i.e. we have SlowArguments). This
change just removes those special cases.
This change means that the FTL will now see SetLocals with a FlushedArguments format.
Previously you wouldn't see them because previously only non-captured variable would be
arguments aliases, and non-captured variables get completely SSAified - i.e. no SetLocals
left. Adding handling for FlushedArguments is a benign and simple change since its
behavior is identical to FlushedJSValue for that code's purposes.
* dfg/DFGArgumentsSimplificationPhase.cpp:
(JSC::DFG::ArgumentsSimplificationPhase::run):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileSetLocal):
* tests/stress/captured-arguments-variable.js: Added.
(foo):
(noInline):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (166280 => 166281)
--- trunk/Source/_javascript_Core/ChangeLog 2014-03-26 03:35:30 UTC (rev 166280)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-03-26 04:08:52 UTC (rev 166281)
@@ -1,3 +1,31 @@
+2014-03-25 Filip Pizlo <fpi...@apple.com>
+
+ Arguments simplification phase should be fine with marking the arguments local itself as an arguments alias
+ https://bugs.webkit.org/show_bug.cgi?id=130764
+ <rdar://problem/16304788>
+
+ Reviewed by Sam Weinig.
+
+ Being an arguments alias just means that your OSR exit recovery should attempt arguments
+ creation. This is true of arguments locals. We had special cases that tried to make it not
+ true of arguments locals. The only consequence of those special cases was to cause crashes
+ in case of arguments that are also captured variables (i.e. we have SlowArguments). This
+ change just removes those special cases.
+
+ This change means that the FTL will now see SetLocals with a FlushedArguments format.
+ Previously you wouldn't see them because previously only non-captured variable would be
+ arguments aliases, and non-captured variables get completely SSAified - i.e. no SetLocals
+ left. Adding handling for FlushedArguments is a benign and simple change since its
+ behavior is identical to FlushedJSValue for that code's purposes.
+
+ * dfg/DFGArgumentsSimplificationPhase.cpp:
+ (JSC::DFG::ArgumentsSimplificationPhase::run):
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::LowerDFGToLLVM::compileSetLocal):
+ * tests/stress/captured-arguments-variable.js: Added.
+ (foo):
+ (noInline):
+
2014-03-25 Mark Hahnenberg <mhahnenb...@apple.com>
Add HeapInlines
Modified: trunk/Source/_javascript_Core/dfg/DFGArgumentsSimplificationPhase.cpp (166280 => 166281)
--- trunk/Source/_javascript_Core/dfg/DFGArgumentsSimplificationPhase.cpp 2014-03-26 03:35:30 UTC (rev 166280)
+++ trunk/Source/_javascript_Core/dfg/DFGArgumentsSimplificationPhase.cpp 2014-03-26 04:08:52 UTC (rev 166281)
@@ -397,10 +397,6 @@
VariableAccessData* variableAccessData = node->variableAccessData();
- if (m_graph.argumentsRegisterFor(node->origin.semantic) == variableAccessData->local()
- || unmodifiedArgumentsRegister(m_graph.argumentsRegisterFor(node->origin.semantic)) == variableAccessData->local())
- break;
-
if (variableAccessData->mergeIsArgumentsAlias(true)) {
changed = true;
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (166280 => 166281)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2014-03-26 03:35:30 UTC (rev 166280)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2014-03-26 04:08:52 UTC (rev 166281)
@@ -806,7 +806,8 @@
{
VariableAccessData* variable = m_node->variableAccessData();
switch (variable->flushFormat()) {
- case FlushedJSValue: {
+ case FlushedJSValue:
+ case FlushedArguments: {
LValue value = lowJSValue(m_node->child1());
m_out.store64(value, addressFor(variable->machineLocal()));
break;
Added: trunk/Source/_javascript_Core/tests/stress/captured-arguments-variable.js (0 => 166281)
--- trunk/Source/_javascript_Core/tests/stress/captured-arguments-variable.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/captured-arguments-variable.js 2014-03-26 04:08:52 UTC (rev 166281)
@@ -0,0 +1,17 @@
+function foo(a) {
+ return arguments[1] + (function() { return a * 101; })();
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var result = foo(42, 97);
+ if (result != 4339)
+ throw "Error: bad result: " + result;
+}
+
+Object.prototype[1] = 111;
+
+var result = foo(42);
+if (result != 4353)
+ throw "Error: bad result at end: " + result;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
