Bug 120270

Summary: [GTK] Add support for building JSC with FTL JIT enabled
Product: WebKit Reporter: Zan Dobersek <zan>
Component: New BugsAssignee: Zan Dobersek <zan>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cmarcelo, commit-queue, fpizlo, mrobinson, wingo, xinchao.peng
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch fpizlo: review+

Description Zan Dobersek 2013-08-25 02:39:32 PDT
[GTK] Add support for building JSC with FTL JIT enabled
Comment 1 Zan Dobersek 2013-08-25 02:51:05 PDT
Created attachment 209582 [details]
Patch
Comment 2 Zan Dobersek 2013-08-25 03:35:55 PDT
Ran the SunSpider benchmark to see that Clang+FTL is already pretty much on par with GCC:

GCC: 241.0ms +/- 2.3%
Clang: 261.6ms +/- 1.2%
Clang+FTL: 239.4ms +- 2.0%
Comment 3 Benjamin Poulain 2013-08-26 13:39:45 PDT
FTL is still in the research phase. I don't know if it should be enabled for other ports yet.
Filip knows what is up.
Comment 4 Filip Pizlo 2013-08-26 13:41:56 PDT
(In reply to comment #2)
> Ran the SunSpider benchmark to see that Clang+FTL is already pretty much on par with GCC:
> 
> GCC: 241.0ms +/- 2.3%
> Clang: 261.6ms +/- 1.2%
> Clang+FTL: 239.4ms +- 2.0%

I don't understand these numbers.  Is the FTL runtime enabled as well, with --useExperimentalFTL=true?

Why are you comparing GCC and Clang?  What's up with that?
Comment 5 Filip Pizlo 2013-08-26 13:42:12 PDT
(In reply to comment #3)
> FTL is still in the research phase. I don't know if it should be enabled for other ports yet.
> Filip knows what is up.

Correct.

It's not production-ready.  Web pages will not load if you enable it.
Comment 6 Filip Pizlo 2013-08-26 13:43:21 PDT
At this point, requiring us to keep GTK+ building while we develop the FTL will only slow down development and will almost certainly not buy GTK anything.

So, unless you plan to become an active contributor to the development of the FTL compiler - in which case you should coordinate closely with me, I don't recommend doing this.
Comment 7 Zan Dobersek 2013-08-27 04:05:40 PDT
(In reply to comment #4)
> (In reply to comment #2)
> > Ran the SunSpider benchmark to see that Clang+FTL is already pretty much on par with GCC:
> > 
> > GCC: 241.0ms +/- 2.3%
> > Clang: 261.6ms +/- 1.2%
> > Clang+FTL: 239.4ms +- 2.0%
> 
> I don't understand these numbers.  Is the FTL runtime enabled as well, with --useExperimentalFTL=true?
> 
> Why are you comparing GCC and Clang?  What's up with that?

I wasn't descriptive enough - the table shows performance of jsc when running the SunSpider benchmark, with JavaScriptCore built by different compilers and setups. So, the table shows mean times for the total suite when JSC was compiled with GCC, compiled with Clang, and compiled with Clang with FTL enabled at build-time.

I of course didn't bother to check for any flags that could enable FTL at run-time, so the last entry doesn't really portray what I though it does since the suite fails to run when FTL is enabled through --useExperimentalFTL.
Comment 8 Zan Dobersek 2013-08-27 04:20:41 PDT
(In reply to comment #6)
> At this point, requiring us to keep GTK+ building while we develop the FTL will only slow down development and will almost certainly not buy GTK anything.
> 
> So, unless you plan to become an active contributor to the development of the FTL compiler - in which case you should coordinate closely with me, I don't recommend doing this.

The proposed patch adds the required files to the build and makes building with FTL enabled possible, but it doesn't enable it by default - you wouldn't have to worry about the GTK+ build as we won't enable the feature until you give a thumbs-up that it's ready for prime time.

Until then, people with interest could still build JSC with FTL enabled for the GTK port if they would like to experiment and research, and I can volunteer to take care of the build.
Comment 9 Filip Pizlo 2013-08-27 11:52:35 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > At this point, requiring us to keep GTK+ building while we develop the FTL will only slow down development and will almost certainly not buy GTK anything.
> > 
> > So, unless you plan to become an active contributor to the development of the FTL compiler - in which case you should coordinate closely with me, I don't recommend doing this.
> 
> The proposed patch adds the required files to the build and makes building with FTL enabled possible, but it doesn't enable it by default - you wouldn't have to worry about the GTK+ build as we won't enable the feature until you give a thumbs-up that it's ready for prime time.
> 
> Until then, people with interest could still build JSC with FTL enabled for the GTK port if they would like to experiment and research, and I can volunteer to take care of the build.

Are you saying that you're OK with the GTK+ build being broken by everyone due to FTL work?
Comment 10 Zan Dobersek 2013-08-27 12:09:54 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #6)
> > > At this point, requiring us to keep GTK+ building while we develop the FTL will only slow down development and will almost certainly not buy GTK anything.
> > > 
> > > So, unless you plan to become an active contributor to the development of the FTL compiler - in which case you should coordinate closely with me, I don't recommend doing this.
> > 
> > The proposed patch adds the required files to the build and makes building with FTL enabled possible, but it doesn't enable it by default - you wouldn't have to worry about the GTK+ build as we won't enable the feature until you give a thumbs-up that it's ready for prime time.
> > 
> > Until then, people with interest could still build JSC with FTL enabled for the GTK port if they would like to experiment and research, and I can volunteer to take care of the build.
> 
> Are you saying that you're OK with the GTK+ build being broken by everyone due to FTL work?

Yes, if we're talking about the GTK+ build with FTL enabled while the feature is still under work - I'm not planning to enable FTL on the buildbots (i.e. when using build-webkit) or anywhere else until it matures.

I would only like to have the proper configuration options in place that would enable me or somebody else to build JSC for the GTK+ port with the feature enabled, for whatever reasons. You wouldn't be obliged to add new FTL-specific files to the GTK+ build system etc., that task would fall onto the individual who would like to test out the current status of the feature under the GTK port but gets stuck on a broken build.
Comment 11 Filip Pizlo 2013-08-27 13:25:43 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (In reply to comment #6)
> > > > At this point, requiring us to keep GTK+ building while we develop the FTL will only slow down development and will almost certainly not buy GTK anything.
> > > > 
> > > > So, unless you plan to become an active contributor to the development of the FTL compiler - in which case you should coordinate closely with me, I don't recommend doing this.
> > > 
> > > The proposed patch adds the required files to the build and makes building with FTL enabled possible, but it doesn't enable it by default - you wouldn't have to worry about the GTK+ build as we won't enable the feature until you give a thumbs-up that it's ready for prime time.
> > > 
> > > Until then, people with interest could still build JSC with FTL enabled for the GTK port if they would like to experiment and research, and I can volunteer to take care of the build.
> > 
> > Are you saying that you're OK with the GTK+ build being broken by everyone due to FTL work?
> 
> Yes, if we're talking about the GTK+ build with FTL enabled while the feature is still under work - I'm not planning to enable FTL on the buildbots (i.e. when using build-webkit) or anywhere else until it matures.
> 
> I would only like to have the proper configuration options in place that would enable me or somebody else to build JSC for the GTK+ port with the feature enabled, for whatever reasons. You wouldn't be obliged to add new FTL-specific files to the GTK+ build system etc., that task would fall onto the individual who would like to test out the current status of the feature under the GTK port but gets stuck on a broken build.

OK - got it.  I just now noticed that your patch makes all of this autoconf-conditional.
Comment 12 Zan Dobersek 2013-08-28 07:51:42 PDT
Committed r154747: <http://trac.webkit.org/changeset/154747>
Comment 13 Zan Dobersek 2013-08-28 07:54:08 PDT
Thanks for approving.

I've added an additional 'experimental' label to the configuration flag description to deter people from unnecessary building with the feature enabled.
Comment 14 Martin Robinson 2013-08-28 09:49:22 PDT
Comment on attachment 209582 [details]
Patch

I'm not sure we want to add a configuration option for an incomplete feature, but feel free to add a hidden option.
Comment 15 Peng Xinchao 2013-09-01 19:39:20 PDT
Excuse me , Why do nobody implement 32bit platform FTL ? I donot clear FTL ,But i think that implement 32bit platform  should not have big truble.
Comment 16 Filip Pizlo 2013-09-01 19:58:07 PDT
(In reply to comment #14)
> (From update of attachment 209582 [details])
> I'm not sure we want to add a configuration option for an incomplete feature, but feel free to add a hidden option.

Yeah, that was the crux of my initial objection.  But I buy that letting Linux users play with this beast is a good idea.

It's noteworthy that FTL has sort of two levels of "enabling":

- Compile-time enable.  This just builds the FTL.

- Run-time enable.  This requires a separate environment variable to be set, or a command-line option in the case of the jsc shell.

This patch only did the former.  I plan on compile-time enabling the FTL on Mac in the near future; we're almost totally set up for it.  I think it might start to be a good idea to compile-time enable it on GTK not long after that.  All that this will do is keep us honest while writing code - it'll no longer be excusable to check in things that break the FTL build - something that happens a fair bit right now. :-/

As for enabling it at run-time, well, we still have a lot more work to do.
Comment 17 Filip Pizlo 2013-09-01 20:00:58 PDT
(In reply to comment #15)
> Excuse me , Why do nobody implement 32bit platform FTL ? I donot clear FTL ,But i think that implement 32bit platform  should not have big truble.

JSC uses a completely different value representation on 32-bit.  All of our execution engines have more than half of their code duplicated for both 32-bit and 64-bit.  32-bit support is dramatically more complicated to support in a compiler because of the need to sometimes use two registers to hold a value.  I don't believe it would be easy to make the FTL support both.  Doing so would add a tremendous amount of complexity.

Therefore, I don't think we want people adding 32-bit support to the FTL at this time.  The FTL is not finished yet.  It wouldn't be constructive to start adding support for a second value representation while the FTL still cannot run most JS code.
Comment 18 Filip Pizlo 2013-09-01 20:05:12 PDT
(In reply to comment #16)
> (In reply to comment #14)
> > (From update of attachment 209582 [details] [details])
> > I'm not sure we want to add a configuration option for an incomplete feature, but feel free to add a hidden option.
> 
> Yeah, that was the crux of my initial objection.  But I buy that letting Linux users play with this beast is a good idea.
> 
> It's noteworthy that FTL has sort of two levels of "enabling":
> 
> - Compile-time enable.  This just builds the FTL.
> 
> - Run-time enable.  This requires a separate environment variable to be set, or a command-line option in the case of the jsc shell.
> 
> This patch only did the former.  I plan on compile-time enabling the FTL on Mac in the near future; we're almost totally set up for it.  I think it might start to be a good idea to compile-time enable it on GTK not long after that.  All that this will do is keep us honest while writing code - it'll no longer be excusable to check in things that break the FTL build - something that happens a fair bit right now. :-/

That actually reminds me - do you guys have any thoughts on how you plan to integrate with LLVM?

Our goal is to stick to using llvm.org trunk for now.  But we do rely on things that aren't yet in any official LLVM release, and that will probably continue to be the case for a while - for example I sometimes commit new API to LLVM and then commit code to WebKit that uses that API shortly thereafter.

On Mac, we're making this simple for the average Mac WebKit contributor by checking in binary drops into WebKitLibraries.  These binary drops are nothing more than builds of llvm.org's svn at some revision on some OS; that revision is noted in WebKitLibraries/ChangeLog.  Not sure how GTK would handle it.  I'm also not sure if it would be sane to have each port checking in its own LLVM builds into WebKitLibraries. :-/
Comment 19 Martin Robinson 2013-09-01 20:23:16 PDT
(In reply to comment #18)

> That actually reminds me - do you guys have any thoughts on how you plan to integrate with LLVM?
> 
> Our goal is to stick to using llvm.org trunk for now.  But we do rely on things that aren't yet in any official LLVM release, and that will probably continue to be the case for a while - for example I sometimes commit new API to LLVM and then commit code to WebKit that uses that API shortly thereafter.
> 
> On Mac, we're making this simple for the average Mac WebKit contributor by checking in binary drops into WebKitLibraries.  These binary drops are nothing more than builds of llvm.org's svn at some revision on some OS; that revision is noted in WebKitLibraries/ChangeLog.  Not sure how GTK would handle it.  I'm also not sure if it would be sane to have each port checking in its own LLVM builds into WebKitLibraries. :-/

I'm not really sure about developers trying pre-release WebKit, but I have some thoughts on releases.

We cannot really rely on shipping LLVM binaries since there is such a diverse variety of downstream distributions. I think that we will have to do one of two things. The first is that when building on a system with an old version of LLVM, we could disable the FTL. The second is that we can try to ship a newer version of the LLVM source as part of our source code release. This second approach seems pretty unlikely. Depending on the schedule of LLVM release / distribution packages, it may be difficult for downstream to use the FTL until it no longer relies on bleeding edge LLVM API.
Comment 20 Zan Dobersek 2013-09-05 12:38:29 PDT
For building with FTL JIT I'm using the nightly LLVM builds.
http://llvm.org/apt/

I also don't think shipping LLVM source inside WebKit would make sense, so I guess that for the GTK port we'll have to wait for a LLVM release that stabilizes the API used by FTL JIT. When that happens we'll be able to switch to automatically enabling the FTL JIT feature at configure-time if the LLVM dependency is satisfied, and disabling it otherwise.