Changeset 253350 in webkit for trunk/Source/JavaScriptCore/dfg

Timestamp:

Dec 10, 2019, 3:08:04 PM (5 years ago)

Author:

[email protected]

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):

File:

• trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp

@@ -1634 +1634 @@
 
     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()) {
@@ -1639 +1652 @@
 
             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(