Title: [226426] trunk/Source/_javascript_Core
Revision
226426
Author
sbar...@apple.com
Date
2018-01-04 16:01:32 -0800 (Thu, 04 Jan 2018)

Log Message

Add a new pattern matching rule to Graph::methodOfGettingAValueProfileFor for SetLocal(@nodeWithHeapPrediction)
https://bugs.webkit.org/show_bug.cgi?id=181296

Reviewed by Filip Pizlo.

Inside Speedometer's Ember test, there is a recompile loop like:
a: GetByVal(..., semanticOriginX)
b: SetLocal(Cell:@a, semanticOriginX)

where the cell check always fails. For reasons I didn't investigate, the
baseline JIT's value profiling doesn't accurately capture the GetByVal's
result.

However, when compiling this cell speculation check in the DFG, we get a null
MethodOfGettingAValueProfile inside Graph::methodOfGettingAValueProfileFor for
this IR pattern because both @a and @b have the same semantic origin. We
should not follow the same semantic origin heuristic when dealing with
SetLocal since SetLocal(@nodeWithHeapPrediction) is such a common IR pattern.
For patterns like this, we introduce a new heuristic: @NodeThatDoesNotProduceAValue(@nodeWithHeapPrediction).
For this IR pattern, we will update the value profile for the semantic origin
for @nodeWithHeapPrediction. So, for the Speedometer example above, we
will correctly update the GetByVal's value profile, which will prevent
an OSR exit loop.

* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::methodOfGettingAValueProfileFor):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (226425 => 226426)


--- trunk/Source/_javascript_Core/ChangeLog	2018-01-05 00:00:25 UTC (rev 226425)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-01-05 00:01:32 UTC (rev 226426)
@@ -1,3 +1,32 @@
+2018-01-04  Saam Barati  <sbar...@apple.com>
+
+        Add a new pattern matching rule to Graph::methodOfGettingAValueProfileFor for SetLocal(@nodeWithHeapPrediction)
+        https://bugs.webkit.org/show_bug.cgi?id=181296
+
+        Reviewed by Filip Pizlo.
+
+        Inside Speedometer's Ember test, there is a recompile loop like:
+        a: GetByVal(..., semanticOriginX)
+        b: SetLocal(Cell:@a, semanticOriginX)
+        
+        where the cell check always fails. For reasons I didn't investigate, the
+        baseline JIT's value profiling doesn't accurately capture the GetByVal's
+        result.
+        
+        However, when compiling this cell speculation check in the DFG, we get a null
+        MethodOfGettingAValueProfile inside Graph::methodOfGettingAValueProfileFor for
+        this IR pattern because both @a and @b have the same semantic origin. We
+        should not follow the same semantic origin heuristic when dealing with
+        SetLocal since SetLocal(@nodeWithHeapPrediction) is such a common IR pattern.
+        For patterns like this, we introduce a new heuristic: @NodeThatDoesNotProduceAValue(@nodeWithHeapPrediction).
+        For this IR pattern, we will update the value profile for the semantic origin
+        for @nodeWithHeapPrediction. So, for the Speedometer example above, we
+        will correctly update the GetByVal's value profile, which will prevent
+        an OSR exit loop.
+
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::methodOfGettingAValueProfileFor):
+
 2018-01-04  Keith Miller  <keith_mil...@apple.com>
 
         Array Storage operations sometimes did not update the indexing mask correctly.

Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (226425 => 226426)


--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp	2018-01-05 00:00:25 UTC (rev 226425)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp	2018-01-05 00:01:32 UTC (rev 226426)
@@ -1617,9 +1617,11 @@
 
 MethodOfGettingAValueProfile Graph::methodOfGettingAValueProfileFor(Node* currentNode, Node* operandNode)
 {
+    // This represents IR like `CurrentNode(@operandNode)`. For example: `GetByVal(..., Int32:@GetLocal)`.
+
     for (Node* node = operandNode; node;) {
         // currentNode is null when we're doing speculation checks for checkArgumentTypes().
-        if (!currentNode || node->origin.semantic != currentNode->origin.semantic) {
+        if (!currentNode || node->origin.semantic != currentNode->origin.semantic || !currentNode->hasResult()) {
             CodeBlock* profiledBlock = baselineCodeBlockFor(node->origin.semantic);
 
             if (node->accessesStack(*this)) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to