Bug 137242 - DFG SSA should use PutLocal/KillLocal instead of SetLocal to communicate what is flushed to the stack and when
Summary: DFG SSA should use PutLocal/KillLocal instead of SetLocal to communicate what...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 137307 137168
  Show dependency treegraph
 
Reported: 2014-09-29 19:48 PDT by Filip Pizlo
Modified: 2014-10-01 13:12 PDT (History)
7 users (show)

See Also:


Attachments
the patch (20.04 KB, patch)
2014-09-30 20:20 PDT, Filip Pizlo
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2014-09-29 19:48:39 PDT
...
Comment 1 Filip Pizlo 2014-09-29 19:49:47 PDT
Probably the best way to solve this problem is to have DFG SSA use some name other than "SetLocal" for this node.  That way the SSA equivalent of the SetLocal NodeType can have NodeMustGenerate as a default flag.

We could call this new node PutLocal.
Comment 2 Filip Pizlo 2014-09-30 20:20:29 PDT
Created attachment 238988 [details]
the patch
Comment 3 Geoffrey Garen 2014-10-01 11:58:57 PDT
Comment on attachment 238988 [details]
the patch

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

r=me

> Source/JavaScriptCore/ChangeLog:36
> +        - PutLocal means that that a value is to be stored to the stack. It makes a flush available.
> +        - KillLocal means that the value stored to the stack is no longer available for the purposes
> +          of OSR (i.e. it no longer accurately corresponds to what that actual bytecode variable
> +          would have been, so you have to fall back on node availability).

I wonder if we can get the flush logic into the names of these nodes.

FlushLocal
KillFlushLocal

?
Comment 4 Filip Pizlo 2014-10-01 13:07:53 PDT
(In reply to comment #3)
> (From update of attachment 238988 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=238988&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/ChangeLog:36
> > +        - PutLocal means that that a value is to be stored to the stack. It makes a flush available.
> > +        - KillLocal means that the value stored to the stack is no longer available for the purposes
> > +          of OSR (i.e. it no longer accurately corresponds to what that actual bytecode variable
> > +          would have been, so you have to fall back on node availability).
> 
> I wonder if we can get the flush logic into the names of these nodes.
> 
> FlushLocal
> KillFlushLocal
> 
> ?

FlushLocal is probably better than PutLocal

KillFlushLocal is a mouthful!

I'll land this patch as-is but I've filed:
https://bugs.webkit.org/show_bug.cgi?id=137307
Comment 5 Filip Pizlo 2014-10-01 13:12:53 PDT
Landed in http://trac.webkit.org/changeset/174165