Bug 135526 - Native inlining in FTL should not require three compiles of the same files
Summary: Native inlining in FTL should not require three compiles of the same files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Matthew Mirman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-01 15:55 PDT by Matthew Mirman
Modified: 2014-08-26 12:59 PDT (History)
6 users (show)

See Also:


Attachments
Merges the two inlining passes (102.48 KB, patch)
2014-08-01 16:57 PDT, Matthew Mirman
no flags Details | Formatted Diff | Diff
Merges the two inlining passes (105.10 KB, patch)
2014-08-18 13:36 PDT, Matthew Mirman
fpizlo: review+
Details | Formatted Diff | Diff
Merges the two inlining passes (104.88 KB, patch)
2014-08-26 12:55 PDT, Matthew Mirman
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Mirman 2014-08-01 15:55:04 PDT
Patch forthcoming
Comment 1 Matthew Mirman 2014-08-01 16:57:59 PDT
Created attachment 235925 [details]
Merges the two inlining passes

Also adds the AvailableExternallyLinkage assertion
Comment 2 Matthew Mirman 2014-08-18 13:36:31 PDT
Created attachment 236781 [details]
Merges the two inlining passes

Now intended for trunk branch.
Comment 3 Oliver Hunt 2014-08-18 13:37:41 PDT
Comment on attachment 236781 [details]
Merges the two inlining passes

Im not incredibly happy about making everything non-static - why is this necessary? Can we not make clang emit ir for static methods?
Comment 4 WebKit Commit Bot 2014-08-18 13:38:54 PDT
Attachment 236781 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/JSDataViewPrototype.cpp:66:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/build-symbol-table-index.py:52:  missing whitespace around operator  [pep8/E225] [5]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Filip Pizlo 2014-08-18 15:27:45 PDT
Comment on attachment 236781 [details]
Merges the two inlining passes

r=me
Comment 6 Matthew Mirman 2014-08-19 10:48:15 PDT
(In reply to comment #3)
> (From update of attachment 236781 [details])
> Im not incredibly happy about making everything non-static - why is this necessary? Can we not make clang emit ir for static methods?

The problem is not that clang can't emit IR, the problem is that the other IR that clang emits makes calls to those static methods, and when we inline it, we need to call out to those methods, which must not be private since they are no longer only being called from the same file.
Comment 7 Matthew Mirman 2014-08-19 12:03:25 PDT
Landed in http://trac.webkit.org/changeset/172756
Comment 8 Csaba Osztrogonác 2014-08-20 14:52:26 PDT
(In reply to comment #7)
> Landed in http://trac.webkit.org/changeset/172756

And EFL buildfix landed in https://trac.webkit.org/changeset/172793.

( Just a note: It wasn't a fairplay game to land the patch
when the EFL noticed the build breakage ahead of time. :-/ )
Comment 9 Matthew Mirman 2014-08-26 12:55:33 PDT
Created attachment 237168 [details]
Merges the two inlining passes
Comment 10 Matthew Mirman 2014-08-26 12:59:05 PDT
Comment on attachment 237168 [details]
Merges the two inlining passes

Erroneous patch.