DFG should inline code blocks that use scoped variable access
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Dec 2012 20:25:24 +0000 (20:25 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Dec 2012 20:25:24 +0000 (20:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=103974

Reviewed by Oliver Hunt.

This mostly just turns on something we could have done all along, but also adds a few key
necessities to make this right:

1) Constant folding of SkipScope, since if we inline with a known JSFunction* then the
   scope is constant.

2) Interference analysis for GetLocal<->PutScopedVar and SetLocal<->GetScopedVar.

This is not meant to be a speed-up on major benchmarks since we don't yet inline most
closure calls for entirely unrelated reasons. But on toy programs it can be >2x faster.

* dfg/DFGAbstractState.cpp:
(JSC::DFG::AbstractState::execute):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::getScope):
(JSC::DFG::ByteCodeParser::parseResolveOperations):
* dfg/DFGCSEPhase.cpp:
(JSC::DFG::CSEPhase::scopedVarLoadElimination):
(JSC::DFG::CSEPhase::scopedVarStoreElimination):
(JSC::DFG::CSEPhase::getLocalLoadElimination):
(JSC::DFG::CSEPhase::setLocalStoreElimination):
* dfg/DFGCapabilities.h:
(JSC::DFG::canInlineResolveOperations):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractState.cpp
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGCSEPhase.cpp
Source/JavaScriptCore/dfg/DFGCapabilities.h

index b7cd46fe52e79d599408e1ddcc85ed33d4b6bcb0..8478d4f330ff29a584a99837f51826ff659066e9 100644 (file)
@@ -1,3 +1,34 @@
+2012-12-03  Filip Pizlo  <fpizlo@apple.com>
+
+        DFG should inline code blocks that use scoped variable access
+        https://bugs.webkit.org/show_bug.cgi?id=103974
+
+        Reviewed by Oliver Hunt.
+
+        This mostly just turns on something we could have done all along, but also adds a few key
+        necessities to make this right:
+        
+        1) Constant folding of SkipScope, since if we inline with a known JSFunction* then the
+           scope is constant.
+        
+        2) Interference analysis for GetLocal<->PutScopedVar and SetLocal<->GetScopedVar.
+        
+        This is not meant to be a speed-up on major benchmarks since we don't yet inline most
+        closure calls for entirely unrelated reasons. But on toy programs it can be >2x faster.
+
+        * dfg/DFGAbstractState.cpp:
+        (JSC::DFG::AbstractState::execute):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::getScope):
+        (JSC::DFG::ByteCodeParser::parseResolveOperations):
+        * dfg/DFGCSEPhase.cpp:
+        (JSC::DFG::CSEPhase::scopedVarLoadElimination):
+        (JSC::DFG::CSEPhase::scopedVarStoreElimination):
+        (JSC::DFG::CSEPhase::getLocalLoadElimination):
+        (JSC::DFG::CSEPhase::setLocalStoreElimination):
+        * dfg/DFGCapabilities.h:
+        (JSC::DFG::canInlineResolveOperations):
+
 2012-12-03  Filip Pizlo  <fpizlo@apple.com>
 
         Replace JSValue::description() with JSValue::dump(PrintStream&)
index a1dc064a1a97b02fd79f634b75343d2a9826b989..772f97f6dc5c71056dc34a719f2fc389f8c699a6 100644 (file)
@@ -1385,11 +1385,21 @@ bool AbstractState::execute(unsigned indexInBlock)
             
     case GetMyScope:
     case SkipTopScope:
-    case SkipScope:
         node.setCanExit(false);
         forNode(nodeIndex).set(SpecCellOther);
         break;
 
+    case SkipScope: {
+        node.setCanExit(false);
+        JSValue child = forNode(node.child1()).value();
+        if (child && trySetConstant(nodeIndex, JSValue(jsCast<JSScope*>(child.asCell())->next()))) {
+            m_foundConstants = true;
+            break;
+        }
+        forNode(nodeIndex).set(SpecCellOther);
+        break;
+    }
+
     case GetScopeRegisters:
         node.setCanExit(false);
         forNode(node.child1()).filter(SpecCell);
