Bug 126040 – Add support for StoreBarrier and friends to the FTL

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

[Mark Hahnenberg]

Patch forthcoming.

[Mark Hahnenberg]

Created attachment 219808 [details]
Patch

[Filip Pizlo]

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.

[Mark Hahnenberg]

Committed r161240: <http://trac.webkit.org/changeset/161240>