Bug 170406

Summary: Inst::forEachArg could compile to more compact code
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, jfbastien, keith_miller, mark.lam, msaboff, saam, sam, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch sam: review+

Filip Pizlo
Reported 2017-04-03 10:50:54 PDT
Patch forthcoming.
Attachments
the patch (19.09 KB, patch)
2017-04-03 11:00 PDT, Filip Pizlo 		sam: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2017-04-03 11:00:06 PDT
Created attachment 306084 [details] the patch
JF Bastien
Comment 2 2017-04-03 11:10:41 PDT
Comment on attachment 306084 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=306084&action=review lgtm > Source/JavaScriptCore/ChangeLog:21 > + shrinks by 2.7 MB. This is a 15% reduction in JavaScriptCore binary size. DAMN > Source/JavaScriptCore/b3/air/opcode_generator.rb:-1143 > -# This is a hack for JSAir. It's a joke. No more joke?
Filip Pizlo
Comment 3 2017-04-03 11:32:27 PDT
(In reply to JF Bastien from comment #2) > Comment on attachment 306084 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=306084&action=review > > lgtm > > > Source/JavaScriptCore/ChangeLog:21 > > + shrinks by 2.7 MB. This is a 15% reduction in JavaScriptCore binary size. > > DAMN Yeah! We were ALWAYS_INLINE'ing a ~1600 LoC autogenerated nested switch lambda monster in >20 places. Kudos to all C++ compilers that compiled this code without ICE'ing. > > > Source/JavaScriptCore/b3/air/opcode_generator.rb:-1143 > > -# This is a hack for JSAir. It's a joke. > > No more joke? We needed that code to generate the ARES-6 "Air" benchmark. It's already been generated. So we don't really need to have this code anymore.
Filip Pizlo
Comment 4 2017-04-03 11:54:31 PDT
It's possible, but unlikely, that this patch will show up as a perf regression or progression. We should roll it out if it regresses anything.
Filip Pizlo
Comment 5 2017-04-03 11:55:56 PDT
Landed in https://trac.webkit.org/changeset/214827/webkit
Note You need to log in before you can comment on or make changes to this bug.