index 58c42100674d8c2068018e157870ee8abb68fe9b..8ca3d2a18bb36b4ff1f5306d1db6f4fc0b63d1b0 100644 (file)
@@ -1886,9 +1886,16 @@ void ByteCodeParser::prepareToParseBlock()
 
 NodeIndex ByteCodeParser::getScope(bool skipTop, unsigned skipCount)
 {
-    NodeIndex localBase = addToGraph(GetMyScope);
-    if (skipTop)
+    NodeIndex localBase;
+    if (m_inlineStackTop->m_inlineCallFrame) {
+        ASSERT(m_inlineStackTop->m_inlineCallFrame->callee);
+        localBase = cellConstant(m_inlineStackTop->m_inlineCallFrame->callee->scope());
+    } else
+        localBase = addToGraph(GetMyScope);
+    if (skipTop) {
+        ASSERT(!m_inlineStackTop->m_inlineCallFrame);
         localBase = addToGraph(SkipTopScope, localBase);
+    }
     for (unsigned n = skipCount; n--;)
         localBase = addToGraph(SkipScope, localBase);
     return localBase;
@@ -1954,7 +1961,6 @@ bool ByteCodeParser::parseResolveOperations(SpeculatedType prediction, unsigned
             break;
 
         case ResolveOperation::SkipScopes:
-            ASSERT(!m_inlineStackTop->m_inlineCallFrame);
             skipCount += pc->m_scopesToSkip;
             skippedScopes = true;
             ++pc;
index 828a31125ae493171b50a74da001f71272c352fe..b914ee10c7100ae31e47c4833d5bae29d93ab05a 100644 (file)
@@ -236,6 +236,13 @@ private:
                     return node.child3().index();
                 break;
             }
+            case SetLocal: {
+                VariableAccessData* variableAccessData = node.variableAccessData();
+                if (variableAccessData->isCaptured()
+                    && variableAccessData->local() == static_cast<VirtualRegister>(varNumber))
+                    return NoNode;
+                break;
+            }
             default:
                 break;
             }
@@ -318,6 +325,14 @@ private:
                     return NoNode;
                 break;
             }
+                
+            case GetLocal: {
+                VariableAccessData* variableAccessData = node.variableAccessData();
+                if (variableAccessData->isCaptured()
+                    && variableAccessData->local() == static_cast<VirtualRegister>(varNumber))
+                    return NoNode;
+                break;
+            }
 
             default:
                 break;
@@ -830,6 +845,11 @@ private:
                 }
                 break;
                 
+            case PutScopedVar:
+                if (static_cast<VirtualRegister>(node.varNumber()) == local)
+                    return NoNode;
+                break;
+                
             default:
                 if (careAboutClobbering && m_graph.clobbersWorld(index))
                     return NoNode;
@@ -882,9 +902,13 @@ private:
                 return result;
             }
                 
+            case GetScopedVar:
+                if (static_cast<VirtualRegister>(node.varNumber()) == local)
+                    result.mayBeAccessed = true;
+                break;
+                
             case GetMyScope:
             case SkipTopScope:
-            case GetScopeRegisters:
                 if (m_graph.uncheckedActivationRegisterFor(node.codeOrigin) == local)
                     result.mayBeAccessed = true;
                 break;
index a89c697f6cbab8ce40bbbe63e1bbc95c83beb13c..501f7c7222efe8ccbf75321a083855b5f7c18285 100644 (file)
@@ -82,6 +82,10 @@ inline bool canInlineResolveOperations(OpcodeID opcode, ResolveOperations* opera
         case ResolveOperation::GetAndReturnGlobalProperty:
         case ResolveOperation::GetAndReturnGlobalVar:
         case ResolveOperation::GetAndReturnGlobalVarWatchable:
+        case ResolveOperation::SkipScopes:
+        case ResolveOperation::SetBaseToScope:
+        case ResolveOperation::ReturnScopeAsBase:
+        case ResolveOperation::GetAndReturnScopedVar:
             continue;
 
         case ResolveOperation::Fail:
@@ -94,12 +98,8 @@ inline bool canInlineResolveOperations(OpcodeID opcode, ResolveOperations* opera
                 return false;
 
         case ResolveOperation::SkipTopScopeNode:
-        case ResolveOperation::SkipScopes:
-        case ResolveOperation::SetBaseToScope:
-        case ResolveOperation::ReturnScopeAsBase:
-        case ResolveOperation::GetAndReturnScopedVar:
-            // These opcodes would be easy to support with inlining, but we currently don't do it.
-            // The issue is that the scope chain will not be set correctly.
+            // We don't inline code blocks that create activations. Creation of
+            // activations is the only thing that leads to SkipTopScopeNode.
             return false;
 
         case ResolveOperation::CheckForDynamicEntriesBeforeGlobalScope: