We should be able to eliminate cloned arguments objects that use the length property
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Mar 2016 20:55:15 +0000 (20:55 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Mar 2016 20:55:15 +0000 (20:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=155391

Reviewed by Geoffrey Garen.

Previously if a programmer tried to use arguments.length in a strict function we would not eliminate the
arguments object. We were unable to eliminate the arguments object because the user would get a cloned arguments
object, which does not special case the length property. Thus, in order to get arguments elimination for cloned
we need to add a special case. There are two things that need to happen for the elimination to succeed.

First, we need to eliminate the CheckStructure blocking the GetByOffset for the length property. In order to
eliminate the check structure we need to prove to the Abstract Interpreter that this structure check is
unnesssary. This didn't occur before for two reasons: 1) CreateClonedArguments did not set the structure it
produced. 2) Even if CreateClonedArguments provided the global object's cloned arguments structure we would
transition the new argements object when we added the length property during construction. To fix the second
problem we now pre-assign a slot on clonedArgumentsStructure for the length property. Additionally, in order to
prevent future transitions of the structure we need to choose an indexing type for the structure. Since, not
eliminating the arguments object is so expensive we choose to have all cloned arguments start with continuous
indexing type, this avoids transitioning when otherwise we would not have to. In the future we should be smarter
about choosing the indexing type but since its relatively rare to have a arguments object escape we don't worry
about this for now.

Additionally, this patch renames all former references of outOfBandArguments to clonedArguments and adds
extra instrumentation to DFGArgumentsEliminationPhase.

* bytecode/BytecodeList.json:
* bytecode/BytecodeUseDef.h:
(JSC::computeUsesForBytecodeOffset):
(JSC::computeDefsForBytecodeOffset):
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::dumpBytecode):
* bytecode/ValueRecovery.h:
(JSC::ValueRecovery::clonedArgumentsThatWereNotCreated):
(JSC::ValueRecovery::outOfBandArgumentsThatWereNotCreated): Deleted.
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGArgumentsEliminationPhase.cpp:
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGCapabilities.cpp:
(JSC::DFG::capabilityLevel):
* dfg/DFGOperations.cpp:
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileCreateClonedArguments):
* dfg/DFGStructureRegistrationPhase.cpp:
(JSC::DFG::StructureRegistrationPhase::run):
* dfg/DFGVariableEventStream.cpp:
(JSC::DFG::VariableEventStream::tryToSetConstantRecovery):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileCreateClonedArguments):
* ftl/FTLOperations.cpp:
(JSC::FTL::operationMaterializeObjectInOSR):
* jit/JIT.cpp:
(JSC::JIT::privateCompileMainPass):
* jit/JIT.h:
* jit/JITOpcodes.cpp:
(JSC::JIT::emit_op_create_cloned_arguments):
(JSC::JIT::emit_op_create_out_of_band_arguments): Deleted.
* llint/LowLevelInterpreter.asm:
* runtime/ClonedArguments.cpp:
(JSC::ClonedArguments::ClonedArguments):
(JSC::ClonedArguments::createEmpty):
(JSC::ClonedArguments::createWithInlineFrame):
(JSC::ClonedArguments::createByCopyingFrom):
(JSC::ClonedArguments::createStructure):
* runtime/ClonedArguments.h:
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::init):
(JSC::JSGlobalObject::visitChildren):
* runtime/JSGlobalObject.h:
(JSC::JSGlobalObject::clonedArgumentsStructure):
(JSC::JSGlobalObject::outOfBandArgumentsStructure): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@198154 268f45cc-cd09-0410-ab3c-d52691b4dbfc

27 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/BytecodeList.json
Source/JavaScriptCore/bytecode/BytecodeUseDef.h
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/bytecode/ValueRecovery.h
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGCapabilities.cpp
Source/JavaScriptCore/dfg/DFGOperations.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGStructureRegistrationPhase.cpp
Source/JavaScriptCore/dfg/DFGVariableEventStream.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/ftl/FTLOperations.cpp
Source/JavaScriptCore/jit/JIT.cpp
Source/JavaScriptCore/jit/JIT.h
Source/JavaScriptCore/jit/JITOpcodes.cpp
Source/JavaScriptCore/llint/LowLevelInterpreter.asm
Source/JavaScriptCore/runtime/ClonedArguments.cpp
Source/JavaScriptCore/runtime/ClonedArguments.h
Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
Source/JavaScriptCore/runtime/CommonSlowPaths.h
Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Source/JavaScriptCore/runtime/JSGlobalObject.h
Source/JavaScriptCore/tests/stress/cloned-arguments-elimination.js [new file with mode: 0644]

