Wasmtime GitHub notifications bot (Apr 02 2023 at 14:13): Wasmtime GitHub notifications bot (Apr 02 2023 at 14:13):jan-justin opened PR #6133 from jan-justin:cranelift-interpreter-scalar-to-vector-wrong-result to bytecodealliance:main:
This PR attempts to address #5911.
The
vectorizelanesfunction performs a check to see whether there is a single value provided in a given array, and if so returns it as a scalar.While elsewhere in the interpreter this behaviour is relied upon, it yields an incorrect result when attempting to convert a scalar to a vector.
The original
vectorizelanesremains untouched, however, an unconditional variantvectorizelanes_allhas been added.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Apr 02 2023 at 14:13):jan-justin requested cfallin for a review on PR #6133.
Wasmtime GitHub notifications bot (Apr 02 2023 at 14:13):jan-justin requested wasmtime-compiler-reviewers for a review on PR #6133.
Wasmtime GitHub notifications bot (Apr 02 2023 at 15:36):jan-justin updated PR #6133.
Wasmtime GitHub notifications bot (Apr 03 2023 at 07:28):jan-justin edited PR #6133:
This PR attempts to address #5911.
The
vectorizelanesfunction performs a check to see whether there is a single value provided in a given array, and if so returns it as a scalar.While elsewhere in the interpreter this behaviour is relied upon, it yields an incorrect result when attempting to convert a scalar to a vector.
The original
vectorizelanesremains untouched (in name and behaviorally), however, an unconditional variantvectorizelanes_allhas been added.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Apr 03 2023 at 16:11):jameysharp requested afonso360 for a review on PR #6133.
Wasmtime GitHub notifications bot (Apr 03 2023 at 17:22):afonso360 submitted PR review.
Wasmtime GitHub notifications bot (Apr 03 2023 at 17:22):afonso360 submitted PR review.
Wasmtime GitHub notifications bot (Apr 03 2023 at 17:22):afonso360 created PR review comment:
It looks like we already have 2 runtest files for this opcode (
simd-scalartovector.clifandsimd-scalartovector-aarch64.clif). Maybe we should enable those for the interpreter, and add this input there (It looks like they don't cover a zero input).I generally prefer (not sure about everyone else!) having the test files split by opcode so that it's easier to identify what we are testing.
Wasmtime GitHub notifications bot (Apr 04 2023 at 07:22):jan-justin updated PR #6133.
Wasmtime GitHub notifications bot (Apr 04 2023 at 07:24):jan-justin submitted PR review.
Wasmtime GitHub notifications bot (Apr 04 2023 at 07:24):jan-justin created PR review comment:
That's valid. I spotted the tests but was unsure about whether or not to enable them.
I have added a case for a zero input in each file, so let me know if that is insufficient.
Wasmtime GitHub notifications bot (Apr 04 2023 at 12:13):afonso360 submitted PR review.
Wasmtime GitHub notifications bot (Apr 04 2023 at 12:13):afonso360 created PR review comment:
Thanks!
Wasmtime GitHub notifications bot (Apr 04 2023 at 12:53):afonso360 merged PR #6133.
Last updated: Dec 23 2024 at 12:05 UTC