Bug 153445 – Switch FTL to B3 on X86_64/Mac

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Filip Pizlo]

Patch forthcoming.  Already rubber stamped by ggaren.

[Filip Pizlo]

Michael, Ossy: I CC'd you guys because this tracks enabling B3 on Mac and disabling LLVM on Mac.  We think that it's stable and fast enough to justify this.

We will continue to work on making B3 even faster after this lands.  It will be easy to make it fast and stable once it's actually enabled.

[Filip Pizlo]

Created attachment 269797 [details]
the patch

[Michael Catanzaro]

Wow, I wasn't expecting to see this so soon!

I think it would be safer to branch for WebKitGTK+ 2.12 before we make this switch, but 2.10 did not depend on LLVM, and it's silly and annoying for distributors for us to introduce a new LLVM dependency in 2.12 that's just going to go away in 2.14 (as I presume we will have enabled B3 by then). So I think we should either (a) enable B3 for 2.12, or (b) disable FTL for 2.12 and enable FTL/B3 for 2.14. I don't want to release 2.12 using LLVM.

Also, once this Mac patch lands, LLVM will be used exclusively for EFL and GTK. I guess if we switch, that code can be removed?

(Note: B3 doesn't build for me, so it can't work, but I don't know how much work it would need once we make it build.)

[Michael Catanzaro]

Comment on attachment 269797 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=269797&action=review

> Source/JavaScriptCore/dfg/DFGCommon.h:40
>  // to 0 before committing!

Don't forget to remove this comment ("Remember to set this to 0 before committing!")

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/195562!

[Filip Pizlo]

(In reply to comment #3)
> Wow, I wasn't expecting to see this so soon!
> 
> I think it would be safer to branch for WebKitGTK+ 2.12 before we make this
> switch, but 2.10 did not depend on LLVM, and it's silly and annoying for
> distributors for us to introduce a new LLVM dependency in 2.12 that's just
> going to go away in 2.14 (as I presume we will have enabled B3 by then). So
> I think we should either (a) enable B3 for 2.12, or (b) disable FTL for 2.12
> and enable FTL/B3 for 2.14. I don't want to release 2.12 using LLVM.
> 
> Also, once this Mac patch lands, LLVM will be used exclusively for EFL and
> GTK. I guess if we switch, that code can be removed?

Don't forget about iOS.  Right now, it's still using LLVM.

> 
> (Note: B3 doesn't build for me, so it can't work, but I don't know how much
> work it would need once we make it build.)

It's probably close.  I'm happy to help. :-)

[Filip Pizlo]

(In reply to comment #4)
> Comment on attachment 269797 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=269797&action=review
> 
> > Source/JavaScriptCore/dfg/DFGCommon.h:40
> >  // to 0 before committing!
> 
> Don't forget to remove this comment ("Remember to set this to 0 before
> committing!")

Ooops, I'll land a follow-up that edits this comment.

[Filip Pizlo]

(In reply to comment #7)
> (In reply to comment #4)
> > Comment on attachment 269797 [details]
> > the patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=269797&action=review
> > 
> > > Source/JavaScriptCore/dfg/DFGCommon.h:40
> > >  // to 0 before committing!
> > 
> > Don't forget to remove this comment ("Remember to set this to 0 before
> > committing!")
> 
> Ooops, I'll land a follow-up that edits this comment.

http://trac.webkit.org/changeset/195563

[Gyuyoung Kim]

(In reply to comment #6)
> (In reply to comment #3)
> > Wow, I wasn't expecting to see this so soon!
> > 
> > I think it would be safer to branch for WebKitGTK+ 2.12 before we make this
> > switch, but 2.10 did not depend on LLVM, and it's silly and annoying for
> > distributors for us to introduce a new LLVM dependency in 2.12 that's just
> > going to go away in 2.14 (as I presume we will have enabled B3 by then). So
> > I think we should either (a) enable B3 for 2.12, or (b) disable FTL for 2.12
> > and enable FTL/B3 for 2.14. I don't want to release 2.12 using LLVM.
> > 
> > Also, once this Mac patch lands, LLVM will be used exclusively for EFL and
> > GTK. I guess if we switch, that code can be removed?
> 
> Don't forget about iOS.  Right now, it's still using LLVM.

Nice job ! Let me check if EFL port also can support B3 soon. Thanks.

[Carlos Garcia Campos]

(In reply to comment #3)
> Wow, I wasn't expecting to see this so soon!
> 
> I think it would be safer to branch for WebKitGTK+ 2.12 before we make this
> switch, but 2.10 did not depend on LLVM, and it's silly and annoying for
> distributors for us to introduce a new LLVM dependency in 2.12 that's just
> going to go away in 2.14 (as I presume we will have enabled B3 by then). So
> I think we should either (a) enable B3 for 2.12, or (b) disable FTL for 2.12
> and enable FTL/B3 for 2.14. I don't want to release 2.12 using LLVM.
> 
> Also, once this Mac patch lands, LLVM will be used exclusively for EFL and
> GTK. I guess if we switch, that code can be removed?
> 
> (Note: B3 doesn't build for me, so it can't work, but I don't know how much
> work it would need once we make it build.)

https://bugs.webkit.org/show_bug.cgi?id=153478

[Csaba Osztrogonác]

Just a question, when do you plan to drop LLVM based FTL support?

B3 based FTL isn't ready on Linux (X86_64) yet. The biggest problem is 
bug153422, but there were crashes (only 5-6) before this regression.

If you plan to remove LLVM based FTL support before anybody can fix Linux
issues, we should disable FTL JIT before you land the removal patch.

[Chris Dumez]

6-7% progression on Speedometer as well :)

[Chris Dumez]

No obvious impact on PLT though.