FTL should support Call/Construct in the worst way possible
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Sep 2013 05:47:57 +0000 (05:47 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Sep 2013 05:47:57 +0000 (05:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=120916

Reviewed by Oliver Hunt.

This adds support for Call/Construct by just calling out to C code that uses
the JSC::call/JSC::construct runtime functions for making calls. This is slow
and terrible, but it dramatically extends FTL coverage.

Supporting calls in a meaningful way meant also supporting
GlobalVarWatchpoint.

The extension of coverage helped to find a bunch of bugs:

- ObjectOrOtherUse was claimed to be supported in the FTL but speculate()
  didn't support it. That means that any node with an ObjectOrOtherUse edge
  that got DCE'd would cause the FTL to ICE.

- There was a bad fall-through compileCompareStrictEq() that led to ICE.

- The OSR exit reconstruction code was assuming it could do fast checks on
  node->child1() before even determining the type of node; that crashes if
  the node is HasVarArgs. Fixed by checking HasVarArgs first.

- The OSR exit compiler was using the wrong peekOffset for CArgumentGetter.
  The default is 1, which assumes that you didn't push anything onto the
  stack after getting called. The OSR exit thunks push FP, so the offset
  should be 2.

This passes stress tests and is probably huge performance regression if you
--useExperimentalFTL=true. The regression will be fixed in
https://bugs.webkit.org/show_bug.cgi?id=113621.

* dfg/DFGOperations.cpp:
* dfg/DFGOperations.h:
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLIntrinsicRepository.h:
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileNode):
(JSC::FTL::LowerDFGToLLVM::compileGlobalVarWatchpoint):
(JSC::FTL::LowerDFGToLLVM::compileCompareStrictEq):
(JSC::FTL::LowerDFGToLLVM::compileCallOrConstruct):
(JSC::FTL::LowerDFGToLLVM::speculate):
(JSC::FTL::LowerDFGToLLVM::speculateObjectOrOther):
(JSC::FTL::LowerDFGToLLVM::addExitArgumentForNode):
* ftl/FTLOSRExitCompiler.cpp:
(JSC::FTL::compileStub):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGOperations.cpp
Source/JavaScriptCore/dfg/DFGOperations.h
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLIntrinsicRepository.h
Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp

index 2126c97..f99799f 100644 (file)
@@ -1,5 +1,56 @@
 2013-09-06  Filip Pizlo  <fpizlo@apple.com>
 
+        FTL should support Call/Construct in the worst way possible
+        https://bugs.webkit.org/show_bug.cgi?id=120916
+
+        Reviewed by Oliver Hunt.
+        
+        This adds support for Call/Construct by just calling out to C code that uses
+        the JSC::call/JSC::construct runtime functions for making calls. This is slow
+        and terrible, but it dramatically extends FTL coverage.
+        
+        Supporting calls in a meaningful way meant also supporting
+        GlobalVarWatchpoint.
+        
+        The extension of coverage helped to find a bunch of bugs:
+        
+        - ObjectOrOtherUse was claimed to be supported in the FTL but speculate()
+          didn't support it. That means that any node with an ObjectOrOtherUse edge
+          that got DCE'd would cause the FTL to ICE.
+        
+        - There was a bad fall-through compileCompareStrictEq() that led to ICE.
+        
+        - The OSR exit reconstruction code was assuming it could do fast checks on
+          node->child1() before even determining the type of node; that crashes if
+          the node is HasVarArgs. Fixed by checking HasVarArgs first.
+        
+        - The OSR exit compiler was using the wrong peekOffset for CArgumentGetter.
+          The default is 1, which assumes that you didn't push anything onto the
+          stack after getting called. The OSR exit thunks push FP, so the offset
+          should be 2.
+        
+        This passes stress tests and is probably huge performance regression if you
+        --useExperimentalFTL=true. The regression will be fixed in
+        https://bugs.webkit.org/show_bug.cgi?id=113621.
+
+        * dfg/DFGOperations.cpp:
+        * dfg/DFGOperations.h:
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLIntrinsicRepository.h:
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::LowerDFGToLLVM::compileNode):
+        (JSC::FTL::LowerDFGToLLVM::compileGlobalVarWatchpoint):
+        (JSC::FTL::LowerDFGToLLVM::compileCompareStrictEq):
+        (JSC::FTL::LowerDFGToLLVM::compileCallOrConstruct):
+        (JSC::FTL::LowerDFGToLLVM::speculate):
+        (JSC::FTL::LowerDFGToLLVM::speculateObjectOrOther):
+        (JSC::FTL::LowerDFGToLLVM::addExitArgumentForNode):
+        * ftl/FTLOSRExitCompiler.cpp:
+        (JSC::FTL::compileStub):
+
+2013-09-06  Filip Pizlo  <fpizlo@apple.com>
+
         jsc shell should destroy VM as a workaround for LLVM's exit-time destructors
         https://bugs.webkit.org/show_bug.cgi?id=120921
 
index 7a3b0bd..80bcf55 100644 (file)
@@ -2200,6 +2200,46 @@ char* DFG_OPERATION triggerOSREntryNow(
         jitCode->optimizeAfterWarmUp(codeBlock);
     return static_cast<char*>(address);
 }
+
+// FIXME: Make calls work well. Currently they're a pure regression.
+// https://bugs.webkit.org/show_bug.cgi?id=113621
+EncodedJSValue DFG_OPERATION operationFTLCall(ExecState* exec)
+{
+    ExecState* callerExec = exec->callerFrame();
+    
+    VM* vm = &callerExec->vm();
+    NativeCallFrameTracer tracer(vm, callerExec);
+    
+    JSValue callee = exec->calleeAsValue();
+    CallData callData;
+    CallType callType = getCallData(callee, callData);
+    if (callType == CallTypeNone) {
+        vm->throwException(callerExec, createNotAFunctionError(callerExec, callee));
+        return JSValue::encode(jsUndefined());
+    }
+    
+    return JSValue::encode(call(callerExec, callee, callType, callData, exec->thisValue(), exec));
+}
+
+// FIXME: Make calls work well. Currently they're a pure regression.
+// https://bugs.webkit.org/show_bug.cgi?id=113621
+EncodedJSValue DFG_OPERATION operationFTLConstruct(ExecState* exec)
+{
+    ExecState* callerExec = exec->callerFrame();
+    
+    VM* vm = &callerExec->vm();
+    NativeCallFrameTracer tracer(vm, callerExec);
+    
+    JSValue callee = exec->calleeAsValue();
+    ConstructData constructData;
+    ConstructType constructType = getConstructData(callee, constructData);
+    if (constructType == ConstructTypeNone) {
+        vm->throwException(callerExec, createNotAFunctionError(callerExec, callee));
+        return JSValue::encode(jsUndefined());
+    }
+    
+    return JSValue::encode(construct(callerExec, callee, constructType, constructData, exec));
+}
 #endif // ENABLE(FTL_JIT)
 
 } // extern "C"
index 6c0171b..8866864 100644 (file)
@@ -254,6 +254,13 @@ JSCell* DFG_OPERATION operationMakeRope3(ExecState*, JSString*, JSString*, JSStr
 char* DFG_OPERATION operationFindSwitchImmTargetForDouble(ExecState*, EncodedJSValue, size_t tableIndex);
 char* DFG_OPERATION operationSwitchString(ExecState*, size_t tableIndex, JSString*);
 
+#if ENABLE(FTL_JIT)
+// FIXME: Make calls work well. Currently they're a pure regression.
+// https://bugs.webkit.org/show_bug.cgi?id=113621
+EncodedJSValue DFG_OPERATION operationFTLCall(ExecState*) WTF_INTERNAL;
+EncodedJSValue DFG_OPERATION operationFTLConstruct(ExecState*) WTF_INTERNAL;
+#endif // ENABLE(FTL_JIT)
+
 // This method is used to lookup an exception hander, keyed by faultLocation, which is
 // the return location from one of the calls out to one of the helper operations above.
 
index b81f370..2b45e45 100644 (file)
@@ -86,6 +86,9 @@ inline CapabilityLevel canCompile(Node* node)
     case Upsilon:
     case ExtractOSREntryLocal:
     case LoopHint:
+    case Call:
+    case Construct:
+    case GlobalVarWatchpoint:
         // These are OK.
         break;
     case GetArrayLength:
index cc99746..c25dd19 100644 (file)
@@ -46,6 +46,7 @@ namespace JSC { namespace FTL {
 
 #define FOR_EACH_FUNCTION_TYPE(macro) \
     macro(I_DFGOperation_EJss, functionType(intPtr, intPtr, intPtr)) \
+    macro(J_DFGOperation_E, functionType(int64, intPtr)) \
     macro(P_DFGOperation_EC, functionType(intPtr, intPtr, intPtr)) \
     macro(V_DFGOperation_EOZD, functionType(voidType, intPtr, intPtr, int32, doubleType)) \
     macro(V_DFGOperation_EOZJ, functionType(voidType, intPtr, intPtr, int32, int64))
index 7398764..cafa57c 100644 (file)
@@ -363,6 +363,9 @@ private:
         case PutGlobalVar:
             compilePutGlobalVar();
             break;
+        case GlobalVarWatchpoint:
+            compileGlobalVarWatchpoint();
+            break;
         case CompareEq:
             compileCompareEq();
             break;
@@ -390,6 +393,10 @@ private:
         case LogicalNot:
             compileLogicalNot();
             break;
+        case Call:
+        case Construct:
+            compileCallOrConstruct();
+            break;
         case Jump:
             compileJump();
             break;
@@ -1396,6 +1403,12 @@ private:
             lowJSValue(m_node->child1()), m_out.absolute(m_node->registerPointer()));
     }
     
+    void compileGlobalVarWatchpoint()
+    {
+        // FIXME: Implement watchpoints.
+        // https://bugs.webkit.org/show_bug.cgi?id=113647
+    }
+    
     void compileCompareEq()
     {
         if (m_node->isBinaryUseKind(Int32Use)
@@ -1428,6 +1441,7 @@ private:
         if (m_node->isBinaryUseKind(NumberUse)) {
             setBoolean(
                 m_out.doubleEqual(lowDouble(m_node->child1()), lowDouble(m_node->child2())));
+            return;
         }
         
         if (m_node->isBinaryUseKind(ObjectUse)) {
@@ -1541,6 +1555,39 @@ private:
         setBoolean(m_out.bitNot(boolify(m_node->child1())));
     }
     
+    void compileCallOrConstruct()
+    {
+        // FIXME: This is unacceptably slow.
+        // https://bugs.webkit.org/show_bug.cgi?id=113621
+        
+        J_DFGOperation_E function =
+            m_node->op() == Call ? operationFTLCall : operationFTLConstruct;
+        
+        int dummyThisArgument = m_node->op() == Call ? 0 : 1;
+        
+        int numPassedArgs = m_node->numChildren() - 1;
+        
+        LValue calleeFrame = m_out.add(
+            m_callFrame,
+            m_out.constIntPtr(sizeof(Register) * codeBlock()->m_numCalleeRegisters));
+        
+        m_out.store32(
+            m_out.constInt32(numPassedArgs + dummyThisArgument),
+            payloadFor(calleeFrame, JSStack::ArgumentCount));
+        m_out.store64(m_callFrame, addressFor(calleeFrame, JSStack::CallerFrame));
+        m_out.store64(
+            lowJSValue(m_graph.varArgChild(m_node, 0)),
+            addressFor(calleeFrame, JSStack::Callee));
+        
+        for (int i = 0; i < numPassedArgs; ++i) {
+            m_out.store64(
+                lowJSValue(m_graph.varArgChild(m_node, 1 + i)),
+                addressFor(calleeFrame, argumentToOperand(i + dummyThisArgument)));
+        }
+        
+        setJSValue(vmCall(m_out.operation(function), calleeFrame));
+    }
+    
     void compileJump()
     {
         m_out.jump(lowBlock(m_node->takenBlock()));
@@ -2212,6 +2259,9 @@ private:
         case ObjectUse:
             speculateObject(edge);
             break;
+        case ObjectOrOtherUse:
+            speculateObjectOrOther(edge);
+            break;
         case StringUse:
             speculateString(edge);
             break;
@@ -2225,6 +2275,7 @@ private:
             speculateBoolean(edge);
             break;
         default:
+            dataLog("Unsupported speculation use kind: ", edge.useKind(), "\n");
             RELEASE_ASSERT_NOT_REACHED();
         }
     }
@@ -2278,6 +2329,42 @@ private:
         speculateObject(edge, lowCell(edge));
     }
     
+    void speculateObjectOrOther(Edge edge)
+    {
+        if (!m_interpreter.needsTypeCheck(edge))
+            return;
+        
+        LValue value = lowJSValue(edge);
+        
+        LBasicBlock cellCase = FTL_NEW_BLOCK(m_out, ("speculateObjectOrOther cell case"));
+        LBasicBlock primitiveCase = FTL_NEW_BLOCK(m_out, ("speculateObjectOrOther primitive case"));
+        LBasicBlock continuation = FTL_NEW_BLOCK(m_out, ("speculateObjectOrOther continuation"));
+        
+        m_out.branch(isNotCell(value), primitiveCase, cellCase);
+        
+        LBasicBlock lastNext = m_out.appendTo(cellCase, primitiveCase);
+        
+        FTL_TYPE_CHECK(
+            jsValueValue(value), edge, (~SpecCell) | SpecObject,
+            m_out.equal(
+                m_out.loadPtr(value, m_heaps.JSCell_structure),
+                m_out.constIntPtr(vm().stringStructure.get())));
+        
+        m_out.jump(continuation);
+        
+        m_out.appendTo(primitiveCase, continuation);
+        
+        FTL_TYPE_CHECK(
+            jsValueValue(value), edge, SpecCell | SpecOther,
+            m_out.notEqual(
+                m_out.bitAnd(value, m_out.constInt64(~TagBitUndefined)),
+                m_out.constInt64(ValueNull)));
+        
+        m_out.jump(continuation);
+        
+        m_out.appendTo(continuation, lastNext);
+    }
+    
     void speculateString(Edge edge, LValue cell)
     {
         FTL_TYPE_CHECK(jsValueValue(cell), edge, SpecString, isNotString(cell));
@@ -2634,6 +2721,8 @@ private:
                 HashSet<Node*>::iterator end = m_live.end();
                 for (; iter != end; ++iter) {
                     Node* candidate = *iter;
+                    if (candidate->flags() & NodeHasVarArgs)
+                        continue;
                     if (!candidate->child1())
                         continue;
                     if (candidate->child1() != node)
index fb43635..74530a0 100644 (file)
@@ -56,7 +56,7 @@ static void compileStub(
     // of: we know that it's two frames beneath us. This is terrible and I feel
     // ashamed of it, but it will work for now.
     
-    CArgumentGetter arguments(jit);
+    CArgumentGetter arguments(jit, 2);
     
     // First recover our call frame and tag thingies.
     arguments.loadNextPtr(GPRInfo::callFrameRegister);