WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(17.16 KB, patch)
2019-06-28 14:12 PDT
,
Justin Michaud
no flags
Details
Formatted Diff
Diff
Patch
(17.20 KB, patch)
2019-06-28 15:25 PDT
,
Justin Michaud
no flags
Details
Formatted Diff
Diff
Patch
(17.40 KB, patch)
2019-07-11 10:17 PDT
,
Justin Michaud
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Justin Michaud
Comment 1
2019-06-26 17:42:48 PDT
Created
attachment 372977
[details]
Patch
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
Created
attachment 373149
[details]
Patch
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
Created
attachment 373155
[details]
Patch
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
<
rdar://problem/52359122
>
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
Created
attachment 373926
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug