Changeset 156376 in webkit

Timestamp:

Sep 24, 2013 5:37:57 PM (11 years ago)

Author:

mhahnenberg@apple.com

Message:

op_get_callee shouldn't use value profiling
​https://bugs.webkit.org/show_bug.cgi?id=121821

Reviewed by Filip Pizlo.

Source/JavaScriptCore:

Currently it's one of the two opcodes that uses m_singletonValue, which is unnecessary.
Our current plan is to remove m_singletonValue so that GenGC can have a simpler story
for handling CodeBlocks/FunctionExecutables during nursery collections.

Instead of using a ValueProfile op_get_callee now has a simple inline cache of the most
recent JSFunction that we saw.

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::CodeBlock):
(JSC::CodeBlock::finalizeUnconditionally):

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::emitCreateThis):

• dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::parseBlock):

• jit/JIT.cpp:

(JSC::JIT::privateCompileSlowCases):

• jit/JIT.h:
• jit/JITOpcodes.cpp:

(JSC::JIT::emit_op_get_callee):
(JSC::JIT::emitSlow_op_get_callee):

• jit/JITOpcodes32_64.cpp:

(JSC::JIT::emit_op_get_callee):
(JSC::JIT::emitSlow_op_get_callee):

• llint/LowLevelInterpreter32_64.asm:
• llint/LowLevelInterpreter64.asm:
• runtime/CommonSlowPaths.cpp:

(JSC::SLOW_PATH_DECL):

• runtime/CommonSlowPaths.h:

LayoutTests:

Added two tests to make sure we didn't regress the performance of op_get_callee.

• js/regress/get_callee_monomorphic-expected.txt: Added.
• js/regress/get_callee_monomorphic.html: Added.
• js/regress/get_callee_polymorphic-expected.txt: Added.
• js/regress/get_callee_polymorphic.html: Added.
• js/regress/script-tests/get_callee_monomorphic.js: Added.
• js/regress/script-tests/get_callee_polymorphic.js: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/js/regress/get_callee_monomorphic-expected.txt (added)
• LayoutTests/js/regress/get_callee_monomorphic.html (added)
• LayoutTests/js/regress/get_callee_polymorphic-expected.txt (added)
• LayoutTests/js/regress/get_callee_polymorphic.html (added)
• LayoutTests/js/regress/script-tests/get_callee_monomorphic.js (added)
• LayoutTests/js/regress/script-tests/get_callee_polymorphic.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/bytecode/CodeBlock.cpp (modified) (2 diffs)
• Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (modified) (1 diff)
• Source/JavaScriptCore/jit/JIT.cpp (modified) (1 diff)
• Source/JavaScriptCore/jit/JIT.h (modified) (1 diff)
• Source/JavaScriptCore/jit/JITOpcodes.cpp (modified) (1 diff)
• Source/JavaScriptCore/jit/JITOpcodes32_64.cpp (modified) (1 diff)
• Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm (modified) (1 diff)
• Source/JavaScriptCore/llint/LowLevelInterpreter64.asm (modified) (1 diff)
• Source/JavaScriptCore/runtime/CommonSlowPaths.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/CommonSlowPaths.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-09-24  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        op_get_callee shouldn't use value profiling
+        https://bugs.webkit.org/show_bug.cgi?id=121821
+
+        Reviewed by Filip Pizlo.
+
+        Added two tests to make sure we didn't regress the performance of op_get_callee.
+
+        * js/regress/get_callee_monomorphic-expected.txt: Added.
+        * js/regress/get_callee_monomorphic.html: Added.
+        * js/regress/get_callee_polymorphic-expected.txt: Added.
+        * js/regress/get_callee_polymorphic.html: Added.
+        * js/regress/script-tests/get_callee_monomorphic.js: Added.
+        * js/regress/script-tests/get_callee_polymorphic.js: Added.
+
 2013-09-24  Bear Travis  <betravis@adobe.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2013-09-24  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        op_get_callee shouldn't use value profiling
