Bug 133166

Summary:

[ftlopt] DFG::clobberize should be blind to the effects of GC

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, bunhere, commit-queue, ggaren, gyuyoung.kim, kling, mark.lam, mhahnenberg, msaboff, oliver, rakuco, sam, sergio

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
the patch	ggaren: review+

Description Filip Pizlo 2014-05-21 20:47:33 PDT

Clobberization should only be used during the earlier parts of compilation, when it's fair game to insert allocations anywhere.  This change will allow for hoisting of MakeRope's.

Comment 1 Filip Pizlo 2014-05-21 20:55:46 PDT

Created attachment 231851 [details]
the patch

Comment 2 Geoffrey Garen 2014-05-21 20:58:45 PDT

Comment on attachment 231851 [details]
the patch

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

r=me

> Source/JavaScriptCore/dfg/DFGDoesGC.cpp:37
> +    switch (node->op()) {

Might be nice to list all the ops here explicitly, and remove the default. That would make it impossible to add a new op without considering whether it does GC.

Comment 3 Filip Pizlo 2014-05-21 20:59:47 PDT

(In reply to comment #2)
> (From update of attachment 231851 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231851&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/dfg/DFGDoesGC.cpp:37
> > +    switch (node->op()) {
> 
> Might be nice to list all the ops here explicitly, and remove the default. That would make it impossible to add a new op without considering whether it does GC.

True.  I'll do that.

Comment 4 Filip Pizlo 2014-05-21 21:11:59 PDT

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

Comment 5 Andreas Kling 2014-05-21 21:43:10 PDT

(In reply to comment #0)
> This change will allow for hoisting of MakeRope's.

I love you.