Bug 185287 - DFG AI should have O(1) clobbering
Summary: DFG AI should have O(1) clobbering
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-03 19:43 PDT by Filip Pizlo
Modified: 2018-05-07 18:06 PDT (History)
7 users (show)

See Also:


Attachments
work in progress (13.23 KB, patch)
2018-05-03 19:43 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (39.41 KB, patch)
2018-05-04 18:50 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (60.12 KB, patch)
2018-05-04 18:53 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (70.34 KB, patch)
2018-05-05 11:15 PDT, Filip Pizlo
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
the patch (114.78 KB, patch)
2018-05-05 18:08 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (114.76 KB, patch)
2018-05-05 18:09 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (114.71 KB, patch)
2018-05-05 18:25 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (115.35 KB, patch)
2018-05-06 10:29 PDT, Filip Pizlo
saam: 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 2018-05-03 19:43:05 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2018-05-03 19:43:30 PDT
Created attachment 339509 [details]
work in progress
Comment 2 annabell 2018-05-03 21:22:28 PDT
Thanks and keep up the good work! https://www.hotmailissue.com
Comment 3 Filip Pizlo 2018-05-04 18:50:14 PDT
Created attachment 339620 [details]
more
Comment 4 Filip Pizlo 2018-05-04 18:53:49 PDT
Created attachment 339623 [details]
more
Comment 5 Filip Pizlo 2018-05-05 11:15:07 PDT
Created attachment 339650 [details]
more
Comment 6 EWS Watchlist 2018-05-05 11:16:45 PDT
Attachment 339650 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1092:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 EWS Watchlist 2018-05-05 12:28:12 PDT
Comment on attachment 339650 [details]
more

Attachment 339650 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/7578343

New failing tests:
v8-v6/v8-deltablue.js.ftl-eager
v8-v6/v8-deltablue.js.ftl-no-cjit-b3o1
stress/v8-deltablue-strict.js.ftl-eager
microbenchmarks/deltablue-for-of.js.ftl-eager
microbenchmarks/deltablue-for-of.js.ftl-no-cjit-b3o1
Comment 8 Filip Pizlo 2018-05-05 17:51:26 PDT
An improved version of this is starting to pass tests and be a compile time speed-up.
Comment 9 Filip Pizlo 2018-05-05 18:08:11 PDT
Created attachment 339661 [details]
the patch
Comment 10 Filip Pizlo 2018-05-05 18:09:01 PDT
Created attachment 339662 [details]
the patch
Comment 11 Filip Pizlo 2018-05-05 18:25:38 PDT
Created attachment 339663 [details]
the patch
Comment 12 EWS Watchlist 2018-05-05 18:27:45 PDT
Attachment 339663 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1092:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Filip Pizlo 2018-05-06 10:29:12 PDT
Created attachment 339687 [details]
the patch
Comment 14 EWS Watchlist 2018-05-06 10:31:07 PDT
Attachment 339687 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1092:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Saam Barati 2018-05-07 16:00:02 PDT
Comment on attachment 339687 [details]
the patch

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

r=me

> Source/JavaScriptCore/dfg/DFGAbstractValue.h:409
> +    // FIXME: Explain this. (OOPS!!)

oops

> Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h:55
> +    ALWAYS_INLINE AbstractValue& forNode(Edge edge)

Why doesn't this need to fastForward as well?
Comment 16 Filip Pizlo 2018-05-07 17:11:59 PDT
(In reply to Saam Barati from comment #15)
> Comment on attachment 339687 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=339687&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/dfg/DFGAbstractValue.h:409
> > +    // FIXME: Explain this. (OOPS!!)
> 
> oops
> 
> > Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h:55
> > +    ALWAYS_INLINE AbstractValue& forNode(Edge edge)
> 
> Why doesn't this need to fastForward as well?

Because it calls forNode(NodeProjection), and that one already fastForwards:

    ALWAYS_INLINE AbstractValue& forNode(NodeFlowProjection node)
    {
        return fastForward(m_abstractValues.at(node));
    }
    
    ALWAYS_INLINE AbstractValue& forNode(Edge edge)
    {
        return forNode(edge.node());
    }
Comment 17 Saam Barati 2018-05-07 17:42:28 PDT
(In reply to Filip Pizlo from comment #16)
> (In reply to Saam Barati from comment #15)
> > Comment on attachment 339687 [details]
> > the patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=339687&action=review
> > 
> > r=me
> > 
> > > Source/JavaScriptCore/dfg/DFGAbstractValue.h:409
> > > +    // FIXME: Explain this. (OOPS!!)
> > 
> > oops
> > 
> > > Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.h:55
> > > +    ALWAYS_INLINE AbstractValue& forNode(Edge edge)
> > 
> > Why doesn't this need to fastForward as well?
> 
> Because it calls forNode(NodeProjection), and that one already fastForwards:
> 
>     ALWAYS_INLINE AbstractValue& forNode(NodeFlowProjection node)
>     {
>         return fastForward(m_abstractValues.at(node));
>     }
>     
>     ALWAYS_INLINE AbstractValue& forNode(Edge edge)
>     {
>         return forNode(edge.node());
>     }

Makes sense, I wasn't considering this that NodeFlowProjection had an implicit constructor.
Comment 18 Filip Pizlo 2018-05-07 18:05:39 PDT
Landed in https://trac.webkit.org/changeset/231471/webkit
Comment 19 Radar WebKit Bug Importer 2018-05-07 18:06:22 PDT
<rdar://problem/40044661>