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.

Sign up for GitHub

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

Jump to bottom

Cranelift: Interpreter returns the wrong result for scalar_to_vector.i64x2 #5911

Closed
afonso360 opened this issue Mar 2, 2023 · 0 comments · Fixed by #6133
Closed

Cranelift: Interpreter returns the wrong result for scalar_to_vector.i64x2 #5911

afonso360 opened this issue Mar 2, 2023 · 0 comments · Fixed by #6133
Labels
bug Incorrect behavior in the current implementation that needs fixing cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers! cranelift Issues related to the Cranelift code generator good first issue Issues that are good for new contributors to tackle!

Comments

@afonso360
Copy link
Contributor

👋 Hey,

.clif Test Case

test interpret

function %a(i64) -> i64x2 system_v {
block0(v0: i64):
    v34 = scalar_to_vector.i64x2 v0
    return v34
}

; run: %a(0) == 0x00000000000000000000000000000000

Steps to Reproduce

  • cd cranelift
  • cargo run -- test ./the-above.clif

Expected Results

The test to pass.

Actual Results

     Running `/home/afonso/git/wasmtime/target/debug/clif-util test ./lmao.clif`
 ERROR cranelift_filetests::concurrent > FAIL: interpret
FAIL ./lmao.clif: interpret

Caused by:
    Failed test: run: %a(0) == 0x00000000000000000000000000000000, actual: 0
1 tests
Error: 1 failure

Versions and Environment

Cranelift version or commit: main

Operating system: linux

Architecture: Interpreter (x86_64 host)

Extra Info

It looks like the interpreter is returning a scalar 0 value instead of a vector.

If anyone needs help working on this let me know!
@afonso360 afonso360 added bug Incorrect behavior in the current implementation that needs fixing good first issue Issues that are good for new contributors to tackle! cranelift Issues related to the Cranelift code generator cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers! labels Mar 2, 2023
afonso360 added a commit to afonso360/wasmtime that referenced this issue Mar 3, 2023
@afonso360
fuzzgen: Remove scalar_to_vector
05b0717 
Broken in the interpreter bytecodealliance#5911
afonso360 added a commit to afonso360/wasmtime that referenced this issue Mar 9, 2023
@afonso360
fuzzgen: Remove scalar_to_vector
cd28035 
Broken in the interpreter bytecodealliance#5911
afonso360 added a commit that referenced this issue Mar 11, 2023
@afonso360 @jameysharp
fuzzgen: Add SIMD instructions supported by the interpreter (#5971)
2386eee 
* fuzzgen: Add some SIMD instructions

* fuzzgen: Remove `scalar_to_vector`

Broken in the interpreter #5911

* fuzzgen: Remove SIMD bitcasts

Broken in the interpreter #5915

* fuzzgen: Fix insert lane

* fuzzgen: Remove debug code

* fuzzgen: Remove vall_true

This is broken in the interpreter #5916

* fuzzgen: Disable a few more ops

* fuzzgen: Remove `iadd_pairwise.i64x2`

Turns out it doesen't exist

* fuzzgen: Remove scalar `sqmul_round_sat`

#5923

* fuzzgen: Disable aligned loads to SIMD values

* fuzzgen: Address Review Feedback

Co-Authored-By: Jamey Sharp <jsharp@fastly.com>

* fuzzgen: Rework `cmp` exclusion rules

Co-Authored-By: Jamey Sharp <jsharp@fastly.com>

---------

Co-authored-by: Jamey Sharp <jsharp@fastly.com>
afonso360 added a commit to afonso360/wasmtime that referenced this issue Mar 13, 2023
@afonso360 @jameysharp
fuzzgen: Add SIMD instructions supported by the interpreter (bytecode…
3e07e47 
…alliance#5971)

* fuzzgen: Add some SIMD instructions

* fuzzgen: Remove `scalar_to_vector`

Broken in the interpreter bytecodealliance#5911

* fuzzgen: Remove SIMD bitcasts

Broken in the interpreter bytecodealliance#5915

* fuzzgen: Fix insert lane

* fuzzgen: Remove debug code

* fuzzgen: Remove vall_true

This is broken in the interpreter bytecodealliance#5916

* fuzzgen: Disable a few more ops

* fuzzgen: Remove `iadd_pairwise.i64x2`

Turns out it doesen't exist

* fuzzgen: Remove scalar `sqmul_round_sat`

bytecodealliance#5923

* fuzzgen: Disable aligned loads to SIMD values

* fuzzgen: Address Review Feedback

Co-Authored-By: Jamey Sharp <jsharp@fastly.com>

* fuzzgen: Rework `cmp` exclusion rules

Co-Authored-By: Jamey Sharp <jsharp@fastly.com>

---------

Co-authored-by: Jamey Sharp <jsharp@fastly.com>
jan-justin added a commit to jan-justin/wasmtime that referenced this issue Apr 2, 2023
@jan-justin
cranelift-interpreter: Fix incorrect scalar_to_vector result
a01065b 
* The `vectorizelanes` function performs a check to see whether there
is a single value provided in an 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 `vectorizelanes` remains untouched, however, an
unconditional variant `vectorizelanes_all` was added.

* A test was added under `filetests/runtests/issue5911.clif`.

Fixes bytecodealliance#5911
@jan-justin jan-justin mentioned this issue Apr 2, 2023
Merged
jan-justin added a commit to jan-justin/wasmtime that referenced this issue Apr 2, 2023
@jan-justin
cranelift-interpreter: Fix incorrect scalar_to_vector result
1ee3e39 
* The `vectorizelanes` function performs a check to see whether there
is a single value provided in an 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 `vectorizelanes` remains untouched, however, an
unconditional variant `vectorizelanes_all` was added.

* A test was added under `filetests/runtests/issue5911.clif`.

Fixes bytecodealliance#5911
@afonso360 afonso360 linked a pull request Apr 3, 2023 that will close this issue
cranelift-interpreter: Fix incorrect scalar_to_vector result #6133
Merged
jan-justin added a commit to jan-justin/wasmtime that referenced this issue Apr 4, 2023
@jan-justin
cranelift-interpreter: Fix incorrect scalar_to_vector result
4ee45fb 
* The `vectorizelanes` function performs a check to see whether there
is a single value provided in an 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 `vectorizelanes` remains untouched, however, an
unconditional variant `vectorizelanes_all` was added.

* A test was added under `filetests/runtests/issue5911.clif`.

Fixes bytecodealliance#5911
afonso360 pushed a commit that referenced this issue Apr 4, 2023
@jan-justin
cranelift-interpreter: Fix incorrect scalar_to_vector result (#6133)
c475735 
* The `vectorizelanes` function performs a check to see whether there
is a single value provided in an 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 `vectorizelanes` remains untouched, however, an
unconditional variant `vectorizelanes_all` was added.

* A test was added under `filetests/runtests/issue5911.clif`.

Fixes #5911
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Incorrect behavior in the current implementation that needs fixing cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers! cranelift Issues related to the Cranelift code generator good first issue Issues that are good for new contributors to tackle!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cranelift-interpreter: Fix incorrect scalar_to_vector result jan-justin/wasmtime
1 participant
@afonso360