index 388de31..3771a56 100644 (file)
@@ -1,3 +1,80 @@
+2016-03-14  Keith Miller  <keith_miller@apple.com>
+
+        We should be able to eliminate cloned arguments objects that use the length property
+        https://bugs.webkit.org/show_bug.cgi?id=155391
+
+        Reviewed by Geoffrey Garen.
+
+        Previously if a programmer tried to use arguments.length in a strict function we would not eliminate the
+        arguments object. We were unable to eliminate the arguments object because the user would get a cloned arguments
+        object, which does not special case the length property. Thus, in order to get arguments elimination for cloned
+        we need to add a special case. There are two things that need to happen for the elimination to succeed.
+
+        First, we need to eliminate the CheckStructure blocking the GetByOffset for the length property. In order to
+        eliminate the check structure we need to prove to the Abstract Interpreter that this structure check is
+        unnesssary. This didn't occur before for two reasons: 1) CreateClonedArguments did not set the structure it
+        produced. 2) Even if CreateClonedArguments provided the global object's cloned arguments structure we would
+        transition the new argements object when we added the length property during construction. To fix the second
+        problem we now pre-assign a slot on clonedArgumentsStructure for the length property. Additionally, in order to
+        prevent future transitions of the structure we need to choose an indexing type for the structure. Since, not
+        eliminating the arguments object is so expensive we choose to have all cloned arguments start with continuous
+        indexing type, this avoids transitioning when otherwise we would not have to. In the future we should be smarter
+        about choosing the indexing type but since its relatively rare to have a arguments object escape we don't worry
+        about this for now.
+
+        Additionally, this patch renames all former references of outOfBandArguments to clonedArguments and adds
+        extra instrumentation to DFGArgumentsEliminationPhase.
+
+        * bytecode/BytecodeList.json:
+        * bytecode/BytecodeUseDef.h:
+        (JSC::computeUsesForBytecodeOffset):
+        (JSC::computeDefsForBytecodeOffset):
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::dumpBytecode):
+        * bytecode/ValueRecovery.h:
+        (JSC::ValueRecovery::clonedArgumentsThatWereNotCreated):
+        (JSC::ValueRecovery::outOfBandArgumentsThatWereNotCreated): Deleted.
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::BytecodeGenerator):
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGArgumentsEliminationPhase.cpp:
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * dfg/DFGCapabilities.cpp:
+        (JSC::DFG::capabilityLevel):
+        * dfg/DFGOperations.cpp:
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileCreateClonedArguments):
+        * dfg/DFGStructureRegistrationPhase.cpp:
+        (JSC::DFG::StructureRegistrationPhase::run):
+        * dfg/DFGVariableEventStream.cpp:
+        (JSC::DFG::VariableEventStream::tryToSetConstantRecovery):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileCreateClonedArguments):
+        * ftl/FTLOperations.cpp:
+        (JSC::FTL::operationMaterializeObjectInOSR):
+        * jit/JIT.cpp:
+        (JSC::JIT::privateCompileMainPass):
+        * jit/JIT.h:
+        * jit/JITOpcodes.cpp:
+        (JSC::JIT::emit_op_create_cloned_arguments):
+        (JSC::JIT::emit_op_create_out_of_band_arguments): Deleted.
+        * llint/LowLevelInterpreter.asm:
+        * runtime/ClonedArguments.cpp:
+        (JSC::ClonedArguments::ClonedArguments):
+        (JSC::ClonedArguments::createEmpty):
+        (JSC::ClonedArguments::createWithInlineFrame):
+        (JSC::ClonedArguments::createByCopyingFrom):
+        (JSC::ClonedArguments::createStructure):
+        * runtime/ClonedArguments.h:
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::init):
+        (JSC::JSGlobalObject::visitChildren):
+        * runtime/JSGlobalObject.h:
+        (JSC::JSGlobalObject::clonedArgumentsStructure):
+        (JSC::JSGlobalObject::outOfBandArgumentsStructure): Deleted.
+
 2016-03-14  Saam barati  <sbarati@apple.com>
 
         [ES6] Make JSON.stringify ES6 compatible
index 053b8dc..8b4d65f 100644 (file)
@@ -7,7 +7,7 @@
             { "name" : "op_get_scope", "length" : 2 },
             { "name" : "op_create_direct_arguments", "length" : 2 },
             { "name" : "op_create_scoped_arguments", "length" : 3 },
-            { "name" : "op_create_out_of_band_arguments", "length" : 2 },
+            { "name" : "op_create_cloned_arguments", "length" : 2 },
             { "name" : "op_create_this", "length" : 5 },
             { "name" : "op_to_this", "length" : 4 },
             { "name" : "op_check_tdz", "length" : 2 },
