Bug 118910

Summary:

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

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, ggaren, mark.lam, mhahnenberg, msaboff, oliver, sam

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

118935, 118940

Bug Blocks:

118749

Attachments:

Description	Flags
work in progress	none
work in progress	none
work in progress	none
more	none
clobberizer is done	none
shockingly, it works	none
make the changelog even more epic	none
make the changelog even more lovely	none
cleanups	none
the patch	sam: review+

Filip Pizlo

Reported 2013-07-19 10:57:15 PDT

Patch forthcoming.

Attachments
work in progress (5.60 KB, patch) 2013-07-19 10:57 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (17.44 KB, patch) 2013-07-19 17:09 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (22.17 KB, patch) 2013-07-19 18:52 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
more (31.05 KB, patch) 2013-07-19 21:58 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
clobberizer is done (31.01 KB, patch) 2013-07-19 23:30 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
shockingly, it works (57.50 KB, patch) 2013-07-20 12:27 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
make the changelog even more epic (57.78 KB, patch) 2013-07-20 12:31 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
make the changelog even more lovely (57.73 KB, patch) 2013-07-20 12:35 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
cleanups (58.08 KB, patch) 2013-07-21 15:01 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (58.76 KB, patch) 2013-07-21 15:19 PDT, Filip Pizlo	sam: review+	Details Formatted Diff Diff
Show Obsolete (9) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2013-07-19 10:57:52 PDT

Created attachment 207124 [details] work in progress

Filip Pizlo

Comment 2 2013-07-19 17:09:03 PDT

Created attachment 207159 [details] work in progress

Filip Pizlo

Comment 3 2013-07-19 18:52:56 PDT

Created attachment 207165 [details] work in progress

Filip Pizlo

Comment 4 2013-07-19 21:58:19 PDT

Created attachment 207182 [details] more

Filip Pizlo

Comment 5 2013-07-19 23:30:03 PDT

Created attachment 207187 [details] clobberizer is done

Filip Pizlo

Comment 6 2013-07-20 12:27:03 PDT

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

Comment 7 2013-07-20 12:31:44 PDT

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

Filip Pizlo

Comment 8 2013-07-20 12:35:43 PDT

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

Filip Pizlo

Comment 9 2013-07-21 15:01:53 PDT

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

Comment 10 2013-07-21 15:19:00 PDT

Created attachment 207214 [details] the patch added a ClobberSet::addAll() method

Sam Weinig

Comment 11 2013-07-21 18:11:43 PDT

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

Comment 12 2013-07-21 19:22:53 PDT

(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

Comment 13 2013-07-21 21:03:56 PDT

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

Note You need to log in before you can comment on or make changes to this bug.