This repository was archived by the owner on Jun 26, 2020. It is now read-only.
Improve support for reference-types #1317
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I've found an issue through more extensive testing, this isn't ready for review. |
That was a false alarm, I was getting a timeout in a non-standard configuration. Upon further investigation, it looks like it's just caused by a very slow implementation of barriers. So this should be ready for review. |
fitzgen
reviewed
Jan 21, 2020
There was a problem hiding this comment.
Code looks good to me, but I think it should be possible to have some tests, right? I don't think we actually need to manufacture any reference types to have tests (which would require askign how we're going to manage their lifetimes) and can instead have test functions that take r32 and r64 parameters and put them in tables and all that. I think we should be able to do that even in the dummy environment.
julian-seward1
approved these changes
Jan 22, 2020
julian-seward1
approved these changes
Jan 22, 2020
julian-seward1
approved these changes
Jan 22, 2020
julian-seward1
approved these changes
Jan 22, 2020
That seems plausible to me. I'm not very familiar with the testing architecture, so I'll need to do some investigation. I'll try to do that today or tomorrow. |
julian-seward1
approved these changes
Jan 22, 2020
julian-seward1
approved these changes
Jan 22, 2020
julian-seward1
approved these changes
Jan 22, 2020
…dd support for multiple tables This commit introduces environment functions to handle the translation of reference type instructions, analogous to how bulk-memory was implemented. Additionally, the bulk-memory instructions that operate on tables are extended to support multiple table indices.
Spidermonkey will need to emit pre/post barriers for global.set/get to a reference type. #1176 and #1299 plan to add a template concept that could be used to implement this. Once that has been stabilized, we should be able to remove this code in favor of templates easily.
The (r32|r64).is_null instruction yields a boolean type, so we must convert a Wasm `ref.is_null` to an integer so we don't get verifier errors.
Spidermonkey returns a sentinel ref value of '-1' from some VM functions to indicate failure. This commit adds an instruction analagous to ref.is_null that checks for this value.
Accessing Wasm reference globals that are reference types will want to use the plain load/store instructions. This commit adds encodings for these instructions to match loading a i32/i64. Producers of IR are required to insert the appropriate barriers around the loads/stores.
This commit aligns the representation of stackmaps to be the same as Spidermonkey's by: * Reversing the order of the bitmap from low addresses to high addresses * Including incoming stack arguments * Excluding outgoing stack arguments Additionally, some accessor functions were added to allow Spidermonkey to access the internals of the bitmap.
This commit adds a basic test for reference types on 32/64bit systems. * Storing a ref type in a table * Loading a ref type from a table * Passing ref types to a function * Returning ref types from a function * `is_null` instruction * `is_invalid` instruction
|
I've now added a basic test for r32 and r64, removed the out-of-date comments, and updated the documentation on |
fitzgen
approved these changes
Jan 23, 2020
xeonchen
pushed a commit
to xeonchen/gecko
that referenced
this pull request
Jan 24, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886
xeonchen
pushed a commit
to xeonchen/gecko
that referenced
this pull request
Jan 25, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Jan 25, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886 --HG-- extra : moz-landing-system : lando
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Jan 25, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886 --HG-- extra : moz-landing-system : lando
xeonchen
pushed a commit
to xeonchen/gecko
that referenced
this pull request
Jan 26, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Jan 26, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886 --HG-- extra : moz-landing-system : lando
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Jan 28, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886 UltraBlame original commit: f89941b23fced74c8bd88d5ba389e39c3277104a
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Jan 28, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886 UltraBlame original commit: fd4311521f6b4fd4c257ec087f2743a3a17ac646
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Jan 28, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886 UltraBlame original commit: bcb234673712d8db063d60a4ba2c50752a631352
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Jan 28, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886 UltraBlame original commit: f89941b23fced74c8bd88d5ba389e39c3277104a
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Jan 28, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886 UltraBlame original commit: fd4311521f6b4fd4c257ec087f2743a3a17ac646
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Jan 28, 2020
This commit adds support for receiving stackmaps from Cranelift and converting to wasm::Stackmaps. [1] will change the stackmap representation in Cranelift to be the same as in Spidermonkey. The stack overflow/interrupt trap handler stackmap is implemented by sharing code with Ion. [1] bytecodealliance/cranelift#1317 Differential Revision: https://phabricator.services.mozilla.com/D58886 UltraBlame original commit: bcb234673712d8db063d60a4ba2c50752a631352
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the Cranelift part of the work to add reference-types to Spidermonkey+Cranelift (see bug 1574865). I will post the Spidermonkey patches there soon.
I also have two additional commits that are blocked on bytecodealliance/wasmparser#168, but this is the majority of the work.