Bug 79554 - DFG should support activations and nested functions
Summary: DFG should support activations and nested functions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-24 20:30 PST by Filip Pizlo
Modified: 2012-02-26 20:06 PST (History)
5 users (show)

See Also:


Attachments
the patch (28.41 KB, patch)
2012-02-24 20:33 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch to fix 32-bit (2.01 KB, patch)
2012-02-26 14:09 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2012-02-24 20:30:35 PST
Patch forthcoming.
Comment 1 Filip Pizlo 2012-02-24 20:33:53 PST
Created attachment 128844 [details]
the patch
Comment 2 Oliver Hunt 2012-02-24 23:00:53 PST
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
Comment 3 Filip Pizlo 2012-02-25 12:36:20 PST
(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.
Comment 4 Filip Pizlo 2012-02-25 15:06:07 PST
Landed in http://trac.webkit.org/changeset/108908
Comment 5 Csaba Osztrogonác 2012-02-25 22:31:45 PST
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]
Comment 6 Filip Pizlo 2012-02-25 23:38:59 PST
:-(

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]
Comment 7 Filip Pizlo 2012-02-26 13:19:38 PST
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>
Comment 8 Filip Pizlo 2012-02-26 14:09:54 PST
Created attachment 128928 [details]
the patch to fix 32-bit
Comment 9 Filip Pizlo 2012-02-26 17:06:35 PST
Comment on attachment 128844 [details]
the patch

Clearing flags since this already landed.
Comment 10 Filip Pizlo 2012-02-26 20:05:53 PST
Comment on attachment 128928 [details]
the patch to fix 32-bit

Landed in http://trac.webkit.org/changeset/108949
Comment 11 Filip Pizlo 2012-02-26 20:06:08 PST
Fixed in http://trac.webkit.org/changeset/108949