Bug 170406 - Inst::forEachArg could compile to more compact code
Summary: Inst::forEachArg could compile to more compact code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-03 10:50 PDT by Filip Pizlo
Modified: 2017-04-03 11:55 PDT (History)
8 users (show)

See Also:


Attachments
the patch (19.09 KB, patch)
2017-04-03 11:00 PDT, Filip Pizlo
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2017-04-03 10:50:54 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2017-04-03 11:00:06 PDT
Created attachment 306084 [details]
the patch
Comment 2 JF Bastien 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?
Comment 3 Filip Pizlo 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.
Comment 4 Filip Pizlo 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.
Comment 5 Filip Pizlo 2017-04-03 11:55:56 PDT
Landed in https://trac.webkit.org/changeset/214827/webkit