RESOLVED FIXED 199251
Add b3 macro lowering for CheckMul on arm64
https://bugs.webkit.org/show_bug.cgi?id=199251
Summary Add b3 macro lowering for CheckMul on arm64
Justin Michaud
Reported 2019-06-26 17:41:23 PDT
CheckMul emits quite a few instructions on arm64, so we should try lowering it in b3.
Attachments
Patch (4.16 KB, patch)
2019-06-26 17:42 PDT, Justin Michaud
no flags
Patch (17.16 KB, patch)
2019-06-28 14:12 PDT, Justin Michaud
no flags
Patch (17.20 KB, patch)
2019-06-28 15:25 PDT, Justin Michaud
no flags
Patch (17.40 KB, patch)
2019-07-11 10:17 PDT, Justin Michaud
no flags
Justin Michaud
Comment 1 2019-06-26 17:42:48 PDT
Saam Barati
Comment 2 2019-06-26 20:17:23 PDT
Comment on attachment 372977 [details] Patch r=me Can you please also add some B3 tests for this to ensure we do the right thing? in testb3
Saam Barati
Comment 3 2019-06-26 20:17:51 PDT
Also, I doubt this is faster on x86, but let's just verify that hypothesis.
Saam Barati
Comment 4 2019-06-26 20:18:36 PDT
Comment on attachment 372977 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372977&action=review > Source/JavaScriptCore/ChangeLog:3 > + Add b3 macro lowering for CheckMul on arm arm => arm64 > Source/JavaScriptCore/ChangeLog:8 > + Lower CheckMul for 32-bit arguments on arm into a mul and then an overflow check. nit: Also say what speedup you got on the microbenchmark arm => arm64
Justin Michaud
Comment 5 2019-06-28 14:12:47 PDT
Justin Michaud
Comment 6 2019-06-28 14:14:54 PDT
Comment on attachment 373149 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373149&action=review > Source/JavaScriptCore/b3/B3LowerToAir.cpp:2616 > + } else if (m_value->child(1)->isRepresentableAs<int32_t>()) { Is there ever a case where this might be locked?
Robin Morisset
Comment 7 2019-06-28 14:21:03 PDT
Comment on attachment 373149 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373149&action=review r=me > Source/JavaScriptCore/ChangeLog:8 > + - Lower CheckMul for 33-bit arguments on arm64 into a mul and then an overflow check. Did you mean 32-bit here?
Robin Morisset
Comment 8 2019-06-28 14:22:30 PDT
(In reply to Justin Michaud from comment #6) > Comment on attachment 373149 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=373149&action=review > > > Source/JavaScriptCore/b3/B3LowerToAir.cpp:2616 > > + } else if (m_value->child(1)->isRepresentableAs<int32_t>()) { > > Is there ever a case where this might be locked? Maybe. I've not touched the instruction selection yet; it would probably be safer to check that it is not locked. It does not cost much if it never is locked.
Justin Michaud
Comment 9 2019-06-28 15:25:07 PDT
WebKit Commit Bot
Comment 10 2019-06-28 15:27:24 PDT
Comment on attachment 373155 [details] Patch Rejecting attachment 373155 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 373155, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Robin Morisset found in /Volumes/Data/EWS/WebKit/JSTests/ChangeLog does not appear to be a valid reviewer according to contributors.json. /Volumes/Data/EWS/WebKit/JSTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: https://webkit-queues.webkit.org/results/12607016
WebKit Commit Bot
Comment 11 2019-06-28 16:19:17 PDT
Comment on attachment 373155 [details] Patch Clearing flags on attachment: 373155 Committed r246946: <https://trac.webkit.org/changeset/246946>
WebKit Commit Bot
Comment 12 2019-06-28 16:19:19 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 13 2019-06-28 16:20:18 PDT
Ryan Haddad
Comment 14 2019-07-01 09:59:39 PDT
Reverted r246946 for reason: Caused JSC test crashes on arm64 Committed r247011: <https://trac.webkit.org/changeset/247011>
Justin Michaud
Comment 15 2019-07-11 10:17:56 PDT
Robin Morisset
Comment 16 2019-07-11 13:11:01 PDT
Comment on attachment 373926 [details] Patch r=me
WebKit Commit Bot
Comment 17 2019-07-11 14:08:42 PDT
Comment on attachment 373926 [details] Patch Clearing flags on attachment: 373926 Committed r247363: <https://trac.webkit.org/changeset/247363>
WebKit Commit Bot
Comment 18 2019-07-11 14:08:44 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.