Diff
Modified: trunk/JSTests/ChangeLog (204991 => 204992)
--- trunk/JSTests/ChangeLog 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/JSTests/ChangeLog 2016-08-25 22:11:59 UTC (rev 204992)
@@ -1,3 +1,18 @@
+2016-08-25 JF Bastien <[email protected]>
+
+ TryGetById should have a ValueProfile so that it can predict its output type
+ https://bugs.webkit.org/show_bug.cgi?id=160921
+
+ Reviewed by Saam Barati.
+
+ * microbenchmarks/try-get-by-id-basic.js: Added.
+ (const.check):
+ (const.bench.f.const.fooPlusBar.createBuiltin):
+ * microbenchmarks/try-get-by-id-polymorphic.js: Added.
+ (const.check):
+ (fooPlusBar.createBuiltin):
+ (bench):
+
2016-08-25 Caio Lima <[email protected]>
NewRegexp should not prevent inlining
Added: trunk/JSTests/microbenchmarks/try-get-by-id-basic.js (0 => 204992)
--- trunk/JSTests/microbenchmarks/try-get-by-id-basic.js (rev 0)
+++ trunk/JSTests/microbenchmarks/try-get-by-id-basic.js 2016-08-25 22:11:59 UTC (rev 204992)
@@ -0,0 +1,28 @@
+// Test tryGetById's value profiling feedback without going too polymorphic.
+
+var it = 1e5;
+
+const check = (got, expect) => { if (got != expect) throw "Error: bad result got " + got + " expected " + expect; };
+
+const bench = f => {
+ // Re-create the builtin each time, so each benchmark gets its own value prediction.
+ const fooPlusBar = createBuiltin(`(function(o) { return @tryGetById(o, "foo") + @tryGetById(o, "bar"); })`);
+ noInline(fooPlusBar);
+ f(fooPlusBar);
+}
+noInline(bench);
+
+// Non bool int32.
+o = { foo: 42, bar: 1337 };
+bench(builtin => { var res = 0; for (var i = 0; i < it; ++i) res += builtin(o); check(res, (o.foo + o.bar) * it); });
+
+// Non int double.
+p = { foo: Math.PI, bar: Math.E };
+bench(builtin => { var res = 0.; for (var i = 0; i < it; ++i) res += builtin(p); check(Math.round(res), Math.round((p.foo + p.bar) * it)); });
+
+// String ident.
+s = { foo: "", bar: "⌽" };
+bench(builtin => { var res = ""; for (var i = 0; i < it; ++i) res += builtin(s); check(res.length, (s.foo.length + s.bar.length) * it); });
+
+// Again: non bool int32.
+bench(builtin => { var res = 0; for (var i = 0; i < it; ++i) res += builtin(o); check(res, (o.foo + o.bar) * it); });
Added: trunk/JSTests/microbenchmarks/try-get-by-id-polymorphic.js (0 => 204992)
--- trunk/JSTests/microbenchmarks/try-get-by-id-polymorphic.js (rev 0)
+++ trunk/JSTests/microbenchmarks/try-get-by-id-polymorphic.js 2016-08-25 22:11:59 UTC (rev 204992)
@@ -0,0 +1,27 @@
+// Test tryGetById's value profiling feedback after it's too polymorphic.
+
+var it = 1e5;
+
+const check = (got, expect) => { if (got != expect) throw "Error: bad result got " + got + " expected " + expect; };
+
+fooPlusBar = createBuiltin(`(function(o) { return @tryGetById(o, "foo") + @tryGetById(o, "bar"); })`);
+noInline(fooPlusBar);
+
+const bench = f => { f(); }
+noInline(bench);
+
+// Non bool int32.
+o = { foo: 42, bar: 1337 };
+bench(() => { var res = 0; for (var i = 0; i < it; ++i) res += fooPlusBar(o); check(res, (o.foo + o.bar) * it); });
+
+// Non int double.
+p = { foo: Math.PI, bar: Math.E };
+bench(() => { var res = 0.; for (var i = 0; i < it; ++i) res += fooPlusBar(p); check(Math.round(res), Math.round((p.foo + p.bar) * it)); });
+
+// String ident.
+// This gets too polymorphic for the engine's taste.
+s = { foo: "", bar: "⌽" };
+bench(() => { var res = ""; for (var i = 0; i < it; ++i) res += fooPlusBar(s); check(res.length, (s.foo.length + s.bar.length) * it); });
+
+// Again: non bool int32.
+bench(() => { var res = 0; for (var i = 0; i < it; ++i) res += fooPlusBar(o); check(res, (o.foo + o.bar) * it); });
Modified: trunk/Source/_javascript_Core/ChangeLog (204991 => 204992)
--- trunk/Source/_javascript_Core/ChangeLog 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-08-25 22:11:59 UTC (rev 204992)
@@ -1,3 +1,38 @@
+2016-08-25 JF Bastien <[email protected]>
+
+ TryGetById should have a ValueProfile so that it can predict its output type
+ https://bugs.webkit.org/show_bug.cgi?id=160921
+
+ Reviewed by Saam Barati.
+
+ Add a ValueProfile to TryGetById, and make sure DFG picks it up.
+
+ A microbenchmark for perfectly predicted computation shows a 20%
+ runtime reduction with no hit if the prediction goes polymorphic.
+
+ * bytecode/BytecodeList.json:
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::dumpBytecode):
+ (JSC::CodeBlock::finishCreation):
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::emitTryGetById):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ * dfg/DFGNode.h:
+ (JSC::DFG::Node::hasHeapPrediction):
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * jit/JITPropertyAccess.cpp:
+ (JSC::JIT::emit_op_try_get_by_id):
+ * jit/JITPropertyAccess32_64.cpp:
+ (JSC::JIT::emit_op_try_get_by_id):
+ * llint/LLIntSlowPaths.cpp:
+ (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+ * llint/LowLevelInterpreter.asm:
+
2016-08-25 Csaba Osztrogonác <[email protected]>
generate-js-builtins.py should generate platform independent files
Modified: trunk/Source/_javascript_Core/bytecode/BytecodeList.json (204991 => 204992)
--- trunk/Source/_javascript_Core/bytecode/BytecodeList.json 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeList.json 2016-08-25 22:11:59 UTC (rev 204992)
@@ -67,7 +67,7 @@
{ "name" : "op_get_by_id_unset", "length" : 9 },
{ "name" : "op_get_by_id_with_this", "length" : 5 },
{ "name" : "op_get_by_val_with_this", "length" : 5 },
- { "name" : "op_try_get_by_id", "length" : 4 },
+ { "name" : "op_try_get_by_id", "length" : 5 },
{ "name" : "op_put_by_id", "length" : 9 },
{ "name" : "op_put_by_id_with_this", "length" : 5 },
{ "name" : "op_del_by_id", "length" : 4 },
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (204991 => 204992)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2016-08-25 22:11:59 UTC (rev 204992)
@@ -1141,6 +1141,7 @@
int id0 = (++it)->u.operand;
printLocationAndOp(out, exec, location, it, "try_get_by_id");
out.printf("%s, %s, %s", registerName(r0).data(), registerName(r1).data(), idName(id0, identifier(id0)).data());
+ dumpValueProfiling(out, it, hasPrintedProfiling);
break;
}
case op_get_by_id:
@@ -2106,6 +2107,7 @@
}
case op_get_direct_pname:
case op_get_by_id:
+ case op_try_get_by_id:
case op_get_from_arguments:
case op_to_number: {
ValueProfile* profile = "" - 1].u.operand];
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (204991 => 204992)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2016-08-25 22:11:59 UTC (rev 204992)
@@ -2454,10 +2454,11 @@
{
ASSERT_WITH_MESSAGE(!parseIndex(property), "Indexed properties are not supported with tryGetById.");
- emitOpcode(op_try_get_by_id);
+ UnlinkedValueProfile profile = ""
instructions().append(kill(dst));
instructions().append(base->index());
instructions().append(addConstant(property));
+ instructions().append(profile);
return dst;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (204991 => 204992)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-08-25 22:11:59 UTC (rev 204992)
@@ -238,7 +238,6 @@
Node* store(Node* base, unsigned identifier, const PutByIdVariant&, Node* value);
- void handleTryGetById(int destinationOperand, Node* base, unsigned identifierNumber, const GetByIdStatus&);
void handleGetById(
int destinationOperand, SpeculatedType, Node* base, unsigned identifierNumber, GetByIdStatus, AccessType);
void emitPutById(
@@ -1204,14 +1203,26 @@
bool m_hasDebuggerEnabled;
};
+// The idiom:
+// if (true) { ...; goto label; } else label: continue
+// Allows using NEXT_OPCODE as a statement, even in unbraced if+else, while containing a `continue`.
+// The more common idiom:
+// do { ...; } while (false)
+// Doesn't allow using `continue`.
#define NEXT_OPCODE(name) \
- m_currentIndex += OPCODE_LENGTH(name); \
- continue
+ if (true) { \
+ m_currentIndex += OPCODE_LENGTH(name); \
+ goto WTF_CONCAT(NEXT_OPCODE_, __LINE__); /* Need a unique label: usable more than once per function. */ \
+ } else \
+ WTF_CONCAT(NEXT_OPCODE_, __LINE__): \
+ continue
+// Chain expressions with comma-operator so LAST_OPCODE can be used as a statement.
#define LAST_OPCODE(name) \
- m_currentIndex += OPCODE_LENGTH(name); \
- m_exitOK = false; \
- return shouldContinueParsing
+ return \
+ m_currentIndex += OPCODE_LENGTH(name), \
+ m_exitOK = false, \
+ shouldContinueParsing
ByteCodeParser::Terminality ByteCodeParser::handleCall(Instruction* pc, NodeType op, CallMode callMode)
{
@@ -4189,21 +4200,7 @@
NEXT_OPCODE(op_put_by_val_with_this);
}
- case op_try_get_by_id: {
- Node* base = get(VirtualRegister(currentInstruction[2].u.operand));
- unsigned identifierNumber = m_inlineStackTop->m_identifierRemap[currentInstruction[3].u.operand];
- UniquedStringImpl* uid = m_graph.identifiers()[identifierNumber];
-
- GetByIdStatus getByIdStatus = GetByIdStatus::computeFor(
- m_inlineStackTop->m_profiledBlock, m_dfgCodeBlock,
- m_inlineStackTop->m_stubInfos, m_dfgStubInfos,
- currentCodeOrigin(), uid);
-
- handleGetById(currentInstruction[1].u.operand, SpecHeapTop, base, identifierNumber, getByIdStatus, AccessType::GetPure);
-
- NEXT_OPCODE(op_try_get_by_id);
- }
-
+ case op_try_get_by_id:
case op_get_by_id:
case op_get_by_id_proto_load:
case op_get_by_id_unset:
@@ -4218,11 +4215,15 @@
m_inlineStackTop->m_profiledBlock, m_dfgCodeBlock,
m_inlineStackTop->m_stubInfos, m_dfgStubInfos,
currentCodeOrigin(), uid);
+ AccessType type = op_try_get_by_id == opcodeID ? AccessType::GetPure : AccessType::Get;
handleGetById(
- currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus, AccessType::Get);
+ currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus, type);
- NEXT_OPCODE(op_get_by_id);
+ if (op_try_get_by_id == opcodeID)
+ NEXT_OPCODE(op_try_get_by_id); // Opcode's length is different from others in this case.
+ else
+ NEXT_OPCODE(op_get_by_id);
}
case op_get_by_id_with_this: {
Node* base = get(VirtualRegister(currentInstruction[2].u.operand));
@@ -4557,11 +4558,10 @@
// If the call is terminal then we should not parse any further bytecodes as the TailCall will exit the function.
// If the call is not terminal, however, then we want the subsequent op_ret/op_jump to update metadata and clean
// things up.
- if (terminality == NonTerminal) {
+ if (terminality == NonTerminal)
NEXT_OPCODE(op_tail_call);
- } else {
+ else
LAST_OPCODE(op_tail_call);
- }
}
case op_construct:
@@ -4582,11 +4582,10 @@
// If the call is terminal then we should not parse any further bytecodes as the TailCall will exit the function.
// If the call is not terminal, however, then we want the subsequent op_ret/op_jump to update metadata and clean
// things up.
- if (terminality == NonTerminal) {
+ if (terminality == NonTerminal)
NEXT_OPCODE(op_tail_call_varargs);
- } else {
+ else
LAST_OPCODE(op_tail_call_varargs);
- }
}
case op_tail_call_forward_arguments: {
@@ -4599,11 +4598,10 @@
// If the call is terminal then we should not parse any further bytecodes as the TailCall will exit the function.
// If the call is not terminal, however, then we want the subsequent op_ret/op_jump to update metadata and clean
// things up.
- if (terminality == NonTerminal) {
+ if (terminality == NonTerminal)
NEXT_OPCODE(op_tail_call);
- } else {
+ else
LAST_OPCODE(op_tail_call);
- }
}
case op_construct_varargs: {
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (204991 => 204992)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2016-08-25 22:11:59 UTC (rev 204992)
@@ -1404,6 +1404,7 @@
case GetDirectPname:
case GetById:
case GetByIdFlush:
+ case TryGetById:
case GetByVal:
case Call:
case TailCallInlinedCaller:
Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (204991 => 204992)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2016-08-25 22:11:59 UTC (rev 204992)
@@ -685,10 +685,6 @@
setPrediction(SpecBytecodeTop);
break;
}
- case TryGetById: {
- setPrediction(SpecBytecodeTop);
- break;
- }
case ArrayPop:
case ArrayPush:
case RegExpExec:
@@ -697,6 +693,7 @@
case StringReplaceRegExp:
case GetById:
case GetByIdFlush:
+ case TryGetById:
case GetByOffset:
case MultiGetByOffset:
case GetDirectPname:
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (204991 => 204992)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2016-08-25 22:11:59 UTC (rev 204992)
@@ -4153,6 +4153,8 @@
}
case GetById: {
+ // FIXME https://bugs.webkit.org/show_bug.cgi?id=161158
+ // dedup with SpeculativeJIT::compileTryGetById and 64-bit version of this.
switch (node->child1().useKind()) {
case CellUse: {
SpeculateCellOperand base(this, node->child1());
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (204991 => 204992)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2016-08-25 22:11:59 UTC (rev 204992)
@@ -4095,6 +4095,8 @@
}
case GetById: {
+ // FIXME https://bugs.webkit.org/show_bug.cgi?id=161158
+ // dedup with SpeculativeJIT::compileTryGetById and 32-bit version of this.
switch (node->child1().useKind()) {
case CellUse: {
SpeculateCellOperand base(this, node->child1());
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (204991 => 204992)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2016-08-25 22:11:59 UTC (rev 204992)
@@ -584,6 +584,7 @@
addSlowCase(gen.slowPathJump());
m_getByIds.append(gen);
+ emitValueProfilingSite();
emitPutVirtualRegister(resultVReg);
}
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp (204991 => 204992)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp 2016-08-25 22:11:59 UTC (rev 204992)
@@ -599,6 +599,7 @@
addSlowCase(gen.slowPathJump());
m_getByIds.append(gen);
+ emitValueProfilingSite();
emitStore(dst, regT1, regT0);
}
Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (204991 => 204992)
--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2016-08-25 22:11:59 UTC (rev 204992)
@@ -572,8 +572,9 @@
PropertySlot slot(baseValue, PropertySlot::PropertySlot::InternalMethodType::VMInquiry);
baseValue.getPropertySlot(exec, ident, slot);
+ JSValue result = slot.getPureResult();
- LLINT_RETURN(slot.getPureResult());
+ LLINT_RETURN_PROFILED(op_try_get_by_id, result);
}
static void setupGetByIdPrototypeCache(ExecState* exec, VM& vm, Instruction* pc, JSCell* baseCell, PropertySlot& slot, const Identifier& ident)
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm (204991 => 204992)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm 2016-08-25 22:11:59 UTC (rev 204992)
@@ -1405,7 +1405,7 @@
_llint_op_try_get_by_id:
traceExecution()
callOpcodeSlowPath(_llint_slow_path_try_get_by_id)
- dispatch(4)
+ dispatch(5)
_llint_op_del_by_id:
Modified: trunk/Source/WTF/ChangeLog (204991 => 204992)
--- trunk/Source/WTF/ChangeLog 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/WTF/ChangeLog 2016-08-25 22:11:59 UTC (rev 204992)
@@ -1,3 +1,14 @@
+2016-08-25 JF Bastien <[email protected]>
+
+ TryGetById should have a ValueProfile so that it can predict its output type
+ https://bugs.webkit.org/show_bug.cgi?id=160921
+
+ Reviewed by Saam Barati.
+
+ Add WTF_CONCAT to StdLibExtras.h
+
+ * wtf/StdLibExtras.h:
+
2016-08-25 Johan K. Jensen <[email protected]>
Don't store networkLoadTiming in the disk cache
Modified: trunk/Source/WTF/wtf/StdLibExtras.h (204991 => 204992)
--- trunk/Source/WTF/wtf/StdLibExtras.h 2016-08-25 22:11:25 UTC (rev 204991)
+++ trunk/Source/WTF/wtf/StdLibExtras.h 2016-08-25 22:11:59 UTC (rev 204992)
@@ -71,6 +71,11 @@
#define STRINGIZE(exp) #exp
#define STRINGIZE_VALUE_OF(exp) STRINGIZE(exp)
+// WTF_CONCAT: concatenate two symbols into one, even expandable macros
+#define WTF_CONCAT_INTERNAL_DONT_USE(a, b) a ## b
+#define WTF_CONCAT(a, b) WTF_CONCAT_INTERNAL_DONT_USE(a, b)
+
+
// Make "PRId64" format specifier work for Visual C++ on Windows.
#if OS(WINDOWS) && !defined(PRId64)
#define PRId64 "lld"