index 14a69f6..38f3115 100644 (file)
@@ -52,7 +52,7 @@ void computeUsesForBytecodeOffset(
     case op_catch:
     case op_profile_control_flow:
     case op_create_direct_arguments:
-    case op_create_out_of_band_arguments:
+    case op_create_cloned_arguments:
     case op_get_rest_length:
     case op_watchdog:
         return;
@@ -410,7 +410,7 @@ void computeDefsForBytecodeOffset(CodeBlock* codeBlock, BytecodeBasicBlock* bloc
     case op_get_scope:
     case op_create_direct_arguments:
     case op_create_scoped_arguments:
-    case op_create_out_of_band_arguments:
+    case op_create_cloned_arguments:
     case op_del_by_id:
     case op_del_by_val:
     case op_unsigned:
index f436c52..63a79b9 100644 (file)
@@ -799,9 +799,9 @@ void CodeBlock::dumpBytecode(
             out.printf("%s, %s", registerName(r0).data(), registerName(r1).data());
             break;
         }
-        case op_create_out_of_band_arguments: {
+        case op_create_cloned_arguments: {
             int r0 = (++it)->u.operand;
-            printLocationAndOp(out, exec, location, it, "create_out_of_band_arguments");
+            printLocationAndOp(out, exec, location, it, "create_cloned_arguments");
             out.printf("%s", registerName(r0).data());
             break;
         }
index 5f6ee9c..8c04f07 100644 (file)
@@ -197,7 +197,7 @@ public:
         return result;
     }
     
-    static ValueRecovery outOfBandArgumentsThatWereNotCreated(DFG::MinifiedID id)
+    static ValueRecovery clonedArgumentsThatWereNotCreated(DFG::MinifiedID id)
     {
         ValueRecovery result;
         result.m_technique = ClonedArgumentsThatWereNotCreated;
index 703ad43..64c7fa9 100644 (file)
@@ -447,8 +447,8 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionNode* functionNode, Unlinke
     }
     
     if (needsArguments && (codeBlock->isStrictMode() || !isSimpleParameterList)) {
-        // Allocate an out-of-bands arguments object.
-        emitOpcode(op_create_out_of_band_arguments);
+        // Allocate a cloned arguments object.
+        emitOpcode(op_create_cloned_arguments);
         instructions().append(m_argumentsRegister->index());
     }
     
index 3225c57..6aff22a 100644 (file)
@@ -1822,7 +1822,7 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         break;
         
     case CreateClonedArguments:
-        forNode(node).setType(m_graph, SpecObjectOther);
+        forNode(node).set(m_graph, m_codeBlock->globalObjectFor(node->origin.semantic)->clonedArgumentsStructure());
         break;
             
     case NewArrowFunction:
index 698cc75..85985c3 100644 (file)
@@ -29,6 +29,7 @@
 #if ENABLE(DFG_JIT)
 
 #include "BytecodeLivenessAnalysisInlines.h"
+#include "ClonedArguments.h"
 #include "DFGArgumentsUtilities.h"
 #include "DFGBasicBlockInlines.h"
 #include "DFGBlockMapInlines.h"
@@ -118,28 +119,30 @@ private:
     // Look for escaping sites, and remove from the candidates set if we see an escape.
     void eliminateCandidatesThatEscape()
     {
-        auto escape = [&] (Edge edge) {
+        auto escape = [&] (Edge edge, Node* source) {
             if (!edge)
                 return;
-            m_candidates.remove(edge.node());
+            bool removed = m_candidates.remove(edge.node());
+            if (removed && verbose)
+                dataLog("eliminating candidate: ", edge.node(), " because it escapes from: ", source, "\n");
         };
         
-        auto escapeBasedOnArrayMode = [&] (ArrayMode mode, Edge edge) {
+        auto escapeBasedOnArrayMode = [&] (ArrayMode mode, Edge edge, Node* source) {
             switch (mode.type()) {
             case Array::DirectArguments:
                 if (edge->op() != CreateDirectArguments)
-                    escape(edge);
+                    escape(edge, source);
                 break;
             
             case Array::Int32:
             case Array::Double:
             case Array::Contiguous:
                 if (edge->op() != CreateClonedArguments)
-                    escape(edge);
+                    escape(edge, source);
                 break;
             
             default:
-                escape(edge);
+                escape(edge, source);
                 break;
             }
         };
@@ -152,14 +155,14 @@ private:
                     break;
                     
                 case GetByVal:
-                    escapeBasedOnArrayMode(node->arrayMode(), node->child1());
-                    escape(node->child2());
-                    escape(node->child3());
+                    escapeBasedOnArrayMode(node->arrayMode(), node->child1(), node);
+                    escape(node->child2(), node);
+                    escape(node->child3(), node);
                     break;
-                    
+
                 case GetArrayLength:
-                    escapeBasedOnArrayMode(node->arrayMode(), node->child1());
-                    escape(node->child2());
+                    escapeBasedOnArrayMode(node->arrayMode(), node->child1(), node);
+                    escape(node->child2(), node);
                     break;
                     
                 case LoadVarargs:
@@ -169,8 +172,8 @@ private:
                 case ConstructVarargs:
                 case TailCallVarargs:
                 case TailCallVarargsInlinedCaller:
-                    escape(node->child1());
-                    escape(node->child3());
+                    escape(node->child1(), node);
+                    escape(node->child3(), node);
                     break;
 
                 case Check:
@@ -183,7 +186,7 @@ private:
                             if (alreadyChecked(edge.useKind(), SpecObject))
                                 return;
                             
-                            escape(edge);
+                            escape(edge, node);
                         });
                     break;
                     
@@ -201,18 +204,19 @@ private:
                     break;
                     
                 case CheckArray:
-                    escapeBasedOnArrayMode(node->arrayMode(), node->child1());
+                    escapeBasedOnArrayMode(node->arrayMode(), node->child1(), node);
                     break;
-                    
-                // FIXME: For cloned arguments, we'd like to allow GetByOffset on length to not be
-                // an escape.
-                // https://bugs.webkit.org/show_bug.cgi?id=143074
+
                     
                 // FIXME: We should be able to handle GetById/GetByOffset on callee.
                 // https://bugs.webkit.org/show_bug.cgi?id=143075
-                    
+
+                case GetByOffset:
+                    if (node->child2()->op() == CreateClonedArguments && node->storageAccessData().offset == clonedArgumentsLengthPropertyOffset)
+                        break;
+                    FALLTHROUGH;
                 default:
-                    m_graph.doToChildren(node, escape);
+                    m_graph.doToChildren(node, [&] (Edge edge) { return escape(edge, node); });
                     break;
                 }
             }
@@ -319,6 +323,8 @@ private:
                     // for this arguments allocation, and we'd have to examine every node in the block,
                     // then we can just eliminate the candidate.
                     if (nodeIndex == block->size() && candidate->owner != block) {
+                        if (verbose)
+                            dataLog("eliminating candidate: ", candidate, " because it is clobbered by: ", block->at(nodeIndex), "\n");
                         m_candidates.remove(candidate);
                         return;
                     }
@@ -344,6 +350,8 @@ private:
                             NoOpClobberize());
                         
                         if (found) {
+                            if (verbose)
+                                dataLog("eliminating candidate: ", candidate, " because it is clobbered by ", block->at(nodeIndex), "\n");
                             m_candidates.remove(candidate);
                             return;
                         }
