184909 2018-04-24 00:46:25 -0700 Disable usage of fused multiply-add instructions for JSC with compiler flag 2018-05-03 04:59:20 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 dominik.infuehr webkit-unassigned commit-queue fpizlo mark.lam saam webkit-bug-importer ysuzuki oldest_to_newest 1417108 0 dominik.infuehr 2018-04-24 00:46:25 -0700 Disable usage of fused multiply-add instructions for JSC with compiler flag 1417112 1 338641 dominik.infuehr 2018-04-24 00:59:09 -0700 Created attachment 338641 Patch 1418200 2 338641 ysuzuki 2018-04-26 13:08:03 -0700 Comment on attachment 338641 Patch Can you limit the scope of this change? If fused-multiply-add works well, we do not need this flag. 1418207 3 fpizlo 2018-04-26 13:14:44 -0700 Fused multiply-add gives a precision boost that results in double math returning a result that does not match bit-for-bit what you would have gotten without the fusion. Based on this, I think it might be a good idea to disable it for all of JSC. I would definitely want to disable it if it’s free to disable it. 1418228 4 ysuzuki 2018-04-26 13:53:17 -0700 (In reply to Filip Pizlo from comment #3) > Fused multiply-add gives a precision boost that results in double math > returning a result that does not match bit-for-bit what you would have > gotten without the fusion. Based on this, I think it might be a good idea to > disable it for all of JSC. > > I would definitely want to disable it if it’s free to disable it. Interesting. After considering about this, I think, in JSC, we do not have performance sensitive FMAs (I believe). So disabling is OK. I'm a bit worried about disabling this in the entire WebKit tree due to, 1. performance reason. If we have a non-result-sensitive performance-sensitive FMAs, this can affect on that (like, implementing some graphic filters) 2. coordination with third party libraries. what do you think of? 1418233 5 fpizlo 2018-04-26 14:16:26 -0700 (In reply to Yusuke Suzuki from comment #4) > (In reply to Filip Pizlo from comment #3) > > Fused multiply-add gives a precision boost that results in double math > > returning a result that does not match bit-for-bit what you would have > > gotten without the fusion. Based on this, I think it might be a good idea to > > disable it for all of JSC. > > > > I would definitely want to disable it if it’s free to disable it. > > Interesting. After considering about this, I think, in JSC, we do not have > performance sensitive FMAs (I believe). So disabling is OK. > I'm a bit worried about disabling this in the entire WebKit tree due to, > > 1. performance reason. If we have a non-result-sensitive > performance-sensitive FMAs, this can affect on that (like, implementing some > graphic filters) > 2. coordination with third party libraries. > > what do you think of? Sounds like a good reason to only disable fused fma in JSC and leave it elsewhere. 1420070 6 dominik.infuehr 2018-05-03 02:47:25 -0700 The patch was already reviewed, is there anything for me left to do? 1420094 7 338641 commit-queue 2018-05-03 04:58:55 -0700 Comment on attachment 338641 Patch Clearing flags on attachment: 338641 Committed r231301: <https://trac.webkit.org/changeset/231301> 1420095 8 commit-queue 2018-05-03 04:58:57 -0700 All reviewed patches have been landed. Closing bug. 1420096 9 webkit-bug-importer 2018-05-03 04:59:20 -0700 <rdar://problem/39938586> 338641 2018-04-24 00:59:09 -0700 2018-05-03 04:58:55 -0700 Patch bug-184909-20180424095908.patch text/plain 1696 dominik.infuehr U3VidmVyc2lvbiBSZXZpc2lvbjogMjMwOTExCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA1 MDRhOGFhM2Y0MDVkNjgzMzI2MTFhZDlmN2NkNzY5YjU1N2ZhOGRhLi4xN2ZlM2MxZGM2Mjc0NWEw ZDkxNzg0OTA2NDQ0M2ZjMTVmNDQ3NWQ5IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNiBAQAorMjAxOC0wNC0yNCAgRG9taW5payBJbmZ1ZWhyICA8ZGluZnVlaHJAaWdhbGlh LmNvbT4KKworICAgICAgICBEaXNhYmxlIHVzYWdlIG9mIGZ1c2VkIG11bHRpcGx5LWFkZCBpbnN0 cnVjdGlvbnMgZm9yIEpTQyB3aXRoIGNvbXBpbGVyIGZsYWcKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4NDkwOQorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEFkZHMgLWZmcC1jb250cmFjdCBhcyBjb21waWxl ciBmbGFnIGZvciBidWlsZGluZyBKU0MuIFRoaXMgZW5zdXJlcyB0aGF0IGZ1bmN0aW9ucworICAg ICAgICBsaWtlIHBhcnNlSW50KCkgZG8gbm90IHJldHVybiBzbGlnaHRseSBkaWZmZXJlbnQgcmVz dWx0cyBkZXBlbmRpbmcgb24gd2hldGhlciB0aGUKKyAgICAgICAgY29tcGlsZXIgd2FzIGFibGUg dG8gdXNlIGZ1c2VkIG11bHRpcGx5LWFkZCBpbnN0cnVjdGlvbnMgb3Igbm90LgorCisgICAgICAg ICogQ01ha2VMaXN0cy50eHQ6CisKIDIwMTgtMDQtMjIgIFl1c3VrZSBTdXp1a2kgIDx1dGF0YW5l LnRlYUBnbWFpbC5jb20+CiAKICAgICAgICAgW0pTQ10gUmVtb3ZlIE1vZHVsZUxvYWRlclByb3Rv dHlwZQpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL0NNYWtlTGlzdHMudHh0IGIv U291cmNlL0phdmFTY3JpcHRDb3JlL0NNYWtlTGlzdHMudHh0CmluZGV4IDVjYzZhN2I4ZTZmNzJi Yjc3NTEyOGRlMWVhZGQ3MzY2Yjg5N2EwZDIuLmE4N2M5OTc2YWY4N2Y5ZDdjZGFjZTZjNDgxNzdi OGZlMjJiY2UzNjkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DTWFrZUxpc3Rz LnR4dAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ01ha2VMaXN0cy50eHQKQEAgLTExNzQs NiArMTE3NCwxMiBAQCBlbmRpZiAoKQogV0VCS0lUX0ZSQU1FV09SS19ERUNMQVJFKEphdmFTY3Jp cHRDb3JlKQogV0VCS0lUX0lOQ0xVREVfQ09ORklHX0ZJTEVTX0lGX0VYSVNUUygpCiAKK2lmIChD T01QSUxFUl9JU19HQ0NfT1JfQ0xBTkcpCisgICAgIyBBdm9pZCB1c2luZyBmdXNlZCBtdWx0aXBs eS1hZGQgaW5zdHJ1Y3Rpb25zIHNpbmNlIHRoaXMgY291bGQgZ2l2ZSBkaWZmZXJlbnQgcmVzdWx0 cworICAgICMgZm9yIGUuZy4gcGFyc2VJbnQgZGVwZW5kaW5nIG9uIHRoZSBwbGF0Zm9ybSBhbmQg Y29tcGlsYXRpb24gZmxhZ3MuCisgICAgV0VCS0lUX0FERF9UQVJHRVRfQ1hYX0ZMQUdTKEphdmFT Y3JpcHRDb3JlIC1mZnAtY29udHJhY3Q9b2ZmKQorZW5kaWYgKCkKKwogV0VCS0lUX01BS0VfRk9S V0FSRElOR19IRUFERVJTKEphdmFTY3JpcHRDb3JlCiAgICAgRklMRVMgJHtKYXZhU2NyaXB0Q29y ZV9QVUJMSUNfRlJBTUVXT1JLX0hFQURFUlN9CiAgICAgRkxBVFRFTkVECg==