153445 2016-01-25 13:45:36 -0800 Switch FTL to B3 on X86_64/Mac 2016-02-01 03:10:45 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build All All RESOLVED FIXED P2 Normal --- 153739 150279 1 fpizlo fpizlo barraclough benjamin cdumez cgarcia ggaren gyuyoung.kim keith_miller mark.lam mcatanzaro mhahnenb msaboff nrotem oliver ossy saam sam oldest_to_newest 1158481 0 fpizlo 2016-01-25 13:45:36 -0800 Patch forthcoming. Already rubber stamped by ggaren. 1158519 1 fpizlo 2016-01-25 14:50:16 -0800 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. 1158527 2 269797 fpizlo 2016-01-25 14:56:19 -0800 Created attachment 269797 the patch 1158540 3 mcatanzaro 2016-01-25 15:20:23 -0800 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.) 1158547 4 269797 mcatanzaro 2016-01-25 15:23:02 -0800 Comment on attachment 269797 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!") 1158566 5 fpizlo 2016-01-25 15:41:25 -0800 Landed in http://trac.webkit.org/changeset/195562! 1158567 6 fpizlo 2016-01-25 15:42:13 -0800 (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. :-) 1158568 7 fpizlo 2016-01-25 15:42:30 -0800 (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. 1158570 8 fpizlo 2016-01-25 15:45:00 -0800 (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 1158626 9 gyuyoung.kim 2016-01-25 17:28:23 -0800 (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. 1158727 10 cgarcia 2016-01-25 23:17:39 -0800 (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 1158787 11 ossy 2016-01-26 04:06:20 -0800 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. 1158840 12 cdumez 2016-01-26 09:28:12 -0800 6-7% progression on Speedometer as well :) 1158842 13 cdumez 2016-01-26 09:34:39 -0800 No obvious impact on PLT though. 269797 2016-01-25 14:56:19 -0800 2016-01-25 14:56:19 -0800 the patch blah.patch text/plain 3556 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTk1NTU1KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDQyIEBA CisyMDE2LTAxLTI1ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg U3dpdGNoIEZUTCB0byBCMyBvbiBYODZfNjQvTWFjCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTM0NDUKKworICAgICAgICBSdWJiZXIgc3RhbXBlZCBi eSBHZW9mZnJleSBHYXJlbi4KKworICAgICAgICBUaGlzIGZpbmFsbHkgc3dpdGNoZXMgZnJvbSBM TFZNIHRvIEIzIGluIHRoZSBGVEwgb24gWDg2XzY0IG9uIHRoZSBNYWMuIFdlIHJlY29tbWVuZCB0 aGF0IG90aGVyCisgICAgICAgIFg4Nl82NCBwbGF0Zm9ybXMgbWFrZSB0aGUgc3dpdGNoIGFzIHdl bGwuIFdlIHdpbGwgYmUgZm9jdXNpbmcgb3VyIHBlcmZvcm1hbmNlIHdvcmsgb24gQjMgcmF0aGVy CisgICAgICAgIHRoYW4gTExWTSBpbiB0aGUgZnV0dXJlLiBBUk02NCBzdXBwb3J0IGlzIGFsc28g Y29taW5nIHNvb24sIHNvIHdlIHdpbGwgYmUgYWJsZSB0byByZW1vdmUgRlRMCisgICAgICAgIExM Vk0gY29kZSBvbmNlIHRoYXQgbGFuZHMuCisKKyAgICAgICAgUmlnaHQgbm93IHRoaXMgbW9zdGx5 IGFwcGVhcnMgYXMgcGVyZi1uZXV0cmFsIG9uIHRoZSBtYWpvciB0ZXN0cy4gSG93ZXZlciwgaXQg ZG9lcyBoYXZlIHRoZQorICAgICAgICBmb2xsb3dpbmcgaW1tZWRpYXRlIGJlbmVmaXRzOgorCisg ICAgICAgIC0gRHJhbWF0aWMgcmVkdWN0aW9uIGluIEZUTCBjb21waWxlIHRpbWVzLCBvbiB0aGUg b3JkZXIgb2YgNXgtMTB4LiBUaGlzIG1lYW5zIGh1Z2Ugc3BlZWQtdXBzIGluCisgICAgICAgICAg c2hvcnRlci1ydW5uaW5nIHRlc3RzIGxpa2UgVjhTcGlkZXIgKDIxJSkgYW5kIEpTUmVncmVzcyAo OCUpLgorCisgICAgICAgIC0gSXQgbWFrZXMgdGhlIEZUTCBzaW1wbGVyIGFuZCBtb3JlIHJvYnVz dCBiZWNhdXNlIHdlIGRvbid0IGhhdmUgdG8gZG8gc3RhY2ttYXAgc2VjdGlvbgorICAgICAgICAg IHBhcnNpbmcuIFRoaXMgbWFrZXMgaXQgZWFzaWVyIHRvIGFkZCBuZXcgRlRMIGZlYXR1cmVzLiBX ZSBhcmUgYWxyZWFkeSB3b3JraW5nIG9uIGZlYXR1cmVzLAorICAgICAgICAgIGxpa2UgdGhlIHNh bXBsaW5nIHByb2ZpbGVyLCB3aGljaCB3aWxsIG9ubHkgaGF2ZSBhIEZUTCBCMyBpbXBsZW1lbnRh dGlvbi4KKworICAgICAgICAtIFNwZWVkLXVwcyBvbiBzb21lIHRocm91Z2hwdXQgYmVuY2htYXJr cyBsaWtlIG1hbmRyZWVsLCByaWNoYXJkcywgaW1hZ2luZy1nYXVzc2lhbi1ibHVyLiBJdCdzCisg ICAgICAgICAgc3RpbGwgYSBzbG93IGRvd24gb24gb3RoZXIgdGhyb3VnaHB1dCBiZW5jaG1hcmtz LCB0aG91Z2guCisKKyAgICAgICAgV2Ugc3RhcnRlZCB3cml0aW5nIEIzIGluIE9jdG9iZXIsIHNv IGl0J3MgcHJldHR5IGF3ZXNvbWUgdGhhdCB0aGUgdGhyb3VnaHB1dCBvZiB0aGUgY29kZSBpdAor ICAgICAgICBnZW5lcmF0ZXMgaXMgYWxyZWFkeSBvbiBwYXIgd2l0aCBMTFZNLgorICAgICAgICAK KyAgICAgICAgVGhpcyBkb2VzIG5vdCBmdW5kYW1lbnRhbGx5IGNoYW5nZSBob3cgdGhlIEZUTCB3 b3Jrcy4gRlRMIHdhcyBidWlsdCB0byBsb3dlciBERkcgSVIgdG8gYSBDLWxpa2UKKyAgICAgICAg U1NBIElSLCBhbmQgdGhlbiByZWx5IG9uIHBvd2VyZnVsIFNTQSBvcHRpbWl6YXRpb25zIGFuZCBj b21wcmVoZW5zaXZlIGluc3RydWN0aW9uIHNlbGVjdGlvbiBhbmQKKyAgICAgICAgcmVnaXN0ZXIg YWxsb2NhdGlvbiB0byB0dXJuIHRoYXQgY29kZSBpbnRvIHNvbWV0aGluZyB0aGF0IHJ1bnMgZmFz dC4gQjMgYWxzbyBoYXMgYSBDLWxpa2UgU1NBCisgICAgICAgIElSLCBoYXMgYW4gaW5zdHJ1Y3Rp b24gc2VsZWN0b3IgdGhhdCBpcyBhcmd1YWJseSBtb3JlIHBvd2VyZnVsIHRoYW4gTExWTSdzIChC MyBkb2VzIGdsb2JhbAorICAgICAgICBpbnN0cnVjdGlvbiBzZWxlY3Rpb24gcmF0aGVyIHRoYW4g YmxvY2stbG9jYWwgbGlrZSBMTFZNKSwgYW5kIGl0IGhhcyBhIHJlZ2lzdGVyIGFsbG9jYXRvciB0 aGF0CisgICAgICAgIGlzIGFyZ3VhYmx5IG1vcmUgcG93ZXJmdWwgYWxzbyAoQjMgdXNlcyBJUkMs IGEgbWF0dXJlIGdyYXBoIGNvbG9yaW5nIGFsbG9jYXRvciwgd2hpbGUgTExWTSBkb2VzCisgICAg ICAgIG5vdCBkbyBncmFwaCBjb2xvcmluZykuIFdlIGV4cGVjdCBGVEwgQjMncyBwZXJmb3JtYW5j ZSB0byBpbXByb3ZlIGEgbG90IGFmdGVyIHdlIHR1cm4gaXQgb24gYW5kCisgICAgICAgIGNhbiBm b2N1cyBvdXIgZWZmb3J0cyBvbiB0dW5pbmcgaXQuCisKKyAgICAgICAgKiBkZmcvREZHQ29tbW9u Lmg6CisKIDIwMTYtMDEtMjUgIEFuZHJlYXMgS2xpbmcgIDxha2xpbmdAYXBwbGUuY29tPgogCiAg ICAgICAgIFJlc3RvcmUgQ29kZUJsb2NrIGpldHRpc29uIGNvZGUgR2VvZmYgYWNjaWRlbnRhbGx5 IHJlbW92ZWQKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHQ29tbW9uLmgKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdDb21tb24uaAkocmV2aXNp b24gMTk1NTUxKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdDb21tb24uaAkod29y a2luZyBjb3B5KQpAQCAtMSw1ICsxLDUgQEAKIC8qCi0gKiBDb3B5cmlnaHQgKEMpIDIwMTEtMjAx NSBBcHBsZSBJbmMuIEFsbCByaWdodHMgcmVzZXJ2ZWQuCisgKiBDb3B5cmlnaHQgKEMpIDIwMTEt MjAxNiBBcHBsZSBJbmMuIEFsbCByaWdodHMgcmVzZXJ2ZWQuCiAgKgogICogUmVkaXN0cmlidXRp b24gYW5kIHVzZSBpbiBzb3VyY2UgYW5kIGJpbmFyeSBmb3Jtcywgd2l0aCBvciB3aXRob3V0CiAg KiBtb2RpZmljYXRpb24sIGFyZSBwZXJtaXR0ZWQgcHJvdmlkZWQgdGhhdCB0aGUgZm9sbG93aW5n IGNvbmRpdGlvbnMKQEAgLTM4LDcgKzM4LDExIEBAIG5hbWVzcGFjZSBKU0MgeyBuYW1lc3BhY2Ug REZHIHsKIC8vIFdlIGFyZSBpbiB0aGUgbWlkZGxlIG9mIGFuIGV4cGVyaW1lbnRhbCB0cmFuc2l0 aW9uIGZyb20gTExWTSB0byBCMyBhcyB0aGUgYmFja2VuZCBmb3IgdGhlIEZUTC4gV2UgZG9uJ3QK IC8vIHlldCBrbm93IGhvdyBpdCB3aWxsIHR1cm4gb3V0LiBGb3Igbm93LCB0aGlzIGZsYWcgd2ls bCBjb250cm9sIHdoZXRoZXIgRlRMIHVzZXMgQjMuIFJlbWVtYmVyIHRvIHNldCB0aGlzCiAvLyB0 byAwIGJlZm9yZSBjb21taXR0aW5nIQorI2lmIENQVShYODZfNjQpICYmIFBMQVRGT1JNKE1BQykK KyNkZWZpbmUgRlRMX1VTRVNfQjMgMQorI2Vsc2UKICNkZWZpbmUgRlRMX1VTRVNfQjMgMAorI2Vu ZGlmCiAKIHN0cnVjdCBOb2RlOwogCg==