@@ -412,6 +420,22 @@ private:
                     node->convertToGetStack(data);
                     break;
                 }
+
+                case GetByOffset: {
+                    Node* candidate = node->child2().node();
+                    if (!m_candidates.contains(candidate))
+                        break;
+
+                    if (node->child2()->op() != PhantomClonedArguments)
+                        break;
+
+                    ASSERT(node->storageAccessData().offset == clonedArgumentsLengthPropertyOffset);
+
+                    // Meh, this is kind of hackish - we use an Identity so that we can reuse the
+                    // getArrayLength() helper.
+                    node->convertToIdentityOn(getArrayLength(candidate));
+                    break;
+                }
                     
                 case GetArrayLength: {
                     Node* candidate = node->child1().node();
index 6cadf89..b6be85e 100644 (file)
@@ -4582,11 +4582,11 @@ bool ByteCodeParser::parseBlock(unsigned limit)
             NEXT_OPCODE(op_create_scoped_arguments);
         }
 
-        case op_create_out_of_band_arguments: {
+        case op_create_cloned_arguments: {
             noticeArgumentsUse();
             Node* createArguments = addToGraph(CreateClonedArguments);
             set(VirtualRegister(currentInstruction[1].u.operand), createArguments);
-            NEXT_OPCODE(op_create_out_of_band_arguments);
+            NEXT_OPCODE(op_create_cloned_arguments);
         }
             
         case op_get_from_arguments: {
index d100d97..8b0bd30 100644 (file)
@@ -193,7 +193,7 @@ CapabilityLevel capabilityLevel(OpcodeID opcodeID, CodeBlock* codeBlock, Instruc
     case op_construct_varargs:
     case op_create_direct_arguments:
     case op_create_scoped_arguments:
-    case op_create_out_of_band_arguments:
+    case op_create_cloned_arguments:
     case op_get_from_arguments:
     case op_put_to_arguments:
     case op_jneq_ptr:
index c5741b3..7ab56cf 100644 (file)
@@ -961,15 +961,14 @@ JSCell* JIT_OPERATION operationCreateClonedArgumentsDuringExit(ExecState* exec,
     
     unsigned length = argumentCount - 1;
     ClonedArguments* result = ClonedArguments::createEmpty(
-        vm, codeBlock->globalObject()->outOfBandArgumentsStructure(), callee);
+        vm, codeBlock->globalObject()->clonedArgumentsStructure(), callee, length);
     
     Register* arguments =
         exec->registers() + (inlineCallFrame ? inlineCallFrame->stackOffset : 0) +
         CallFrame::argumentOffset(0);
     for (unsigned i = length; i--;)
-        result->putDirectIndex(exec, i, arguments[i].jsValue());
-    
-    result->putDirect(vm, vm.propertyNames->length, jsNumber(length));
+        result->initializeIndex(vm, i, arguments[i].jsValue());
+
     
     return result;
 }
index 369ca89..4401003 100644 (file)
@@ -5966,7 +5966,7 @@ void SpeculativeJIT::compileCreateClonedArguments(Node* node)
         1, [&] (GPRReg destGPR) {
             m_jit.move(
                 TrustedImmPtr(
-                    m_jit.globalObjectFor(node->origin.semantic)->outOfBandArgumentsStructure()),
+                    m_jit.globalObjectFor(node->origin.semantic)->clonedArgumentsStructure()),
                 destGPR);
         });
     m_jit.setupArgument(0, [&] (GPRReg destGPR) { m_jit.move(GPRInfo::callFrameRegister, destGPR); });
index 0081a10..c014cb9 100644 (file)
@@ -136,7 +136,11 @@ public:
                 case CreateScopedArguments:
                     registerStructure(m_graph.globalObjectFor(node->origin.semantic)->scopedArgumentsStructure());
                     break;
-                    
+
+                case CreateClonedArguments:
+                    registerStructure(m_graph.globalObjectFor(node->origin.semantic)->clonedArgumentsStructure());
+                    break;
+
                 case NewRegexp:
                     registerStructure(m_graph.globalObjectFor(node->origin.semantic)->regExpStructure());
                     break;
index 6392f14..1809621 100644 (file)
@@ -107,7 +107,7 @@ bool VariableEventStream::tryToSetConstantRecovery(ValueRecovery& recovery, Mini
     }
     
     if (node->op() == PhantomClonedArguments) {
-        recovery = ValueRecovery::outOfBandArgumentsThatWereNotCreated(node->id());
+        recovery = ValueRecovery::clonedArgumentsThatWereNotCreated(node->id());
         return true;
     }
     
index 1826b02..6b5248c 100644 (file)
@@ -3500,7 +3500,7 @@ private:
         LValue result = vmCall(
             m_out.int64, m_out.operation(operationCreateClonedArguments), m_callFrame,
             weakPointer(
-                m_graph.globalObjectFor(m_node->origin.semantic)->outOfBandArgumentsStructure()),
+                m_graph.globalObjectFor(m_node->origin.semantic)->clonedArgumentsStructure()),
             getArgumentsStart(), getArgumentsLength().value, getCurrentCallee());
         
         setJSValue(result);
