Bug 214243

Summary: [JSC] We should keep unaligned access feature in certain architectures in macro-assembler
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

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.