Bug 79554 – DFG should support activations and nested functions

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 128844 [details]
the patch

[Oliver Hunt]

Comment on attachment 128844 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=128844&action=review

Basically r=me, but it would be nice if you could try to remove the duplicated code :-(

Also we probably want a bug to track to do function instantiation inline like we do in the baseline JIT.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:3540
> +    case CreateActivation: {
> +        JSValueOperand value(this, node.child1());
> +        GPRTemporary result(this, value);
> +        
> +        GPRReg valueTagGPR = value.tagGPR();
> +        GPRReg valuePayloadGPR = value.payloadGPR();
> +        GPRReg resultGPR = result.gpr();
> +        
> +        m_jit.move(valuePayloadGPR, resultGPR);
> +        
> +        JITCompiler::Jump alreadyCreated = m_jit.branch32(JITCompiler::NotEqual, valueTagGPR, TrustedImm32(JSValue::EmptyValueTag));
> +        
> +        silentSpillAllRegisters(resultGPR);
> +        callOperation(operationCreateActivation, resultGPR);
> +        silentFillAllRegisters(resultGPR);
> +        
> +        alreadyCreated.link(&m_jit);
> +        
> +        cellResult(resultGPR, m_compileIndex);
> +        break;
> +    }
> +        
> +    case TearOffActivation: {
> +        JSValueOperand value(this, node.child1());
> +        GPRTemporary result(this, value);
> +        
> +        GPRReg valueTagGPR = value.tagGPR();
> +        GPRReg valuePayloadGPR = value.payloadGPR();
> +        
> +        JITCompiler::Jump notCreated = m_jit.branch32(JITCompiler::Equal, valueTagGPR, TrustedImm32(JSValue::EmptyValueTag));
> +        
> +        silentSpillAllRegisters(InvalidGPRReg);
> +        callOperation(operationTearOffActivation, valuePayloadGPR);
> +        silentFillAllRegisters(InvalidGPRReg);
> +        
> +        notCreated.link(&m_jit);
> +        
> +        noResult(m_compileIndex);
> +        break;
> +    }
> +        
> +    case NewFunctionNoCheck: {
> +        GPRResult result(this);
> +        GPRReg resultGPR = result.gpr();
> +        flushRegisters();
> +        callOperation(
> +            operationNewFunction, resultGPR, m_jit.codeBlock()->functionDecl(node.functionDeclIndex()));
> +        cellResult(resultGPR, m_compileIndex);
> +        break;
> +    }

It feels like these should be entirely sharable between  32bit and 64bit implementations

[Filip Pizlo]

(In reply to comment #2)
> (From update of attachment 128844 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=128844&action=review
> 
> Basically r=me, but it would be nice if you could try to remove the duplicated code :-(
> 
> Also we probably want a bug to track to do function instantiation inline like we do in the baseline JIT.
> 
> > Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:3540
> > +    case CreateActivation: {
> > +        JSValueOperand value(this, node.child1());
> > +        GPRTemporary result(this, value);
> > +        
> > +        GPRReg valueTagGPR = value.tagGPR();
> > +        GPRReg valuePayloadGPR = value.payloadGPR();
> > +        GPRReg resultGPR = result.gpr();
> > +        
> > +        m_jit.move(valuePayloadGPR, resultGPR);
> > +        
> > +        JITCompiler::Jump alreadyCreated = m_jit.branch32(JITCompiler::NotEqual, valueTagGPR, TrustedImm32(JSValue::EmptyValueTag));
> > +        
> > +        silentSpillAllRegisters(resultGPR);
> > +        callOperation(operationCreateActivation, resultGPR);
> > +        silentFillAllRegisters(resultGPR);
> > +        
> > +        alreadyCreated.link(&m_jit);
> > +        
> > +        cellResult(resultGPR, m_compileIndex);
> > +        break;
> > +    }
> > +        
> > +    case TearOffActivation: {
> > +        JSValueOperand value(this, node.child1());
> > +        GPRTemporary result(this, value);
> > +        
> > +        GPRReg valueTagGPR = value.tagGPR();
> > +        GPRReg valuePayloadGPR = value.payloadGPR();
> > +        
> > +        JITCompiler::Jump notCreated = m_jit.branch32(JITCompiler::Equal, valueTagGPR, TrustedImm32(JSValue::EmptyValueTag));
> > +        
> > +        silentSpillAllRegisters(InvalidGPRReg);
> > +        callOperation(operationTearOffActivation, valuePayloadGPR);
> > +        silentFillAllRegisters(InvalidGPRReg);
> > +        
> > +        notCreated.link(&m_jit);
> > +        
> > +        noResult(m_compileIndex);
> > +        break;
> > +    }
> > +        
> > +    case NewFunctionNoCheck: {
> > +        GPRResult result(this);
> > +        GPRReg resultGPR = result.gpr();
> > +        flushRegisters();
> > +        callOperation(
> > +            operationNewFunction, resultGPR, m_jit.codeBlock()->functionDecl(node.functionDeclIndex()));
> > +        cellResult(resultGPR, m_compileIndex);
> > +        break;
> > +    }
> 
> It feels like these should be entirely sharable between  32bit and 64bit implementations

Actually not.  The only parts that are sharable are the really easy ones (NewFunctionNoCheck and NewFunctionExpression).  The others are not sharable because they have different numbers of registers and use different logic for testing empty values.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/108908

[Csaba Osztrogonác]

Reopen, because it made inspector tests crash on Qt:
http://build.webkit.org/results/Qt%20Linux%20Release/r108909%20%2843679%29/results.html

Maybe on GTK too, but GTK doesn't run these tests for some reason.
It might be a 32 bit related bug, not Qt related.

crash log for fast/harness/results.html:

1   0x8066e5b /ramdisk/qt-linux-release/build/WebKitBuild/Release/bin/DumpRenderTree() [0x8066e5b]
2   0xf7723400 [0xf7723400]
3   0xf6db35f7 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(operationTearOffActivation+0x17) [0xf6db35f7]
4   0xef915fc2 [0xef915fc2]
5   0xf6e10811 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x401) [0xf6e10811]
6   0xf6f0bf86 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x66) [0xf6f0bf86]
7   0xf6f0137e /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(+0x1aa137e) [0xf6f0137e]
8   0xefa591a9 [0xefa591a9]
9   0xf6e10811 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x401) [0xf6e10811]
10  0xf6f0bf86 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x66) [0xf6f0bf86]
11  0xf5defa63 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*)+0x453) [0xf5defa63]
12  0xf5fd276d /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1u>&)+0x12d) [0xf5fd276d]
13  0xf5fd292d /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::EventTarget::fireEventListeners(WebCore::Event*)+0x5d) [0xf5fd292d]
14  0xf6348b1a /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::DOMWindow::dispatchEvent(WTF::PassRefPtr<WebCore::Event>, WTF::PassRefPtr<WebCore::EventTarget>)+0xca) [0xf6348b1a]
15  0xf634fb77 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::DOMWindow::dispatchLoadEvent()+0x117) [0xf634fb77]
16  0xf5f895be /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::Document::dispatchWindowLoadEvent()+0x2e) [0xf5f895be]
17  0xf5f930ee /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::Document::implicitClose()+0x12e) [0xf5f930ee]
18  0xf62d0474 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::FrameLoader::checkCallImplicitClose()+0x74) [0xf62d0474]
19  0xf62d9551 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::FrameLoader::checkCompleted()+0xc1) [0xf62d9551]
20  0xf62d96bd /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::FrameLoader::loadDone()+0x1d) [0xf62d96bd]
21  0xf62be82b /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::CachedResourceLoader::loadDone()+0x4b) [0xf62be82b]
22  0xf630e6cf /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::SubresourceLoader::releaseResources()+0x5f) [0xf630e6cf]
23  0xf63044ba /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::ResourceLoader::didFinishLoading(double)+0x3a) [0xf63044ba]
24  0xf630ef91 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::SubresourceLoader::didFinishLoading(double)+0xe1) [0xf630ef91]
25  0xf6303f81 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*, double)+0x71) [0xf6303f81]
26  0xf661b93d /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::QNetworkReplyHandler::finish()+0x1bd) [0xf661b93d]
27  0xf66182a4 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::QNetworkReplyHandlerCallQueue::flush()+0x64) [0xf66182a4]
28  0xf6618844 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::QNetworkReplyHandlerCallQueue::push(void (WebCore::QNetworkReplyHandler::*)())+0x34) [0xf6618844]
29  0xf661888f /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::QNetworkReplyWrapper::didReceiveFinished()+0x3f) [0xf661888f]
30  0xf661afd8 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(+0x11bafd8) [0xf661afd8]
31  0xf3b38af4 /usr/local/Trolltech/Qt-4.8.0/lib/libQtCore.so.4(QMetaObject::activate(QObject*, QMetaObject const*, int, void**)+0x274) [0xf3b38af4]

[Filip Pizlo]

:-(

FYI, I will not have a chance to get to this tonight, but will investigate tomorrow (i.e. around 26 Feb 18:00 GMT or so).

-F



(In reply to comment #5)
> Reopen, because it made inspector tests crash on Qt:
> http://build.webkit.org/results/Qt%20Linux%20Release/r108909%20%2843679%29/results.html
> 
> Maybe on GTK too, but GTK doesn't run these tests for some reason.
> It might be a 32 bit related bug, not Qt related.
> 
> crash log for fast/harness/results.html:
> 
> 1   0x8066e5b /ramdisk/qt-linux-release/build/WebKitBuild/Release/bin/DumpRenderTree() [0x8066e5b]
> 2   0xf7723400 [0xf7723400]
> 3   0xf6db35f7 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(operationTearOffActivation+0x17) [0xf6db35f7]
> 4   0xef915fc2 [0xef915fc2]
> 5   0xf6e10811 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x401) [0xf6e10811]
> 6   0xf6f0bf86 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x66) [0xf6f0bf86]
> 7   0xf6f0137e /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(+0x1aa137e) [0xf6f0137e]
> 8   0xefa591a9 [0xefa591a9]
> 9   0xf6e10811 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x401) [0xf6e10811]
> 10  0xf6f0bf86 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x66) [0xf6f0bf86]
> 11  0xf5defa63 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*)+0x453) [0xf5defa63]
> 12  0xf5fd276d /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1u>&)+0x12d) [0xf5fd276d]
> 13  0xf5fd292d /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::EventTarget::fireEventListeners(WebCore::Event*)+0x5d) [0xf5fd292d]
> 14  0xf6348b1a /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::DOMWindow::dispatchEvent(WTF::PassRefPtr<WebCore::Event>, WTF::PassRefPtr<WebCore::EventTarget>)+0xca) [0xf6348b1a]
> 15  0xf634fb77 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::DOMWindow::dispatchLoadEvent()+0x117) [0xf634fb77]
> 16  0xf5f895be /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::Document::dispatchWindowLoadEvent()+0x2e) [0xf5f895be]
> 17  0xf5f930ee /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::Document::implicitClose()+0x12e) [0xf5f930ee]
> 18  0xf62d0474 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::FrameLoader::checkCallImplicitClose()+0x74) [0xf62d0474]
> 19  0xf62d9551 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::FrameLoader::checkCompleted()+0xc1) [0xf62d9551]
> 20  0xf62d96bd /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::FrameLoader::loadDone()+0x1d) [0xf62d96bd]
> 21  0xf62be82b /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::CachedResourceLoader::loadDone()+0x4b) [0xf62be82b]
> 22  0xf630e6cf /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::SubresourceLoader::releaseResources()+0x5f) [0xf630e6cf]
> 23  0xf63044ba /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::ResourceLoader::didFinishLoading(double)+0x3a) [0xf63044ba]
> 24  0xf630ef91 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::SubresourceLoader::didFinishLoading(double)+0xe1) [0xf630ef91]
> 25  0xf6303f81 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*, double)+0x71) [0xf6303f81]
> 26  0xf661b93d /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::QNetworkReplyHandler::finish()+0x1bd) [0xf661b93d]
> 27  0xf66182a4 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::QNetworkReplyHandlerCallQueue::flush()+0x64) [0xf66182a4]
> 28  0xf6618844 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::QNetworkReplyHandlerCallQueue::push(void (WebCore::QNetworkReplyHandler::*)())+0x34) [0xf6618844]
> 29  0xf661888f /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(WebCore::QNetworkReplyWrapper::didReceiveFinished()+0x3f) [0xf661888f]
> 30  0xf661afd8 /ramdisk/qt-linux-release/build/WebKitBuild/Release/lib/libQtWebKit.so.4(+0x11bafd8) [0xf661afd8]
> 31  0xf3b38af4 /usr/local/Trolltech/Qt-4.8.0/lib/libQtCore.so.4(QMetaObject::activate(QObject*, QMetaObject const*, int, void**)+0x274) [0xf3b38af4]

[Filip Pizlo]

I think I found the problem.  It's 32-bit specific but not Qt-specific.  Testing now.

(In reply to comment #6)
> :-(
> 
> FYI, I will not have a chance to get to this tonight, but will investigate tomorrow (i.e. around 26 Feb 18:00 GMT or so).
> 
> -F
> 
> 
> 
> (In reply to comment #5)
> > Reopen, because it made inspector tests crash on Qt:
> > http://build.webkit.org/results/Qt%20Linux%20Release/r108909%20%2843679%29/results.html
> > 
> > Maybe on GTK too, but GTK doesn't run these tests for some reason.
> > It might be a 32 bit related bug, not Qt related.
> > 
<snip>

[Filip Pizlo]

Created attachment 128928 [details]
the patch to fix 32-bit

[Filip Pizlo]

Comment on attachment 128844 [details]
the patch

Clearing flags since this already landed.

[Filip Pizlo]

Comment on attachment 128928 [details]
the patch to fix 32-bit

Landed in http://trac.webkit.org/changeset/108949

[Filip Pizlo]

Fixed in http://trac.webkit.org/changeset/108949