index 45c5bb9..664fc87 100644 (file)
@@ -328,7 +328,7 @@ extern "C" JSCell* JIT_OPERATION operationMaterializeObjectInOSR(
         case PhantomClonedArguments: {
             unsigned length = argumentCount - 1;
             ClonedArguments* result = ClonedArguments::createEmpty(
-                vm, codeBlock->globalObject()->outOfBandArgumentsStructure(), callee);
+                vm, codeBlock->globalObject()->clonedArgumentsStructure(), callee, length);
             
             for (unsigned i = materialization->properties().size(); i--;) {
                 const ExitPropertyValue& property = materialization->properties()[i];
@@ -338,10 +338,9 @@ extern "C" JSCell* JIT_OPERATION operationMaterializeObjectInOSR(
                 unsigned index = property.location().info();
                 if (index >= length)
                     continue;
-                result->putDirectIndex(exec, index, JSValue::decode(values[i]));
+                result->initializeIndex(vm, index, JSValue::decode(values[i]));
             }
             
-            result->putDirect(vm, vm.propertyNames->length, jsNumber(length));
             return result;
         }
         default:
index ac8c132..738e027 100644 (file)
@@ -214,7 +214,7 @@ void JIT::privateCompileMainPass()
         DEFINE_OP(op_to_this)
         DEFINE_OP(op_create_direct_arguments)
         DEFINE_OP(op_create_scoped_arguments)
-        DEFINE_OP(op_create_out_of_band_arguments)
+        DEFINE_OP(op_create_cloned_arguments)
         DEFINE_OP(op_copy_rest)
         DEFINE_OP(op_get_rest_length)
         DEFINE_OP(op_check_tdz)
index e818242..5fd89d4 100644 (file)
@@ -490,7 +490,7 @@ namespace JSC {
         void emit_op_to_this(Instruction*);
         void emit_op_create_direct_arguments(Instruction*);
         void emit_op_create_scoped_arguments(Instruction*);
-        void emit_op_create_out_of_band_arguments(Instruction*);
+        void emit_op_create_cloned_arguments(Instruction*);
         void emit_op_copy_rest(Instruction*);
         void emit_op_get_rest_length(Instruction*);
         void emit_op_check_tdz(Instruction*);
index 6094976..821b7fd 100644 (file)
@@ -1393,9 +1393,9 @@ void JIT::emit_op_create_scoped_arguments(Instruction* currentInstruction)
     slowPathCall.call();
 }
 
-void JIT::emit_op_create_out_of_band_arguments(Instruction* currentInstruction)
+void JIT::emit_op_create_cloned_arguments(Instruction* currentInstruction)
 {
-    JITSlowPathCall slowPathCall(this, currentInstruction, slow_path_create_out_of_band_arguments);
+    JITSlowPathCall slowPathCall(this, currentInstruction, slow_path_create_cloned_arguments);
     slowPathCall.call();
 }
 
index e8ae7e5..39a8660 100644 (file)
@@ -1212,9 +1212,9 @@ _llint_op_create_scoped_arguments:
     dispatch(3)
 
 
-_llint_op_create_out_of_band_arguments:
+_llint_op_create_cloned_arguments:
     traceExecution()
-    callSlowPath(_slow_path_create_out_of_band_arguments)
+    callSlowPath(_slow_path_create_cloned_arguments)
     dispatch(2)
 
 
index 481f16b..b3acbf6 100644 (file)
@@ -36,28 +36,40 @@ STATIC_ASSERT_IS_TRIVIALLY_DESTRUCTIBLE(ClonedArguments);
 
 const ClassInfo ClonedArguments::s_info = { "Arguments", &Base::s_info, 0, CREATE_METHOD_TABLE(ClonedArguments) };
 
-ClonedArguments::ClonedArguments(VM& vm, Structure* structure)
-    : Base(vm, structure, nullptr)
+ClonedArguments::ClonedArguments(VM& vm, Structure* structure, Butterfly* butterfly)
+    : Base(vm, structure, butterfly)
 {
 }
 
 ClonedArguments* ClonedArguments::createEmpty(
-    VM& vm, Structure* structure, JSFunction* callee)
+    VM& vm, Structure* structure, JSFunction* callee, unsigned length)
 {
+    unsigned vectorLength = std::max(BASE_VECTOR_LEN, length);
+    if (vectorLength > MAX_STORAGE_VECTOR_LENGTH)
+        return 0;
+
+    void* temp;
+    if (!vm.heap.tryAllocateStorage(0, Butterfly::totalSize(0, structure->outOfLineCapacity(), true, vectorLength * sizeof(EncodedJSValue)), &temp))
+        return 0;
+    Butterfly* butterfly = Butterfly::fromBase(temp, 0, structure->outOfLineCapacity());
+    butterfly->setVectorLength(vectorLength);
+    butterfly->setPublicLength(length);
+
     ClonedArguments* result =
         new (NotNull, allocateCell<ClonedArguments>(vm.heap))
-        ClonedArguments(vm, structure);
+        ClonedArguments(vm, structure, butterfly);
     result->finishCreation(vm);
+
     result->m_callee.set(vm, result, callee);
+    result->putDirect(vm, clonedArgumentsLengthPropertyOffset, jsNumber(length));
     return result;
 }
 
-ClonedArguments* ClonedArguments::createEmpty(ExecState* exec, JSFunction* callee)
+ClonedArguments* ClonedArguments::createEmpty(ExecState* exec, JSFunction* callee, unsigned length)
 {
     // NB. Some clients might expect that the global object of of this object is the global object
     // of the callee. We don't do this for now, but maybe we should.
-    return createEmpty(
-        exec->vm(), exec->lexicalGlobalObject()->outOfBandArgumentsStructure(), callee);
+    return createEmpty(exec->vm(), exec->lexicalGlobalObject()->clonedArgumentsStructure(), callee, length);
 }
 
 ClonedArguments* ClonedArguments::createWithInlineFrame(ExecState* myFrame, ExecState* targetFrame, InlineCallFrame* inlineCallFrame, ArgumentsMode mode)
