Bug 182959

Summary: DFG::VarargsForwardingPhase should eliminate getting argument length
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
none
WIP
none
patch
keith_miller: review+
patch for landing none

Saam Barati
Reported 2018-02-20 01:10:00 PST
I'm measuring around a 1-3% speedup on ARES's ML test by doing this.
Attachments
WIP (4.08 KB, patch)
2018-02-20 01:21 PST, Saam Barati
no flags
WIP (4.08 KB, patch)
2018-02-20 01:21 PST, Saam Barati
no flags
patch (17.09 KB, patch)
2018-02-20 14:19 PST, Saam Barati
keith_miller: review+
patch for landing (17.09 KB, patch)
2018-02-20 20:27 PST, Saam Barati
no flags
Saam Barati
Comment 1 2018-02-20 01:21:10 PST
Saam Barati
Comment 2 2018-02-20 01:21:47 PST
Saam Barati
Comment 3 2018-02-20 14:19:46 PST
Radar WebKit Bug Importer
Comment 4 2018-02-20 15:04:00 PST
Keith Miller
Comment 5 2018-02-20 20:09:58 PST
Comment on attachment 334302 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=334302&action=review r=me. > Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:202 > + if (node->child1()->op() == GetButterfly > + && candidateButterflies.contains(node->child1().node()) > + && node->child2() == candidate > + && node->storageAccessData().offset == clonedArgumentsLengthPropertyOffset) { > + ASSERT(node->child1()->child1() == candidate); Might be worth a static_assert that clonedArgumentsLengthPropertyOffset is out of line.
Saam Barati
Comment 6 2018-02-20 20:23:24 PST
Comment on attachment 334302 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=334302&action=review >> Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:202 >> + ASSERT(node->child1()->child1() == candidate); > > Might be worth a static_assert that clonedArgumentsLengthPropertyOffset is out of line. I'll add a debug assert. Unfortunately, isOutOfLineOffset isn't constexpr.
Saam Barati
Comment 7 2018-02-20 20:27:52 PST
Created attachment 334338 [details] patch for landing
WebKit Commit Bot
Comment 8 2018-02-20 22:56:16 PST
Comment on attachment 334338 [details] patch for landing Clearing flags on attachment: 334338 Committed r228860: <https://trac.webkit.org/changeset/228860>
WebKit Commit Bot
Comment 9 2018-02-20 22:56:17 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.