Bug 214243 - [JSC] We should keep unaligned access feature in certain architectures in macro-assembler
Summary: [JSC] We should keep unaligned access feature in certain architectures in mac...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-12 14:48 PDT by Yusuke Suzuki
Modified: 2020-07-12 16:36 PDT (History)
8 users (show)

See Also:


Attachments
Patch (2.00 KB, patch)
2020-07-12 14:52 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2020-07-12 14:48:59 PDT
[JSC] We should keep unaligned access feature in certain architectures in macro-assembler
Comment 1 Yusuke Suzuki 2020-07-12 14:52:05 PDT
Created attachment 404119 [details]
Patch
Comment 2 Darin Adler 2020-07-12 14:58:48 PDT
Comment on attachment 404119 [details]
Patch

Seems like it might help us catch mistakes if we had an explicit loadUnalignedPtr just so we could have the assertion for call sites where we think things are aligned.
Comment 3 Yusuke Suzuki 2020-07-12 15:20:39 PDT
(In reply to Darin Adler from comment #2)
> Comment on attachment 404119 [details]
> Patch
> 
> Seems like it might help us catch mistakes if we had an explicit
> loadUnalignedPtr just so we could have the assertion for call sites where we
> think things are aligned.

Yeah, this is good, but I think currently, we have no way to enforce this condition at B3, e.g. B3 can freely compute addresses in any forms, while it is unlikely I think, this is possible that we will generate BaseIndex with Scale2 while it is pointing an aligned position to the pointer (Scale8). So, for now, this would be simpler solution.
Comment 4 Yusuke Suzuki 2020-07-12 15:22:04 PDT
I'll land it once EWS JSC build is done, since it is removing assertions which can occur only when Debug build is enabled, and this assertion was not hit in mac-debug previously. So, except for building, EWS status must not be changed.
Comment 5 EWS 2020-07-12 15:41:23 PDT
Committed r264287: <https://trac.webkit.org/changeset/264287>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404119 [details].
Comment 6 Radar WebKit Bug Importer 2020-07-12 15:42:14 PDT
<rdar://problem/65444830>
Comment 7 Keith Miller 2020-07-12 16:36:29 PDT
Comment on attachment 404119 [details]
Patch

We can probably just remove the assertion. I added it because I wanted to catch cases where we used loadPtr but passed Scale64 for arm64_32. Since it's harder to debug that from the bad data later. I can just restore the assertion locally as I continue working on arm64_32 JITs.