Add argument_count bytecode for concat
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Jun 2016 23:06:39 +0000 (23:06 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Jun 2016 23:06:39 +0000 (23:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=158358

Reviewed by Geoffrey Garen.

This patch adds a new argument count bytecode. Normally, we would
just make sure that the argument.length bytecode was fast enough
that we shouldn't need such an bytecode.  However, for the case of
Array.prototype.concat the overhead of the arguments object
allocation in the LLInt was too high and caused regressions.

* bytecode/BytecodeIntrinsicRegistry.h:
* bytecode/BytecodeList.json:
* bytecode/BytecodeUseDef.h:
(JSC::computeUsesForBytecodeOffset):
(JSC::computeDefsForBytecodeOffset):
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::dumpBytecode):
* bytecompiler/NodesCodegen.cpp:
(JSC::BytecodeIntrinsicNode::emit_intrinsic_argumentCount):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::getArgumentCount):
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGCapabilities.cpp:
(JSC::DFG::capabilityLevel):
* jit/JIT.cpp:
(JSC::JIT::privateCompileMainPass):
* jit/JIT.h:
* jit/JITOpcodes.cpp:
(JSC::JIT::emit_op_argument_count):
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
* tests/stress/argument-count-bytecode.js: Added.
(inlineCount):
(inlineCount1):
(inlineCount2):
(inlineCountVarArgs):
(assert):

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

27 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.h
Source/JavaScriptCore/bytecode/BytecodeList.json
Source/JavaScriptCore/bytecode/BytecodeUseDef.h
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGArgumentsUtilities.cpp
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGCapabilities.cpp
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGDoesGC.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGMayExit.cpp
Source/JavaScriptCore/dfg/DFGNodeType.h
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
Source/JavaScriptCore/dfg/DFGSafeToExecute.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/jit/JIT.cpp
Source/JavaScriptCore/jit/JIT.h
Source/JavaScriptCore/jit/JITOpcodes.cpp
Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
Source/JavaScriptCore/tests/stress/argument-count-bytecode.js [new file with mode: 0644]

index ca53ea7..eabec3c 100644 (file)
@@ -1,3 +1,44 @@
+2016-06-03  Keith Miller  <keith_miller@apple.com>
+
+        Add argument_count bytecode for concat
+        https://bugs.webkit.org/show_bug.cgi?id=158358
+
+        Reviewed by Geoffrey Garen.
+
+        This patch adds a new argument count bytecode. Normally, we would
+        just make sure that the argument.length bytecode was fast enough
+        that we shouldn't need such an bytecode.  However, for the case of
+        Array.prototype.concat the overhead of the arguments object
+        allocation in the LLInt was too high and caused regressions.
+
+        * bytecode/BytecodeIntrinsicRegistry.h:
+        * bytecode/BytecodeList.json:
+        * bytecode/BytecodeUseDef.h:
+        (JSC::computeUsesForBytecodeOffset):
+        (JSC::computeDefsForBytecodeOffset):
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::dumpBytecode):
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::BytecodeIntrinsicNode::emit_intrinsic_argumentCount):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::getArgumentCount):
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * dfg/DFGCapabilities.cpp:
+        (JSC::DFG::capabilityLevel):
+        * jit/JIT.cpp:
+        (JSC::JIT::privateCompileMainPass):
+        * jit/JIT.h:
+        * jit/JITOpcodes.cpp:
+        (JSC::JIT::emit_op_argument_count):
+        * llint/LowLevelInterpreter32_64.asm:
+        * llint/LowLevelInterpreter64.asm:
+        * tests/stress/argument-count-bytecode.js: Added.
+        (inlineCount):
+        (inlineCount1):
+        (inlineCount2):
+        (inlineCountVarArgs):
+        (assert):
+
 2016-06-03  Geoffrey Garen  <ggaren@apple.com>
 
         Clients of PolymorphicAccess::addCases shouldn't have to malloc
index bbe9a91..93e9680 100644 (file)
@@ -40,6 +40,7 @@ class RegisterID;
 class Identifier;
 
 #define JSC_COMMON_BYTECODE_INTRINSIC_FUNCTIONS_EACH_NAME(macro) \
