- Revision
- 226436
- Author
- [email protected]
- Date
- 2018-01-04 21:12:51 -0800 (Thu, 04 Jan 2018)
Log Message
Do value profiling in to_this
https://bugs.webkit.org/show_bug.cgi?id=181299
Reviewed by Filip Pizlo.
This patch adds value profiling to to_this. We use the result of the value
profiling only for strict mode code when we don't predict that the input is
of a specific type. This helps when the input is SpecCellOther. Such cells
might implement a custom ToThis, which can produce an arbitrary result. Before
this patch, in prediction propagation, we were saying that a ToThis with a
SpecCellOther input also produced SpecCellOther. However, this is incorrect,
given that the input may implement ToThis that produces an arbitrary result.
This is seen inside Speedometer. This patch fixes an OSR exit loop in Speedometer.
Interestingly, this patch only does value profiling on the slow path. The fast
path of to_this in the LLInt/baseline just perform a structure check. If it
passes, the result is the same as the input. Therefore, doing value profiling
from the fast path wouldn't actually produce new information for the ValueProfile.
* bytecode/BytecodeDumper.cpp:
(JSC::BytecodeDumper<Block>::dumpBytecode):
* bytecode/BytecodeList.json:
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::finishCreation):
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::emitToThis):
* bytecompiler/BytecodeGenerator.h:
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGNode.h:
(JSC::DFG::Node::hasHeapPrediction):
* dfg/DFGPredictionPropagationPhase.cpp:
* runtime/CommonSlowPaths.cpp:
(JSC::SLOW_PATH_DECL):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (226435 => 226436)
--- trunk/Source/_javascript_Core/ChangeLog 2018-01-05 04:34:07 UTC (rev 226435)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-01-05 05:12:51 UTC (rev 226436)
@@ -1,3 +1,41 @@
+2018-01-04 Saam Barati <[email protected]>
+
+ Do value profiling in to_this
+ https://bugs.webkit.org/show_bug.cgi?id=181299
+
+ Reviewed by Filip Pizlo.
+
+ This patch adds value profiling to to_this. We use the result of the value
+ profiling only for strict mode code when we don't predict that the input is
+ of a specific type. This helps when the input is SpecCellOther. Such cells
+ might implement a custom ToThis, which can produce an arbitrary result. Before
+ this patch, in prediction propagation, we were saying that a ToThis with a
+ SpecCellOther input also produced SpecCellOther. However, this is incorrect,
+ given that the input may implement ToThis that produces an arbitrary result.
+ This is seen inside Speedometer. This patch fixes an OSR exit loop in Speedometer.
+
+ Interestingly, this patch only does value profiling on the slow path. The fast
+ path of to_this in the LLInt/baseline just perform a structure check. If it
+ passes, the result is the same as the input. Therefore, doing value profiling
+ from the fast path wouldn't actually produce new information for the ValueProfile.
+
+ * bytecode/BytecodeDumper.cpp:
+ (JSC::BytecodeDumper<Block>::dumpBytecode):
+ * bytecode/BytecodeList.json:
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::finishCreation):
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::BytecodeGenerator):
+ (JSC::BytecodeGenerator::emitToThis):
+ * bytecompiler/BytecodeGenerator.h:
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ * dfg/DFGNode.h:
+ (JSC::DFG::Node::hasHeapPrediction):
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ * runtime/CommonSlowPaths.cpp:
+ (JSC::SLOW_PATH_DECL):
+
2018-01-04 Yusuke Suzuki <[email protected]>
[DFG] Unify ToNumber implementation in 32bit and 64bit by changing 32bit Int32Tag and LowestTag
Modified: trunk/Source/_javascript_Core/bytecode/BytecodeDumper.cpp (226435 => 226436)
--- trunk/Source/_javascript_Core/bytecode/BytecodeDumper.cpp 2018-01-05 04:34:07 UTC (rev 226435)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeDumper.cpp 2018-01-05 05:12:51 UTC (rev 226436)
@@ -705,6 +705,7 @@
if (structure)
out.print(", cache(struct = ", RawPointer(structure), ")");
out.print(", ", getToThisStatus(*(++it)));
+ dumpValueProfiling(out, it, hasPrintedProfiling);
break;
}
case op_check_tdz: {
Modified: trunk/Source/_javascript_Core/bytecode/BytecodeList.json (226435 => 226436)
--- trunk/Source/_javascript_Core/bytecode/BytecodeList.json 2018-01-05 04:34:07 UTC (rev 226435)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeList.json 2018-01-05 05:12:51 UTC (rev 226436)
@@ -15,7 +15,7 @@
{"cachedCallee" : "WriteBarrier<JSCell>"}]},
{ "name" : "op_get_argument", "length" : 4 },
{ "name" : "op_argument_count", "length" : 2 },
- { "name" : "op_to_this", "length" : 4 },
+ { "name" : "op_to_this", "length" : 5 },
{ "name" : "op_check_tdz", "length" : 2 },
{ "name" : "op_new_object", "length" : 4 },
{ "name" : "op_new_array", "length" : 5 },
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (226435 => 226436)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2018-01-05 04:34:07 UTC (rev 226435)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2018-01-05 05:12:51 UTC (rev 226436)
@@ -574,6 +574,11 @@
break;
}
+ case op_to_this: {
+ linkValueProfile(i, opLength);
+ break;
+ }
+
case op_in:
case op_put_by_val:
case op_put_by_val_direct: {
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (226435 => 226436)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2018-01-05 04:34:07 UTC (rev 226435)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2018-01-05 05:12:51 UTC (rev 226436)
@@ -589,11 +589,7 @@
// FIXME: Emit to_this only when Generator uses it.
// https://bugs.webkit.org/show_bug.cgi?id=151586
- m_codeBlock->addPropertyAccessInstruction(instructions().size());
- emitOpcode(op_to_this);
- instructions().append(kill(&m_thisRegister));
- instructions().append(0);
- instructions().append(0);
+ emitToThis();
emitMove(m_generatorRegister, &m_calleeRegister);
emitCreateThis(m_generatorRegister);
@@ -611,11 +607,7 @@
if (parseMode != SourceParseMode::AsyncArrowFunctionMode) {
// FIXME: Emit to_this only when AsyncFunctionBody uses it.
// https://bugs.webkit.org/show_bug.cgi?id=151586
- m_codeBlock->addPropertyAccessInstruction(instructions().size());
- emitOpcode(op_to_this);
- instructions().append(kill(&m_thisRegister));
- instructions().append(0);
- instructions().append(0);
+ emitToThis();
}
emitNewObject(m_generatorRegister);
@@ -677,13 +669,8 @@
shouldEmitToThis = true;
}
- if (shouldEmitToThis) {
- m_codeBlock->addPropertyAccessInstruction(instructions().size());
- emitOpcode(op_to_this);
- instructions().append(kill(&m_thisRegister));
- instructions().append(0);
- instructions().append(0);
- }
+ if (shouldEmitToThis)
+ emitToThis();
}
}
break;
@@ -5226,6 +5213,16 @@
}
}
+void BytecodeGenerator::emitToThis()
+{
+ m_codeBlock->addPropertyAccessInstruction(instructions().size());
+ UnlinkedValueProfile profile = ""
+ instructions().append(kill(&m_thisRegister));
+ instructions().append(0);
+ instructions().append(0);
+ instructions().append(profile);
+}
+
} // namespace JSC
namespace WTF {
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (226435 => 226436)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2018-01-05 04:34:07 UTC (rev 226435)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2018-01-05 05:12:51 UTC (rev 226436)
@@ -967,6 +967,8 @@
bool isNewTargetUsedInInnerArrowFunction();
bool isArgumentsUsedInInnerArrowFunction();
+ void emitToThis();
+
public:
bool isSuperUsedInInnerArrowFunction();
bool isSuperCallUsedInInnerArrowFunction();
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (226435 => 226436)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-05 04:34:07 UTC (rev 226435)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-05 05:12:51 UTC (rev 226436)
@@ -4360,7 +4360,7 @@
|| m_inlineStackTop->m_profiledBlock->couldTakeSlowCase(m_currentIndex)
|| m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadCache)
|| (op1->op() == GetLocal && op1->variableAccessData()->structureCheckHoistingFailed())) {
- setThis(addToGraph(ToThis, op1));
+ setThis(addToGraph(ToThis, OpInfo(), OpInfo(getPrediction()), op1));
} else {
addToGraph(
CheckStructure,
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (226435 => 226436)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2018-01-05 04:34:07 UTC (rev 226435)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2018-01-05 05:12:51 UTC (rev 226436)
@@ -1593,6 +1593,7 @@
case AtomicsXor:
case GetDynamicVar:
case ExtractValueFromWeakMapGet:
+ case ToThis:
return true;
default:
return false;
Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (226435 => 226436)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-01-05 04:34:07 UTC (rev 226435)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-01-05 05:12:51 UTC (rev 226436)
@@ -442,13 +442,13 @@
}
SpeculatedType prediction = node->child1()->prediction();
- if (prediction) {
+ if (ecmaMode == StrictMode)
+ changed |= mergePrediction(node->getHeapPrediction());
+ else if (prediction) {
if (prediction & ~SpecObject) {
// Wrapper objects are created only in sloppy mode.
- if (ecmaMode != StrictMode) {
- prediction &= SpecObject;
- prediction = mergeSpeculations(prediction, SpecObjectOther);
- }
+ prediction &= SpecObject;
+ prediction = mergeSpeculations(prediction, SpecObjectOther);
}
changed |= mergePrediction(prediction);
}
Modified: trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (226435 => 226436)
--- trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2018-01-05 04:34:07 UTC (rev 226435)
+++ trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2018-01-05 05:12:51 UTC (rev 226436)
@@ -267,7 +267,13 @@
pc[3].u.toThisStatus = ToThisConflicted;
pc[2].u.structure.clear();
}
- RETURN(v1.toThis(exec, exec->codeBlock()->isStrictMode() ? StrictMode : NotStrictMode));
+ // Note: We only need to do this value profiling here on the slow path. The fast path
+ // just returns the input to to_this if the structure check succeeds. If the structure
+ // check succeeds, doing value profiling here is equivalent to doing it with a potentially
+ // different object that still has the same structure on the fast path since it'll produce
+ // the same SpeculatedType. Therefore, we don't need to worry about value profiling on the
+ // fast path.
+ RETURN_PROFILED(op_to_this, v1.toThis(exec, exec->codeBlock()->isStrictMode() ? StrictMode : NotStrictMode));
}
SLOW_PATH_DECL(slow_path_throw_tdz_error)
