Bug 79554 - DFG should support activations and nested functions
: DFG should support activations and nested functions
Status: RESOLVED FIXED
: WebKit
JavaScriptCore
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2012-02-24 20:30 PST by
Modified: 2012-02-26 20:06 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-02-24 20:30:35 PST
Patch forthcoming.
------- Comment #1 From 2012-02-24 20:33:53 PST -------
Created an attachment (id=128844) [details]
the patch
------- Comment #2 From 2012-02-24 23:00:53 PST -------
(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
------- Comment #3 From 2012-02-25 12:36:20 PST -------
(In reply to comment #2)
> (From update of attachment 128844 [details] [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 From 2012-02-25 15:06:07 PST -------
Landed in http://trac.webkit.org/changeset/108908
------- Comment #5 From 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 From 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 From 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 From 2012-02-26 14:09:54 PST -------
Created an attachment (id=128928) [details]
the patch to fix 32-bit
------- Comment #9 From 2012-02-26 17:06:35 PST -------
(From update of attachment 128844 [details])
Clearing flags since this already landed.
------- Comment #10 From 2012-02-26 20:05:53 PST -------
(From update of attachment 128928 [details])
Landed in http://trac.webkit.org/changeset/108949
------- Comment #11 From 2012-02-26 20:06:08 PST -------
Fixed in http://trac.webkit.org/changeset/108949