+        https://bugs.webkit.org/show_bug.cgi?id=121821
+
+        Reviewed by Filip Pizlo.
+
+        Currently it's one of the two opcodes that uses m_singletonValue, which is unnecessary. 
+        Our current plan is to remove m_singletonValue so that GenGC can have a simpler story 
+        for handling CodeBlocks/FunctionExecutables during nursery collections.
+
+        Instead of using a ValueProfile op_get_callee now has a simple inline cache of the most 
+        recent JSFunction that we saw.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::CodeBlock):
+        (JSC::CodeBlock::finalizeUnconditionally):
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::emitCreateThis):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * jit/JIT.cpp:
+        (JSC::JIT::privateCompileSlowCases):
+        * jit/JIT.h:
+        * jit/JITOpcodes.cpp:
+        (JSC::JIT::emit_op_get_callee):
+        (JSC::JIT::emitSlow_op_get_callee):
+        * jit/JITOpcodes32_64.cpp:
+        (JSC::JIT::emit_op_get_callee):
+        (JSC::JIT::emitSlow_op_get_callee):
+        * llint/LowLevelInterpreter32_64.asm:
+        * llint/LowLevelInterpreter64.asm:
+        * runtime/CommonSlowPaths.cpp:
+        (JSC::SLOW_PATH_DECL):
+        * runtime/CommonSlowPaths.h:
+
 2013-09-24  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -1737 +1737 @@
         case op_to_this:
         case op_get_by_id:
-        case op_call_varargs:
-        case op_get_callee: {
+        case op_call_varargs: {
             ValueProfile* profile = &m_valueProfiles[pc[i + opLength - 1].u.operand];
             ASSERT(profile->m_bytecodeOffset == -1);
@@ -2239 +2238 @@
                 break;
             case op_get_array_length:
+                break;
+            case op_get_callee:
+                if (!curInstruction[2].u.jsCell || Heap::isMarked(curInstruction[2].u.jsCell.get()))
+                    break;
+                if (Options::verboseOSR())
+                    dataLogF("Clearing LLInt get callee with function %p.\n", curInstruction[2].u.jsCell.get());
+                curInstruction[2].u.jsCell.clear();
                 break;
             case op_get_from_scope:

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -1405 +1405 @@
     RefPtr<RegisterID> func = newTemporary(); 
 
-    UnlinkedValueProfile profile = emitProfiledOpcode(op_get_callee);
+    emitOpcode(op_get_callee);
     instructions().append(func->index());
-    instructions().append(profile);
+    instructions().append(0);
 
     size_t begin = instructions().size();

trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

@@ -2008 +2008 @@
             
         case op_get_callee: {
-            ConcurrentJITLocker locker(m_inlineStackTop->m_profiledBlock->m_lock);
-            ValueProfile* profile = currentInstruction[2].u.profile;
-            profile->computeUpdatedPrediction(locker);
-            if (profile->m_singletonValueIsTop
-                || !profile->m_singletonValue
-                || !profile->m_singletonValue.isCell())
+            JSCell* cachedFunction = currentInstruction[2].u.jsCell.get();
+            if (!cachedFunction 
+                || m_inlineStackTop->m_profiledBlock->couldTakeSlowCase(m_currentIndex)
+                || m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadFunction)) {
                 set(currentInstruction[1].u.operand, get(JSStack::Callee));
-            else {
-                ASSERT(profile->m_singletonValue.asCell()->inherits(JSFunction::info()));
+            } else {
+                ASSERT(cachedFunction->inherits(JSFunction::info()));
                 Node* actualCallee = get(JSStack::Callee);
-                addToGraph(CheckFunction, OpInfo(profile->m_singletonValue.asCell()), actualCallee);
-                set(currentInstruction[1].u.operand, addToGraph(WeakJSConstant, OpInfo(profile->m_singletonValue.asCell())));
+                addToGraph(CheckFunction, OpInfo(cachedFunction), actualCallee);
+                set(currentInstruction[1].u.operand, addToGraph(WeakJSConstant, OpInfo(cachedFunction)));
             }
             NEXT_OPCODE(op_get_callee);

trunk/Source/JavaScriptCore/jit/JIT.cpp

@@ -408 +408 @@
         DEFINE_SLOWCASE_OP(op_div)
         DEFINE_SLOWCASE_OP(op_eq)
+        DEFINE_SLOWCASE_OP(op_get_callee)
         case op_get_by_id_out_of_line:
         case op_get_array_length:

trunk/Source/JavaScriptCore/jit/JIT.h

@@ -747 +747 @@
         void emitSlow_op_div(Instruction*, Vector<SlowCaseEntry>::iterator&);
         void emitSlow_op_eq(Instruction*, Vector<SlowCaseEntry>::iterator&);
+        void emitSlow_op_get_callee(Instruction*, Vector<SlowCaseEntry>::iterator&);
         void emitSlow_op_get_by_id(Instruction*, Vector<SlowCaseEntry>::iterator&);
         void emitSlow_op_get_arguments_length(Instruction*, Vector<SlowCaseEntry>::iterator&);

trunk/Source/JavaScriptCore/jit/JITOpcodes.cpp

@@ -879 +879 @@
 {
     int result = currentInstruction[1].u.operand;
+    WriteBarrierBase<JSCell>* cachedFunction = &currentInstruction[2].u.jsCell;
     emitGetFromCallFrameHeaderPtr(JSStack::Callee, regT0);
-    emitValueProfilingSite(regT4);
+
+    loadPtr(cachedFunction, regT2);
+    addSlowCase(branchPtr(NotEqual, regT0, regT2));
+
     emitPutVirtualRegister(result);
+}
+
+void JIT::emitSlow_op_get_callee(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter)
+{
+    linkSlowCase(iter);
+
+    JITSlowPathCall slowPathCall(this, currentInstruction, slow_path_get_callee);
+    slowPathCall.call();
+    emitGetVirtualRegister(currentInstruction[1].u.operand, regT0);
 }
 

trunk/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp

@@ -1130 +1130 @@
 void JIT::emit_op_get_callee(Instruction* currentInstruction)
 {
-    int dst = currentInstruction[1].u.operand;
+    int result = currentInstruction[1].u.operand;
+    WriteBarrierBase<JSCell>* cachedFunction = &currentInstruction[2].u.jsCell;
     emitGetFromCallFrameHeaderPtr(JSStack::Callee, regT0);
+
+    loadPtr(cachedFunction, regT2);
+    addSlowCase(branchPtr(NotEqual, regT0, regT2));
+
     move(TrustedImm32(JSValue::CellTag), regT1);
-    emitValueProfilingSite(regT4);
-    emitStore(dst, regT1, regT0);
+    emitStore(result, regT1, regT0);
+}
+
+void JIT::emitSlow_op_get_callee(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter)
+{
+    linkSlowCase(iter);
+
+    JITSlowPathCall slowPathCall(this, currentInstruction, slow_path_get_callee);
+    slowPathCall.call();
+    emitLoad(currentInstruction[1].u.operand, regT1, regT0);
 }
 

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm

@@ -415 +415 @@
     loadi 4[PC], t0
     loadp PayloadOffset + Callee[cfr], t1
-    valueProfile(CellTag, t1, 8, t2)
+    loadpFromInstruction(2, t2)
+    bpneq t1, t2, .opGetCalleeSlow
     storei CellTag, TagOffset[cfr, t0, 8]
     storei t1, PayloadOffset[cfr, t0, 8]
     dispatch(3)
 
+.opGetCalleeSlow:
+    callSlowPath(_slow_path_get_callee)
+    dispatch(3)
 
 _llint_op_to_this:

trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

@@ -296 +296 @@
     loadisFromInstruction(1, t0)
     loadp Callee[cfr], t1
-    valueProfile(t1, 2, t2)
+    loadpFromInstruction(2, t2)
+    bpneq t1, t2, .opGetCalleeSlow
     storep t1, [cfr, t0, 8]
     dispatch(3)
 
+.opGetCalleeSlow:
+    callSlowPath(_slow_path_get_callee)
+    dispatch(3)
 
 _llint_op_to_this:

trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp

@@ -194 +194 @@
 }
 
+SLOW_PATH_DECL(slow_path_get_callee)
+{
+    BEGIN();
+    JSFunction* callee = jsCast<JSFunction*>(exec->callee());
+    pc[2].u.jsCell.set(exec->vm(), exec->codeBlock()->ownerExecutable(), callee);
+    RETURN(callee);
+}
+
 SLOW_PATH_DECL(slow_path_create_arguments)
 {

trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.h

@@ -154 +154 @@
 SLOW_PATH_HIDDEN_DECL(slow_path_create_arguments);
 SLOW_PATH_HIDDEN_DECL(slow_path_create_this);
+SLOW_PATH_HIDDEN_DECL(slow_path_get_callee);
 SLOW_PATH_HIDDEN_DECL(slow_path_to_this);
 SLOW_PATH_HIDDEN_DECL(slow_path_not);