Title: [253350] trunk/Source/_javascript_Core
- Revision
- 253350
- Author
- [email protected]
- Date
- 2019-12-10 15:08:04 -0800 (Tue, 10 Dec 2019)
Log Message
methodOfGettingAValueProfileFor should return argument value profiles even when node and operandNode are the same origin
https://bugs.webkit.org/show_bug.cgi?id=205083
Reviewed by Yusuke Suzuki.
Inside methodOfGettingAValueProfileFor, we only grab profiles when the child
node and the parent node were from different code origins. This policy doesn't
make sense when the child node is the load of an argument value. In that case,
we can always just grab the argument profile.
We might want to reconsider this policy in general, since it's common for a
node to emit a GetLocal to grab its incoming arguments (this is frequently
done in the DFG when reloading locals across basic blocks).
This fixes an OSR exit compile loop inside Speedometer 2's React-Redux-TodoMVC
benchmark. That benchmark would repeatedly exit inside CompareStrictEq by
repeatedly speculating Object. That node would run with 95% incoming Objects,
and 5% incoming strings, and because we didn't grab the argument value profile
during exit, we never updated the profile with the String type information.
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::methodOfGettingAValueProfileFor):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (253349 => 253350)
--- trunk/Source/_javascript_Core/ChangeLog 2019-12-10 23:06:55 UTC (rev 253349)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-12-10 23:08:04 UTC (rev 253350)
@@ -1,3 +1,28 @@
+2019-12-10 Saam Barati <[email protected]>
+
+ methodOfGettingAValueProfileFor should return argument value profiles even when node and operandNode are the same origin
+ https://bugs.webkit.org/show_bug.cgi?id=205083
+
+ Reviewed by Yusuke Suzuki.
+
+ Inside methodOfGettingAValueProfileFor, we only grab profiles when the child
+ node and the parent node were from different code origins. This policy doesn't
+ make sense when the child node is the load of an argument value. In that case,
+ we can always just grab the argument profile.
+
+ We might want to reconsider this policy in general, since it's common for a
+ node to emit a GetLocal to grab its incoming arguments (this is frequently
+ done in the DFG when reloading locals across basic blocks).
+
+ This fixes an OSR exit compile loop inside Speedometer 2's React-Redux-TodoMVC
+ benchmark. That benchmark would repeatedly exit inside CompareStrictEq by
+ repeatedly speculating Object. That node would run with 95% incoming Objects,
+ and 5% incoming strings, and because we didn't grab the argument value profile
+ during exit, we never updated the profile with the String type information.
+
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::methodOfGettingAValueProfileFor):
+
2019-12-10 Commit Queue <[email protected]>
Unreviewed, rolling out r253321.
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (253349 => 253350)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2019-12-10 23:06:55 UTC (rev 253349)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2019-12-10 23:08:04 UTC (rev 253350)
@@ -1633,20 +1633,24 @@
// This represents IR like `CurrentNode(@operandNode)`. For example: `GetByVal(..., Int32:@GetLocal)`.
for (Node* node = operandNode; node;) {
+ if (node->accessesStack(*this)) {
+ if (m_form != SSA && node->local().isArgument()) {
+ int argument = node->local().toArgument();
+ Node* argumentNode = m_rootToArguments.find(block(0))->value[argument];
+ // FIXME: We should match SetArgumentDefinitely nodes at other entrypoints as well:
+ // https://bugs.webkit.org/show_bug.cgi?id=175841
+ if (argumentNode && node->variableAccessData() == argumentNode->variableAccessData()) {
+ CodeBlock* profiledBlock = baselineCodeBlockFor(node->origin.semantic);
+ return &profiledBlock->valueProfileForArgument(argument);
+ }
+ }
+ }
+
// currentNode is null when we're doing speculation checks for checkArgumentTypes().
if (!currentNode || node->origin.semantic != currentNode->origin.semantic || !currentNode->hasResult()) {
CodeBlock* profiledBlock = baselineCodeBlockFor(node->origin.semantic);
if (node->accessesStack(*this)) {
- if (m_form != SSA && node->local().isArgument()) {
- int argument = node->local().toArgument();
- Node* argumentNode = m_rootToArguments.find(block(0))->value[argument];
- // FIXME: We should match SetArgumentDefinitely nodes at other entrypoints as well:
- // https://bugs.webkit.org/show_bug.cgi?id=175841
- if (argumentNode && node->variableAccessData() == argumentNode->variableAccessData())
- return &profiledBlock->valueProfileForArgument(argument);
- }
-
if (node->op() == GetLocal) {
return MethodOfGettingAValueProfile::fromLazyOperand(
profiledBlock,
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
