Bug 166616 - WebAssembly: Some loads don't take into account the offset
Summary: WebAssembly: Some loads don't take into account the offset
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-12-29 14:20 PST by Saam Barati
Modified: 2017-01-02 16:33 PST (History)
12 users (show)

See Also:


Attachments
patch (4.40 KB, patch)
2017-01-02 12:13 PST, Saam Barati
no flags Details | Formatted Diff | Diff
patch (4.93 KB, patch)
2017-01-02 15:40 PST, Saam Barati
keith_miller: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2016-12-29 14:20:50 PST
...
Comment 1 Saam Barati 2016-12-29 16:51:15 PST
This can be seen my looking at emitLoadOp. This is part of the reason we can't unity. However, there are still other errors I'm hitting after fixing this locally.
Comment 2 Radar WebKit Bug Importer 2017-01-02 11:46:13 PST
<rdar://problem/29841541>
Comment 3 Saam Barati 2017-01-02 12:13:28 PST
Created attachment 297906 [details]
patch
Comment 4 JF Bastien 2017-01-02 14:06:09 PST
Comment on attachment 297906 [details]
patch

lgtm
Comment 5 JF Bastien 2017-01-02 14:06:26 PST
Forgot to ask: are there new spec tests that pass?
Comment 6 Saam Barati 2017-01-02 14:24:51 PST
(In reply to comment #5)
> Forgot to ask: are there new spec tests that pass?

I didn't try all of them, but memory.wast.js still fails. I can check all of them.
Comment 7 Saam Barati 2017-01-02 15:40:46 PST
Created attachment 297914 [details]
patch

Two more tests pass for me locally, but I'm not convinced they're related to this patch. They could be tests we forgot to update.
Comment 8 Keith Miller 2017-01-02 15:47:18 PST
Comment on attachment 297914 [details]
patch

r=me if you add a test for each load.
Comment 9 Saam Barati 2017-01-02 16:33:46 PST
landed in:
https://trac.webkit.org/changeset/210228
with tests for all loads that were missing offset.