Bug 118910 – fourthTier: DFG Nodes should be able to abstractly tell you what they read and what they write

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

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 207124 [details]
work in progress

[Filip Pizlo]

Created attachment 207159 [details]
work in progress

[Filip Pizlo]

Created attachment 207165 [details]
work in progress

[Filip Pizlo]

Created attachment 207182 [details]
more

[Filip Pizlo]

Created attachment 207187 [details]
clobberizer is done

[Filip Pizlo]

Created attachment 207199 [details]
shockingly, it works

And by "works" I mean that I can dump some clobbers here and there.  Visual inspection confirms that it's as good as it will be before I write a real client that uses this in anger.

[Filip Pizlo]

Created attachment 207200 [details]
make the changelog even more epic

[Filip Pizlo]

Created attachment 207201 [details]
make the changelog even more lovely

[Filip Pizlo]

Created attachment 207213 [details]
cleanups

overlapsReads is renamed to readsOverlap.  Same for writes.

Added a method called didWrites() that just tells you if any writes occurred.

[Filip Pizlo]

Created attachment 207214 [details]
the patch

added a ClobberSet::addAll() method

[Sam Weinig]

Comment on attachment 207214 [details]
the patch

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

Is there anyway to test all this (is it worth it to unit test it)?  Or will I have to wait for LICM?

> Source/JavaScriptCore/dfg/DFGAbstractHeap.h:275
> +    int64_t m_value;

Even though it is not too confusing, you should probably add a short comment explaining the structure of m_value, bitwise.

> Source/JavaScriptCore/dfg/DFGClobberSet.cpp:106
> +    out.print("(Direct:[", sortedListDump(direct()), "], Super:[", sortedListDump(super()), "])");

Please adding timing information so you can have a function called clobberingTime().

[Filip Pizlo]

(In reply to comment #11)
> (From update of attachment 207214 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207214&action=review
> 
> Is there anyway to test all this (is it worth it to unit test it)?  Or will I have to wait for LICM?

Yeah this will have to wait for LICM and GVN or CSE. 
> 
> > Source/JavaScriptCore/dfg/DFGAbstractHeap.h:275
> > +    int64_t m_value;
> 
> Even though it is not too confusing, you should probably add a short comment explaining the structure of m_value, bitwise.

Will do. 

> 
> > Source/JavaScriptCore/dfg/DFGClobberSet.cpp:106
> > +    out.print("(Direct:[", sortedListDump(direct()), "], Super:[", sortedListDump(super()), "])");
> 
> Please adding timing information so you can have a function called clobberingTime().

I'll consider it.

[Filip Pizlo]

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