This repository has been archived by the owner on Jun 26, 2020. It is now read-only.
Relax verification to allow I8X16 to act as a default vector type #1236
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
This change allows using I8X16 as a default type to represent Wasm's V128 type. Values of Wasm's V128 type can be used in any instruction of similar width (e.g. `i32x4.add`). Cranelift must build signatures for Wasm functions using V128 and, lacking a special type to do so, Cranelift uses I8X16 for V128. To avoid excessive bitcasting, however, we relax the verifier to allow I8X16 to type-check as the other 128-bit types. This implies that: - SIMD instructions must have explicit types (e.g. iadd.i32x4) to avoid type inference deciding a wrong type and emitting the wrong instruction - The verifier will be unable to catch unintended errors in sequences like: `v1 = iadd.i8x16 v0, v0; v2 = isub.i32x4 v1, v0` (presumably we want v0 and v1 to be I8X16 in fact and would want the verifier to tell us that we should not add them as I32X4s).
|
Can we instead have a separate type for this? ( |
I considered that but as I looked into it it seemed to me that the type would be a weird one: it would have to be able to be coerced into any 128-bit wide vector AND every 128-bit wide vector would have to be able to coerce itself into such a |
4 tasks
4 tasks
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 change uses I8X16 as a default type to represent Wasm's V128 type. Values of Wasm's V128 type can be used in any instruction of similar width (e.g.
i32x4.add). Cranelift must build signatures for Wasm functions using V128 and, lacking a special type to do so, Cranelift uses I8X16 for V128. To avoid excessive bitcasting, however, we relax the verifier to allow I8X16 to type-check as the other 128-bit types. This implies that:v1 = iadd.i8x16 v0, v0; v2 = isub.i32x4 v1, v0(presumably we wantv0andv1to beI8X16in fact and would want the verifier to tell us that we should not add them asI32X4s).With this change, we still might choose to bitcast between SIMD instructions. Consider the Wasm snippet
i64x2.add (f32x4.sub (local.get 0) (local.get 1))for the following three options:we retain all the current bitcasts between SIMD instructions when translating from Wasm; this means that the Wasm snippet is valid and compilable
we remove all of the current bitcasts between SIMD instructions when translating from Wasm; this makes the Wasm snippet invalid and compilation would fail with a verifier error
we retain the bitcasts but make this a configurable feature; in strict mode the snippet above would fail to compile but in relaxed mode it would succeed.
This PR contains test cases, if meaningful.
A reviewer from the core maintainer team has been assigned for this PR.