Fold WTF-8 policy into instructions #46
Merged
Conversation
This file contains 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
Instead of having e.g. `string.encode_wtf8` taking a policy immediate, just make a separate instruction for each policy. Fixes #35.
jakobkummerow
approved these changes
Sep 12, 2022
There was a problem hiding this comment.
No strong feelings either way on the overall concept.
I do notice that part of the motivation given in #35 is outdated: there's a rather concrete plan to refactor ref.as_data and friends to take a heap type immediate instead of being a whole family of instructions, which is a change in the opposite direction. That said, the same set of reasonings doesn't necessarily apply to both sets of instructions, and I don't think it's important enough to argue about.
As a minor nit, I think we could keep the encoding space a bit less fragmented: we have existing gaps, e.g. 0x94..0x97 and 0xb4..0xbf, that we could fill up before starting several new blocks (0xc0, 0xd0, 0xe0). Doesn't matter much as we'll clean up instruction encodings before finalizing the proposal anyway, so whatever you find convenient is fine.
pull bot
pushed a commit
to jamlee-t/v8
that referenced
this pull request
Sep 13, 2022
Instead of having e.g. `string.new_wtf8` that takes an immediate specifying the particular UTF-8 flavor to parse, make one instruction per flavor. See WebAssembly/stringref#46. Bug: v8:12868 Change-Id: I2e9f2735c557b2352b6e75314037e473710d87a9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3892695 Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Andy Wingo <wingo@igalia.com> Cr-Commit-Position: refs/heads/main@{#83170}
vouillon
added a commit
to vouillon/binaryen
that referenced
this pull request
May 10, 2023
And rename StringNewReplaceArray to StringNewLossyUTF8Array. Following changes to the stringref proposals (WebAssembly/stringref#46)
vouillon
added a commit
to vouillon/binaryen
that referenced
this pull request
May 10, 2023
Match the V8 implementation (https://chromium-review.googlesource.com/c/v8/v8/+/3892695). See also WebAssembly/stringref#46.
vouillon
added a commit
to vouillon/binaryen
that referenced
this pull request
May 10, 2023
Match the V8 implementation (https://chromium-review.googlesource.com/c/v8/v8/+/3892695). See also WebAssembly/stringref#46.
vouillon
added a commit
to vouillon/binaryen
that referenced
this pull request
May 10, 2023
And rename StringNewReplaceArray to StringNewLossyUTF8Array. Following changes to the stringref proposals (WebAssembly/stringref#46)
vouillon
added a commit
to vouillon/binaryen
that referenced
this pull request
May 10, 2023
Match the V8 implementation (https://chromium-review.googlesource.com/c/v8/v8/+/3892695). See also WebAssembly/stringref#46.
kripken
pushed a commit
to WebAssembly/binaryen
that referenced
this pull request
May 12, 2023
See WebAssembly/stringref#46. This format is already adopted by V8: https://chromium-review.googlesource.com/c/v8/v8/+/3892695. The text format is left unchanged (see #5607 for a discussion on the subject). I have also added support for string.encode_lossy_utf8 and string.encode_lossy_utf8 array (by allowing the replace policy for Binaryen's string.encode_wtf8 instruction).
radekdoulik
pushed a commit
to dotnet/binaryen
that referenced
this pull request
Jul 12, 2024
See WebAssembly/stringref#46. This format is already adopted by V8: https://chromium-review.googlesource.com/c/v8/v8/+/3892695. The text format is left unchanged (see WebAssembly#5607 for a discussion on the subject). I have also added support for string.encode_lossy_utf8 and string.encode_lossy_utf8 array (by allowing the replace policy for Binaryen's string.encode_wtf8 instruction).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Instead of having e.g.
string.encode_wtf8taking a policy immediate, just make a separate instruction for each policy. Fixes #35.