@@ -71,7 +83,7 @@ ClonedArguments* ClonedArguments::createWithInlineFrame(ExecState* myFrame, Exec
     else
         callee = jsCast<JSFunction*>(targetFrame->callee());
 
-    ClonedArguments* result = createEmpty(myFrame, callee);
+    ClonedArguments* result;
     
     unsigned length = 0; // Initialize because VC needs it.
     switch (mode) {
@@ -82,25 +94,26 @@ ClonedArguments* ClonedArguments::createWithInlineFrame(ExecState* myFrame, Exec
             else
                 length = inlineCallFrame->arguments.size();
             length--;
-            
+            result = createEmpty(myFrame, callee, length);
+
             for (unsigned i = length; i--;)
-                result->putDirectIndex(myFrame, i, inlineCallFrame->arguments[i + 1].recover(targetFrame));
+                result->initializeIndex(vm, i, inlineCallFrame->arguments[i + 1].recover(targetFrame));
         } else {
             length = targetFrame->argumentCount();
-            
+            result = createEmpty(myFrame, callee, length);
+
             for (unsigned i = length; i--;)
-                result->putDirectIndex(myFrame, i, targetFrame->uncheckedArgument(i));
+                result->initializeIndex(vm, i, targetFrame->uncheckedArgument(i));
         }
         break;
     }
         
     case ArgumentsMode::FakeValues: {
-        length = 0;
+        result = createEmpty(myFrame, callee, 0);
         break;
     } }
-    
-    result->putDirect(vm, vm.propertyNames->length, jsNumber(length), DontEnum);
-    
+
+    ASSERT(myFrame->lexicalGlobalObject()->clonedArgumentsStructure() == result->structure());
     return result;
 }
 
@@ -114,18 +127,24 @@ ClonedArguments* ClonedArguments::createByCopyingFrom(
     JSFunction* callee)
 {
     VM& vm = exec->vm();
-    ClonedArguments* result = createEmpty(vm, structure, callee);
+    ClonedArguments* result = createEmpty(vm, structure, callee, length);
     
     for (unsigned i = length; i--;)
-        result->putDirectIndex(exec, i, argumentStart[i].jsValue());
-    
-    result->putDirect(vm, vm.propertyNames->length, jsNumber(length), DontEnum);
+        result->initializeIndex(vm, i, argumentStart[i].jsValue());
+
+    ASSERT(exec->lexicalGlobalObject()->clonedArgumentsStructure() == result->structure());
     return result;
 }
 
 Structure* ClonedArguments::createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
 {
-    return Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info());
+    // We use contiguous storage because optimizations in the FTL assume that cloned arguments creation always produces the same initial structure.
+
+    Structure* structure = Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info(), NonArrayWithContiguous);
+    PropertyOffset offset;
+    structure = structure->addPropertyTransition(vm, structure, vm.propertyNames->length, DontEnum, offset);
+    ASSERT(offset == clonedArgumentsLengthPropertyOffset);
+    return structure;
 }
 
 bool ClonedArguments::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName ident, PropertySlot& slot)
index 5d22218..a1fa32e 100644 (file)
@@ -44,11 +44,11 @@ public:
     static const unsigned StructureFlags = Base::StructureFlags | OverridesGetOwnPropertySlot | OverridesGetPropertyNames;
     
 private:
-    ClonedArguments(VM&, Structure*);
+    ClonedArguments(VM&, Structure*, Butterfly*);
 
 public:
-    static ClonedArguments* createEmpty(VM&, Structure*, JSFunction* callee);
-    static ClonedArguments* createEmpty(ExecState*, JSFunction* callee);
+    static ClonedArguments* createEmpty(VM&, Structure*, JSFunction* callee, unsigned length);
+    static ClonedArguments* createEmpty(ExecState*, JSFunction* callee, unsigned length);
     static ClonedArguments* createWithInlineFrame(ExecState* myFrame, ExecState* targetFrame, InlineCallFrame*, ArgumentsMode);
     static ClonedArguments* createWithMachineFrame(ExecState* myFrame, ExecState* targetFrame, ArgumentsMode);
     static ClonedArguments* createByCopyingFrom(ExecState*, Structure*, Register* argumentsStart, unsigned length, JSFunction* callee);
@@ -73,6 +73,8 @@ private:
     WriteBarrier<JSFunction> m_callee; // Set to nullptr when we materialize all of our special properties.
 };
 
+static const PropertyOffset clonedArgumentsLengthPropertyOffset = 100;
+
 } // namespace JSC
 
 #endif // ClonedArguments_h
index 46fa1cb..4b63d4e 100644 (file)
@@ -212,7 +212,7 @@ SLOW_PATH_DECL(slow_path_create_scoped_arguments)
     RETURN(ScopedArguments::createByCopying(exec, table, scope));
 }
 
