Bug 126040

Summary: Add support for StoreBarrier and friends to the FTL
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 121074    
Attachments:
Description Flags
Patch fpizlo: review+

Description Mark Hahnenberg 2013-12-19 19:08:27 PST
Patch forthcoming.
Comment 1 Mark Hahnenberg 2013-12-20 16:20:54 PST
Created attachment 219808 [details]
Patch
Comment 2 Filip Pizlo 2014-01-02 15:40:08 PST
Comment on attachment 219808 [details]
Patch

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

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:605
> +        DFG_NODE_DO_TO_CHILDREN(m_graph, m_node, speculate);

Might as well just say: speculate(m_node->child1()).  Maybe you also have to pass m_node, I don't remember the signature.

But maybe it would be even better to say:

compileStoreBarrier(lowCell(m_node->child1()));

And then have the #if ENABLE(GGC) inside compileStoreBarrier().  You can rely on lowCell() doing the right thing if the result is unused.

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:612
> +        compileBaseValueStoreBarrier(m_node->child1(), m_node->child2());

Does this helper buy anything?  I think you call it only here.

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:614
> +        DFG_NODE_DO_TO_CHILDREN(m_graph, m_node, speculate);

Might as well just say:

speculate(m_node->child1());
specualte(m_node->child2());

But see above.

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:630
> +        DFG_NODE_DO_TO_CHILDREN(m_graph, m_node, speculate);

See above.

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:4066
> +    void compileStoreBarrier(LValue base, LValue value, Edge& valueEdge)

Rename to emitStoreBarrier.  We use the term "compileBlah" to mean that Blah is a node type that we're lowering.

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:4083
> +    void compileStoreBarrier(LValue base)

Rename to "emitStoreBarrier".

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:4114
> +    void compileBaseValueStoreBarrier(Edge& baseEdge, Edge& valueEdge)

You probably don't need this helper.
Comment 3 Mark Hahnenberg 2014-01-02 16:20:29 PST
Committed r161240: <http://trac.webkit.org/changeset/161240>