Bug 185287 – DFG AI should have O(1) clobbering

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

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 339509 [details]
work in progress

[Filip Pizlo]

Created attachment 339620 [details]
more

[Filip Pizlo]

Created attachment 339623 [details]
more

[Filip Pizlo]

Created attachment 339650 [details]
more

[EWS Watchlist]

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.

[EWS Watchlist]

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

[Filip Pizlo]

An improved version of this is starting to pass tests and be a compile time speed-up.

[Filip Pizlo]

Created attachment 339661 [details]
the patch

[Filip Pizlo]

Created attachment 339662 [details]
the patch

[Filip Pizlo]

Created attachment 339663 [details]
the patch

[EWS Watchlist]

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.

[Filip Pizlo]

Created attachment 339687 [details]
the patch

[EWS Watchlist]

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.

[Saam Barati]

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?

[Filip Pizlo]

(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());
    }

[Saam Barati]

(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.

[Filip Pizlo]

Landed in https://trac.webkit.org/changeset/231471/webkit

[Radar WebKit Bug Importer]

<rdar://problem/40044661>