-SLOW_PATH_DECL(slow_path_create_out_of_band_arguments)
+SLOW_PATH_DECL(slow_path_create_cloned_arguments)
 {
     BEGIN();
     RETURN(ClonedArguments::createWithMachineFrame(exec, exec, ArgumentsMode::Cloned));
index 98fe5db..705d861 100644 (file)
@@ -188,7 +188,7 @@ SLOW_PATH_HIDDEN_DECL(slow_path_call_arityCheck);
 SLOW_PATH_HIDDEN_DECL(slow_path_construct_arityCheck);
 SLOW_PATH_HIDDEN_DECL(slow_path_create_direct_arguments);
 SLOW_PATH_HIDDEN_DECL(slow_path_create_scoped_arguments);
-SLOW_PATH_HIDDEN_DECL(slow_path_create_out_of_band_arguments);
+SLOW_PATH_HIDDEN_DECL(slow_path_create_cloned_arguments);
 SLOW_PATH_HIDDEN_DECL(slow_path_create_this);
 SLOW_PATH_HIDDEN_DECL(slow_path_enter);
 SLOW_PATH_HIDDEN_DECL(slow_path_get_callee);
index 004214a..7875230 100644 (file)
@@ -341,7 +341,7 @@ void JSGlobalObject::init(VM& vm)
     m_callbackFunctionStructure.set(vm, this, JSCallbackFunction::createStructure(vm, this, m_functionPrototype.get()));
     m_directArgumentsStructure.set(vm, this, DirectArguments::createStructure(vm, this, m_objectPrototype.get()));
     m_scopedArgumentsStructure.set(vm, this, ScopedArguments::createStructure(vm, this, m_objectPrototype.get()));
-    m_outOfBandArgumentsStructure.set(vm, this, ClonedArguments::createStructure(vm, this, m_objectPrototype.get()));
+    m_clonedArgumentsStructure.set(vm, this, ClonedArguments::createStructure(vm, this, m_objectPrototype.get()));
     m_callbackConstructorStructure.set(vm, this, JSCallbackConstructor::createStructure(vm, this, m_objectPrototype.get()));
     m_callbackObjectStructure.set(vm, this, JSCallbackObject<JSDestructibleObject>::createStructure(vm, this, m_objectPrototype.get()));
 
@@ -887,7 +887,7 @@ void JSGlobalObject::visitChildren(JSCell* cell, SlotVisitor& visitor)
     visitor.append(&thisObject->m_moduleEnvironmentStructure);
     visitor.append(&thisObject->m_directArgumentsStructure);
     visitor.append(&thisObject->m_scopedArgumentsStructure);
-    visitor.append(&thisObject->m_outOfBandArgumentsStructure);
+    visitor.append(&thisObject->m_clonedArgumentsStructure);
     for (unsigned i = 0; i < NumberOfIndexingShapes; ++i)
         visitor.append(&thisObject->m_originalArrayStructureForIndexingShape[i]);
     for (unsigned i = 0; i < NumberOfIndexingShapes; ++i)
index e1cd5da..7c712cd 100644 (file)
@@ -246,7 +246,7 @@ protected:
     WriteBarrier<Structure> m_moduleEnvironmentStructure;
     WriteBarrier<Structure> m_directArgumentsStructure;
     WriteBarrier<Structure> m_scopedArgumentsStructure;
-    WriteBarrier<Structure> m_outOfBandArgumentsStructure;
+    WriteBarrier<Structure> m_clonedArgumentsStructure;
         
     // Lists the actual structures used for having these particular indexing shapes.
     WriteBarrier<Structure> m_originalArrayStructureForIndexingShape[NumberOfIndexingShapes];
@@ -479,7 +479,7 @@ public:
     Structure* moduleEnvironmentStructure() const { return m_moduleEnvironmentStructure.get(); }
     Structure* directArgumentsStructure() const { return m_directArgumentsStructure.get(); }
     Structure* scopedArgumentsStructure() const { return m_scopedArgumentsStructure.get(); }
-    Structure* outOfBandArgumentsStructure() const { return m_outOfBandArgumentsStructure.get(); }
+    Structure* clonedArgumentsStructure() const { return m_clonedArgumentsStructure.get(); }
     Structure* originalArrayStructureForIndexingType(IndexingType indexingType) const
     {
         ASSERT(indexingType & IsArray);
diff --git a/Source/JavaScriptCore/tests/stress/cloned-arguments-elimination.js b/Source/JavaScriptCore/tests/stress/cloned-arguments-elimination.js
new file mode 100644 (file)
index 0000000..0e4e381
--- /dev/null
@@ -0,0 +1,33 @@
+function testModifyLength() {
+    "use strict";
+
+    arguments.length = 10;
+    return arguments.length;
+}
+noInline(testModifyLength);
+
+function testAddOtherProperty() {
+    "use strict";
+
+    arguments.foo = 1;
+    return arguments.length;
+}
+noInline(testAddOtherProperty);
+
+function testAddOtherPropertyInBranch() {
+    "use strict";
+
+    if (arguments[0] % 2)
+        arguments.foo = 1;
+    return arguments.length;
+}
+noInline(testAddOtherPropertyInBranch);
+
+for (i = 0; i < 100000; i++) {
+    if (testModifyLength(1) !== 10)
+        throw "bad";
+    if (testAddOtherProperty(1) !== 1)
+        throw "bad";
+    if (testAddOtherPropertyInBranch(i) !== 1)
+        throw "bad";
+}