WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
153445
Switch FTL to B3 on X86_64/Mac
https://bugs.webkit.org/show_bug.cgi?id=153445
Summary
Switch FTL to B3 on X86_64/Mac
Filip Pizlo
Reported
2016-01-25 13:45:36 PST
Patch forthcoming. Already rubber stamped by ggaren.
Attachments
the patch
(3.47 KB, patch)
2016-01-25 14:56 PST
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Filip Pizlo
Comment 1
2016-01-25 14:50:16 PST
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
Comment 2
2016-01-25 14:56:19 PST
Created
attachment 269797
[details]
the patch
Michael Catanzaro
Comment 3
2016-01-25 15:20:23 PST
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 4
2016-01-25 15:23:02 PST
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
Comment 5
2016-01-25 15:41:25 PST
Landed in
http://trac.webkit.org/changeset/195562
!
Filip Pizlo
Comment 6
2016-01-25 15:42:13 PST
(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
Comment 7
2016-01-25 15:42:30 PST
(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
Comment 8
2016-01-25 15:45:00 PST
(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
Comment 9
2016-01-25 17:28:23 PST
(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
Comment 10
2016-01-25 23:17:39 PST
(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
Comment 11
2016-01-26 04:06:20 PST
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
Comment 12
2016-01-26 09:28:12 PST
6-7% progression on Speedometer as well :)
Chris Dumez
Comment 13
2016-01-26 09:34:39 PST
No obvious impact on PLT though.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug