Add a new pattern matching rule to Graph::methodOfGettingAValueProfileFor for SetLoca...
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jan 2018 00:01:32 +0000 (00:01 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jan 2018 00:01:32 +0000 (00:01 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@226426 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGGraph.cpp

index 2d32f1c..e63f2bc 100644 (file)
@@ -1,3 +1,32 @@
+2018-01-04  Saam Barati  <sbarati@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_miller@apple.com>
 
         Array Storage operations sometimes did not update the indexing mask correctly.
index 4c82712..630a32c 100644 (file)
@@ -1617,9 +1617,11 @@ ControlEquivalenceAnalysis& Graph::ensureControlEquivalenceAnalysis()
 
 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)) {