+    macro(argumentCount) \
     macro(assert) \
     macro(isObject) \
     macro(tryGetById) \
index 483be64..8e9805a 100644 (file)
@@ -9,6 +9,7 @@
             { "name" : "op_create_scoped_arguments", "length" : 3 },
             { "name" : "op_create_cloned_arguments", "length" : 2 },
             { "name" : "op_create_this", "length" : 5 },
+            { "name" : "op_argument_count", "length" : 2 },
             { "name" : "op_to_this", "length" : 4 },
             { "name" : "op_check_tdz", "length" : 2 },
             { "name" : "op_new_object", "length" : 4 },
index 17bb58b..c37a79c 100644 (file)
@@ -53,6 +53,7 @@ void computeUsesForBytecodeOffset(
     case op_jmp:
     case op_new_object:
     case op_enter:
+    case op_argument_count:
     case op_catch:
     case op_profile_control_flow:
     case op_create_direct_arguments:
@@ -362,6 +363,7 @@ void computeDefsForBytecodeOffset(CodeBlock* codeBlock, BytecodeBasicBlock* bloc
 #undef LLINT_HELPER_OPCODES
         return;
     // These all have a single destination for the first argument.
+    case op_argument_count:
     case op_to_index_string:
     case op_get_enumerable_length:
     case op_has_indexed_property:
index a648ae6..57f315f 100644 (file)
@@ -818,6 +818,11 @@ void CodeBlock::dumpBytecode(
             out.printf("%s", registerName(r0).data());
             break;
         }
+        case op_argument_count: {
+            int r0 = (++it)->u.operand;
+            printLocationOpAndRegisterOperand(out, exec, location, it, "argument_count", r0);
+            break;
+        }
         case op_copy_rest: {
             int r0 = (++it)->u.operand;
             int r1 = (++it)->u.operand;
index a40259b..dfb4167 100644 (file)
@@ -854,6 +854,13 @@ RegisterID* BytecodeIntrinsicNode::emitBytecode(BytecodeGenerator& generator, Re
     return (this->*m_emitter)(generator, dst);
 }
 
+RegisterID* BytecodeIntrinsicNode::emit_intrinsic_argumentCount(BytecodeGenerator& generator, RegisterID* dst)
+{
+    ASSERT(!m_args->m_listNode);
+
+    return generator.emitUnaryNoDstOp(op_argument_count, generator.finalDestination(dst));
+}
+
 RegisterID* BytecodeIntrinsicNode::emit_intrinsic_assert(BytecodeGenerator& generator, RegisterID* dst)
 {
 #ifndef NDEBUG
index 4010c12..35f6f43 100644 (file)
@@ -1973,7 +1973,7 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         forNode(node).setType(m_graph, SpecFunction);
         break;
         
-    case GetArgumentCount:
+    case GetArgumentCountIncludingThis:
         forNode(node).setType(SpecInt32Only);
         break;
         
index 75c21b3..fce2afc 100644 (file)
@@ -76,7 +76,7 @@ Node* emitCodeToGetArgumentsArrayLength(
     
     Node* argumentCount;
     if (!inlineCallFrame)
-        argumentCount = insertionSet.insertNode(nodeIndex, SpecInt32Only, GetArgumentCount, origin);
+        argumentCount = insertionSet.insertNode(nodeIndex, SpecInt32Only, GetArgumentCountIncludingThis, origin);
     else {
         VirtualRegister argumentCountRegister(inlineCallFrame->stackOffset + JSStack::ArgumentCount);
         
index 482bd0b..bb3be17 100644 (file)
@@ -196,6 +196,7 @@ private:
     Terminality handleVarargsCall(Instruction* pc, NodeType op, CallMode);
     void emitFunctionChecks(CallVariant, Node* callTarget, VirtualRegister thisArgumnt);
     void emitArgumentPhantoms(int registerOffset, int argumentCountIncludingThis);
+    Node* getArgumentCount();
     unsigned inliningCost(CallVariant, int argumentCountIncludingThis, CallMode); // Return UINT_MAX if it's not an inlining candidate. By convention, intrinsics have a cost of 1.
     // Handle inlining. Return true if it succeeded, false if we need to plant a call.
     bool handleInlining(Node* callTargetNode, int resultOperand, const CallLinkStatus&, int registerOffset, VirtualRegister thisArgument, VirtualRegister argumentsArgument, unsigned argumentsOffset, int argumentCountIncludingThis, unsigned nextOffset, NodeType callOp, InlineCallFrame::Kind, SpeculatedType prediction);
@@ -1322,6 +1323,19 @@ void ByteCodeParser::emitFunctionChecks(CallVariant callee, Node* callTarget, Vi
     addToGraph(CheckCell, OpInfo(m_graph.freeze(calleeCell)), callTargetForCheck, thisArgument);
 }
 
+Node* ByteCodeParser::getArgumentCount()
+{
+    Node* argumentCount;
+    if (m_inlineStackTop->m_inlineCallFrame) {
+        if (m_inlineStackTop->m_inlineCallFrame->isVarargs())
+            argumentCount = get(VirtualRegister(JSStack::ArgumentCount));
+        else
+            argumentCount = jsConstant(m_graph.freeze(jsNumber(m_inlineStackTop->m_inlineCallFrame->arguments.size()))->value());
+    } else
+        argumentCount = addToGraph(GetArgumentCountIncludingThis, OpInfo(0), OpInfo(SpecInt32Only));
+    return argumentCount;
+}
+
 void ByteCodeParser::emitArgumentPhantoms(int registerOffset, int argumentCountIncludingThis)
 {
     for (int i = 0; i < argumentCountIncludingThis; ++i)
@@ -4886,7 +4900,14 @@ bool ByteCodeParser::parseBlock(unsigned limit)
             set(VirtualRegister(currentInstruction[1].u.operand), result);
             NEXT_OPCODE(op_get_scope);
         }
-            
+
+        case op_argument_count: {
+            Node* sub = addToGraph(ArithSub, OpInfo(Arith::Unchecked), OpInfo(SpecInt32Only), getArgumentCount(), addToGraph(JSConstant, OpInfo(m_constantOne)));
+
+            set(VirtualRegister(currentInstruction[1].u.operand), sub);
+            NEXT_OPCODE(op_argument_count);
+        }
+
         case op_create_direct_arguments: {
             noticeArgumentsUse();
             Node* createArguments = addToGraph(CreateDirectArguments);
index 8eaba38..2267c3e 100644 (file)
@@ -106,6 +106,7 @@ CapabilityLevel capabilityLevel(OpcodeID opcodeID, CodeBlock* codeBlock, Instruc
     switch (opcodeID) {
     case op_enter:
     case op_to_this:
+    case op_argument_count:
     case op_check_tdz:
     case op_create_this:
     case op_bitand:
index e670e5b..110f91b 100644 (file)
@@ -502,7 +502,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         def(HeapLocation(StackLoc, AbstractHeap(Stack, JSStack::Callee)), LazyNode(node));
         return;
         
-    case GetArgumentCount:
+    case GetArgumentCountIncludingThis:
         read(AbstractHeap(Stack, JSStack::ArgumentCount));
         def(HeapLocation(StackPayloadLoc, AbstractHeap(Stack, JSStack::ArgumentCount)), LazyNode(node));
         return;
index 3936d26..d718b4d 100644 (file)
@@ -50,7 +50,7 @@ bool doesGC(Graph& graph, Node* node)
     case LazyJSConstant:
     case Identity:
     case GetCallee:
-    case GetArgumentCount:
+    case GetArgumentCountIncludingThis:
     case GetRestLength:
     case GetLocal:
     case SetLocal:
index 377a02a..a36c3a4 100644 (file)
@@ -1508,7 +1508,7 @@ private:
         case DoubleConstant:
         case GetLocal:
         case GetCallee:
-        case GetArgumentCount:
+        case GetArgumentCountIncludingThis:
         case GetRestLength:
         case Flush:
         case PhantomLocal:
index aee1cff..570b10a 100644 (file)
@@ -71,7 +71,7 @@ ExitMode mayExitImpl(Graph& graph, Node* node, StateType& state)
     case KillStack:
     case GetStack:
     case GetCallee:
-    case GetArgumentCount:
+    case GetArgumentCountIncludingThis:
     case GetRestLength:
     case GetScope:
     case PhantomLocal:
index 0608102..66d9e0c 100644 (file)
@@ -53,7 +53,7 @@ namespace JSC { namespace DFG {
     macro(ToThis, NodeResultJS) \
     macro(CreateThis, NodeResultJS) /* Note this is not MustGenerate since we're returning it anyway. */ \
     macro(GetCallee, NodeResultJS) \
-    macro(GetArgumentCount, NodeResultInt32) \
+    macro(GetArgumentCountIncludingThis, NodeResultInt32) \
     \
     /* Nodes for local variable access. These nodes are linked together using Phi nodes. */\
     /* Any two nodes that are part of the same Phi graph will share the same */\
index a06dd4d..30c3f83 100644 (file)
@@ -737,7 +737,7 @@ private:
             break;
         }
             
-        case GetArgumentCount: {
+        case GetArgumentCountIncludingThis: {
             setPrediction(SpecInt32Only);
             break;
         }
index 627d797..4c1c623 100644 (file)
@@ -142,7 +142,7 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node)
     case ToThis:
     case CreateThis:
     case GetCallee:
-    case GetArgumentCount:
+    case GetArgumentCountIncludingThis:
     case GetRestLength:
     case GetLocal:
     case SetLocal:
index 50e78d3..6b3862a 100644 (file)
@@ -4070,7 +4070,7 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
         
-    case GetArgumentCount: {
+    case GetArgumentCountIncludingThis: {
         GPRTemporary result(this);
         m_jit.load32(JITCompiler::payloadFor(JSStack::ArgumentCount), result.gpr());
         int32Result(result.gpr(), node);
index 0d01802..744139b 100644 (file)
@@ -4007,7 +4007,7 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
         
-    case GetArgumentCount: {
+    case GetArgumentCountIncludingThis: {
         GPRTemporary result(this);
         m_jit.load32(JITCompiler::payloadFor(JSStack::ArgumentCount), result.gpr());
         int32Result(result.gpr(), node);
index 21aee05..a371e66 100644 (file)
@@ -160,7 +160,7 @@ inline CapabilityLevel canCompile(Node* node)
     case GetExecutable:
     case GetScope:
     case GetCallee:
-    case GetArgumentCount:
+    case GetArgumentCountIncludingThis:
     case ToString:
     case CallStringConstructor:
     case MakeRope:
index 34a84e5..7456462 100644 (file)
@@ -776,8 +776,8 @@ private:
         case GetCallee:
             compileGetCallee();
             break;
-        case GetArgumentCount:
-            compileGetArgumentCount();
+        case GetArgumentCountIncludingThis:
+            compileGetArgumentCountIncludingThis();
             break;
         case GetScope:
             compileGetScope();
@@ -4670,7 +4670,7 @@ private:
         setJSValue(m_out.loadPtr(addressFor(JSStack::Callee)));
     }
     
-    void compileGetArgumentCount()
+    void compileGetArgumentCountIncludingThis()
     {
         setInt32(m_out.load32(payloadFor(JSStack::ArgumentCount)));
     }
index 30497ab..95b1ac1 100644 (file)
@@ -223,6 +223,7 @@ void JIT::privateCompileMainPass()
         DEFINE_OP(op_create_direct_arguments)
         DEFINE_OP(op_create_scoped_arguments)
         DEFINE_OP(op_create_cloned_arguments)
+        DEFINE_OP(op_argument_count)
         DEFINE_OP(op_copy_rest)
         DEFINE_OP(op_get_rest_length)
         DEFINE_OP(op_check_tdz)
index 77910b1..82e831e 100644 (file)
@@ -491,6 +491,7 @@ namespace JSC {
         void emit_op_create_direct_arguments(Instruction*);
         void emit_op_create_scoped_arguments(Instruction*);
         void emit_op_create_cloned_arguments(Instruction*);
+        void emit_op_argument_count(Instruction*);
         void emit_op_copy_rest(Instruction*);
         void emit_op_get_rest_length(Instruction*);
         void emit_op_check_tdz(Instruction*);
index 932a957..e38fd16 100644 (file)
@@ -1419,6 +1419,16 @@ void JIT::emit_op_create_cloned_arguments(Instruction* currentInstruction)
     slowPathCall.call();
 }
 
+void JIT::emit_op_argument_count(Instruction* currentInstruction)
+{
+    int dst = currentInstruction[1].u.operand;
+    load32(payloadFor(JSStack::ArgumentCount), regT0);
+    sub32(TrustedImm32(1), regT0);
+    JSValueRegs result = JSValueRegs::withTwoAvailableRegs(regT0, regT1);
+    boxInt32(regT0, result);
+    emitPutVirtualRegister(dst, result);
+}
+
 void JIT::emit_op_copy_rest(Instruction* currentInstruction)
 {
     JITSlowPathCall slowPathCall(this, currentInstruction, slow_path_copy_rest);
index f5f7560..f43000a 100644 (file)
@@ -677,6 +677,17 @@ _llint_op_enter:
     dispatch(1)
 
 
+_llint_op_argument_count:
+    traceExecution()
+    loadisFromInstruction(1, t2)
+    loadi PayloadOffset + ArgumentCount[cfr], t0
+    subi 1, t0
+    move Int32Tag, t1
+    storei t1, TagOffset[cfr, t2, 8]
+    storei t0, PayloadOffset[cfr, t2, 8]
+    dispatch(2)
+
+
 _llint_op_get_scope:
     traceExecution()
     loadi Callee + PayloadOffset[cfr], t0
index b9c17a5..2802353 100644 (file)
@@ -584,6 +584,16 @@ _llint_op_enter:
     dispatch(1)
 
 
+_llint_op_argument_count:
+    traceExecution()
+    loadisFromInstruction(1, t1)
+    loadi PayloadOffset + ArgumentCount[cfr], t0
+    subi 1, t0
+    orq TagTypeNumber, t0
+    storeq t0, [cfr, t1, 8]
+    dispatch(2)
+
+
 _llint_op_get_scope:
     traceExecution()
     loadp Callee[cfr], t0
diff --git a/Source/JavaScriptCore/tests/stress/argument-count-bytecode.js b/Source/JavaScriptCore/tests/stress/argument-count-bytecode.js
new file mode 100644 (file)
index 0000000..b19b82f
--- /dev/null
@@ -0,0 +1,38 @@
+count = createBuiltin("(function () { return @argumentCount(); })");
+countNoInline = createBuiltin("(function () { return @argumentCount(); })");
+noInline(countNoInline);
+
+
+function inlineCount() { return count(); }
+noInline(inlineCount);
+
+function inlineCount1() { return count(1); }
+noInline(inlineCount1);
+
+function inlineCount2() { return count(1,2); }
+noInline(inlineCount2);
+
+function inlineCountVarArgs(list) { return count(...list); }
+noInline(inlineCountVarArgs);
+
+function assert(condition, message) {
+    if (!condition)
+        throw new Error(message);
+}
+
+for (i = 0; i < 1000000; i++) {
+    assert(count(1,1,2) === 3, i);
+    assert(count() === 0, i);
+    assert(count(1) === 1, i);
+    assert(count(...[1,2,3,4,5]) === 5, i);
+    assert(count(...[]) === 0, i);
+    assert(inlineCount() === 0, i);
+    assert(inlineCount1() === 1, i);
+    assert(inlineCount2() === 2, i);
+    assert(inlineCountVarArgs([1,2,3,4]) === 4, i);
+    assert(inlineCountVarArgs([]) === 0, i);
+    // Insert extra junk so that inlineCountVarArgs.arguments.length !== count.arguments.length
+    assert(inlineCountVarArgs([1], 2, 4) === 1, i);
+    assert(countNoInline(4) === 1, i)
+    assert(countNoInline() === 0, i);
+}