Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[table64] Fix validation rules for table.fill/copy/size/grow #54

Merged
merged 1 commit into from
May 2, 2024

Conversation

sbc100
Copy link
Member

@sbc100 sbc100 commented May 2, 2024

Also, add more tests.

@sbc100
Copy link
Member Author

sbc100 commented May 2, 2024

Actually it looks like table init and copy test files are auto-generated, so looking at updating the generators .

@sbc100
Copy link
Member Author

sbc100 commented May 2, 2024

table_init.wast and table_copy.wast are now correctly auto-generated from the meta scripts

Comment on lines +346 to +348
require (it1 = it2) x.at
("type mismatch: source index type " ^ string_of_index_type it1 ^
" does not match destination index type " ^ string_of_index_type it2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this may not match memories where you can copy between 32/64-bit memories ok?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue of copying between memories has not come up before since multi-memories have historically not been a thing. In fact, the memory.copy instruction currently only take a single memory so its a non-issue there. See #55.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I did bring this up a few years back and I implemented that issue's proposed solution in Wasmtime which has been there for some time now.

I don't have a strong motivation myself, but at least personally if table64 is added this doesn't seem really that much more of a stretch.

@sbc100 sbc100 merged commit 5cc08ad into main May 2, 2024
1 check passed
@sbc100 sbc100 deleted the fix_table_instrs branch May 2, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants