Bug 124760 - Restructure global variable constant inference so that it could work for any kind of symbol table variable
Summary: Restructure global variable constant inference so that it could work for any ...
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: 124761
Blocks: 124630
  Show dependency treegraph
 
Reported: 2013-11-21 21:29 PST by Filip Pizlo
Modified: 2013-11-26 18:45 PST (History)
7 users (show)

See Also:


Attachments
work in progress (20.39 KB, patch)
2013-11-23 13:58 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more things (40.92 KB, patch)
2013-11-23 15:15 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the wrong approach (48.32 KB, patch)
2013-11-23 16:26 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (58.86 KB, patch)
2013-11-25 21:27 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (67.82 KB, patch)
2013-11-26 09:40 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (68.06 KB, patch)
2013-11-26 10:56 PST, Filip Pizlo
oliver: review+
Details | Formatted Diff | Diff
patch for landing (68.41 KB, patch)
2013-11-26 17:21 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-11-21 21:29:11 PST
Patch forthcoming.
Comment 1 Filip Pizlo 2013-11-23 13:58:18 PST
Created attachment 217752 [details]
work in progress
Comment 2 Filip Pizlo 2013-11-23 15:15:27 PST
Created attachment 217753 [details]
more things
Comment 3 Oliver Hunt 2013-11-23 15:18:54 PST
Comment on attachment 217753 [details]
more things

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

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:3100
> +                JSValue specificValue =
> +                    watchpointSet ? watchpointSet->inferredValue() : JSValue();
> +                if (!specificValue) {

JSValue specificValue;
if (watchpointSet)
   specificValue = watchPointSet->inferredValue(); (can inferredValue here ever be JSValue()?)
Comment 4 Filip Pizlo 2013-11-23 15:20:39 PST
(In reply to comment #3)
> (From update of attachment 217753 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=217753&action=review
> 
> > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:3100
> > +                JSValue specificValue =
> > +                    watchpointSet ? watchpointSet->inferredValue() : JSValue();
> > +                if (!specificValue) {
> 
> JSValue specificValue;
> if (watchpointSet)
>    specificValue = watchPointSet->inferredValue(); (can inferredValue here ever be JSValue()?)

That would be wrong, since watchpointSet->inferredValue() can be JSValue().
Comment 5 Filip Pizlo 2013-11-23 16:17:19 PST
Actually, this whole approach is wrong.  Consider this:

function foo() {
    var WIDTH;
    function bar() {
        ... use WIDTH ...
    }
    WIDTH = 42;
    ...
    use bar
}

1) Unless we do some static analysis we can't be sure that bar will be called after "WIDTH = 42".

2) op_enter is technically an assignment to WIDTH.
Comment 6 Filip Pizlo 2013-11-23 16:26:48 PST
Created attachment 217755 [details]
the wrong approach

This is the wrong approach but I wanted to set aside the code anyway.
Comment 7 Filip Pizlo 2013-11-25 21:27:06 PST
Actually, I think we totally want this.  It'll work when combined with one-shot closure inference.
Comment 8 Filip Pizlo 2013-11-25 21:27:51 PST
Created attachment 217861 [details]
more
Comment 9 Filip Pizlo 2013-11-26 09:40:07 PST
Created attachment 217883 [details]
more
Comment 10 Filip Pizlo 2013-11-26 10:56:39 PST
Created attachment 217889 [details]
the patch
Comment 11 Filip Pizlo 2013-11-26 17:21:51 PST
Created attachment 217912 [details]
patch for landing
Comment 12 Filip Pizlo 2013-11-26 18:45:26 PST
Landed in http://trac.webkit.org/changeset/159798