Add support for vector in DataValueExt::int() by gurry · Pull Request #6844 · bytecodealliance/wasmtime

gurry · 2023-08-14T06:52:50Z

gurry · 2023-08-14T06:54:20Z

DataValueExt::int() doesn't currently seem to have any tests. Please let me know if I should add any tests for this PR or for DataValueExt::int() in general.

Wonder if it would be a good idea to create a test from the .clif in #6827

Issue bytecodealliance#6827

gurry · 2023-08-14T07:32:30Z

Just saw that the test already exists:

wasmtime/cranelift/filetests/filetests/runtests/simd-ineg.clif

Lines 30 to 35 in 30d086f

    
           function %ineg_i64x2(i64x2) -> i64x2 { 
        
           block0(v0: i64x2): 
        
               v1 = ineg v0 
        
               return v1 
        
           } 
        
           ; run: %ineg_i64x2([99 -10]) == [-99 10]

So I've enabled it for the interpreter.

and also improve comment

afonso360

👋 Hey,

This looks great! Thanks for fixing this!

DataValueExt::int() doesn't currently seem to have any tests. Please let me know if I should add any tests for this PR or for DataValueExt::int() in general.

Yeah, we usually test the opcodes via clif tests instead of the internals, so enabling the ineg tests is great!

gurry · 2023-08-14T11:17:12Z

Thanks @afonso360

In order to test with an 8 byte vector input to DataValueExt::int() I tried adding the following test to simd-ineg.clif but it crashed with the below trace:

RUST_BACKTRACE=1 cargo test --test filetests
    Finished test [unoptimized + debuginfo] target(s) in 0.47s
     Running tests/filetests.rs ([...]\wasmtime\target\debug\deps\filetests-d08e13383c5d8a1e.exe)
slow: 0.026 filetests\filetests\wasm\aarch64-relaxed-simd.wat
thread 'worker #2' panicked at 'called `Option::unwrap()` on a `None` value', cranelift\codegen\src\isa\x64\lower\isle.rs:550:21
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library\std\src\panicking.rs:593
   1: core::panicking::panic_fmt
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library\core\src\panicking.rs:67
   2: core::panicking::panic
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library\core\src\panicking.rs:117
   3: enum2$<core::option::Option<cranelift_codegen::isa::x64::inst::args::Gpr> >::unwrap<cranelift_codegen::isa::x64::inst::args::Gpr>
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26\library\core\src\option.rs:935
   4: cranelift_codegen::isa::x64::lower::isle::impl$0::gpr_new
             at .\codegen\src\isa\x64\lower\isle.rs:550
   5: cranelift_codegen::isa::x64::lower::isle::generated_code::constructor_put_in_gpr<cranelift_codegen::machinst::isle::IsleContext<enum2$<cranelift_codegen::isa::x64::lower::isle::generated_code::MInst>,cranelift_codegen::isa::x64::X64Backend> >    
             at C:\Users\gurinder.singh\Projects\Personal\wasmtime_dev\wasmtime\target\debug\build\cranelift-codegen-3e0280ce8237aaeb\out\isle_x64.rs:2610
   6: cranelift_codegen::isa::x64::lower::isle::generated_code::constructor_lower<cranelift_codegen::machinst::isle::IsleContext<enum2$<cranelift_codegen::isa::x64::lower::isle::generated_code::MInst>,cranelift_codegen::isa::x64::X64Backend> >
             at C:\Users\gurinder.singh\Projects\Personal\wasmtime_dev\wasmtime\target\debug\build\cranelift-codegen-3e0280ce8237aaeb\out\isle_x64.rs:16695
   7: cranelift_codegen::isa::x64::lower::isle::lower
             at .\codegen\src\isa\x64\lower\isle.rs:64
   8: cranelift_codegen::isa::x64::lower::impl$1::lower
             at .\codegen\src\isa\x64\lower.rs:325
   9: cranelift_codegen::machinst::lower::Lower<enum2$<cranelift_codegen::isa::x64::lower::isle::generated_code::MInst> >::lower_clif_block<enum2$<cranelift_codegen::isa::x64::lower::isle::generated_code::MInst>,cranelift_codegen::isa::x64::X64Backend>             at .\codegen\src\machinst\lower.rs:743
  10: cranelift_codegen::machinst::lower::Lower<enum2$<cranelift_codegen::isa::x64::lower::isle::generated_code::MInst> >::lower<enum2$<cranelift_codegen::isa::x64::lower::isle::generated_code::MInst>,cranelift_codegen::isa::x64::X64Backend>
             at .\codegen\src\machinst\lower.rs:1067
  11: cranelift_codegen::machinst::compile::compile<cranelift_codegen::isa::x64::X64Backend>
             at .\codegen\src\machinst\compile.rs:40
  12: cranelift_codegen::isa::x64::X64Backend::compile_vcode
             at .\codegen\src\isa\x64\mod.rs:61
  13: cranelift_codegen::isa::x64::impl$1::compile_function
             at .\codegen\src\isa\x64\mod.rs:73
  14: cranelift_codegen::context::Context::compile_stencil
             at .\codegen\src\context.rs:146
  15: cranelift_codegen::context::Context::compile
             at .\codegen\src\context.rs:206
  16: cranelift_jit::backend::impl$2::define_function_with_control_plane
             at .\jit\src\backend.rs:689
  17: cranelift_filetests::function_runner::TestFileCompiler::define_function
             at .\filetests\src\function_runner.rs:248
  18: cranelift_filetests::function_runner::TestFileCompiler::add_functions
             at .\filetests\src\function_runner.rs:148
  19: cranelift_filetests::function_runner::TestFileCompiler::add_testfile
             at .\filetests\src\function_runner.rs:165
  20: cranelift_filetests::test_run::compile_testfile
             at .\filetests\src\test_run.rs:127
  21: cranelift_filetests::test_run::impl$0::run_target
             at .\filetests\src\test_run.rs:197
  22: cranelift_filetests::runone::run
             at .\filetests\src\runone.rs:102
  23: cranelift_filetests::concurrent::worker_thread::closure$0::closure$0
             at .\filetests\src\concurrent.rs:152
  24: std::panicking::try::do_call<cranelift_filetests::concurrent::worker_thread::closure$0::closure_env$0,enum2$<core::result::Result<core::time::Duration,anyhow::Error> > >
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26\library\std\src\panicking.rs:500
  25: std::panicking::try::do_catch<cranelift_filetests::concurrent::worker_thread::closure$0::closure_env$0,enum2$<core::result::Result<core::time::Duration,anyhow::Error> > >
  26: std::panicking::try<enum2$<core::result::Result<core::time::Duration,anyhow::Error> >,cranelift_filetests::concurrent::worker_thread::closure$0::closure_env$0>
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26\library\std\src\panicking.rs:464
  27: std::panic::catch_unwind<cranelift_filetests::concurrent::worker_thread::closure$0::closure_env$0,enum2$<core::result::Result<core::time::Duration,anyhow::Error> > >
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26\library\std\src\panic.rs:142
  28: cranelift_filetests::concurrent::worker_thread::closure$0
             at .\filetests\src\concurrent.rs:152
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
FAIL filetests\filetests\runtests\simd-ineg.clif: panicked in worker #2: called `Option::unwrap()` on a `None` value
1355 tests
Error: 1 failure
error: test failed, to rerun pass `--test filetests`

While the interpreter worked fine, it was the compiler that crashed. Is this expected?

afonso360 · 2023-08-14T11:24:28Z

While the interpreter worked fine, it was the compiler that crashed. Is this expected?

Well, sort of. Ideally we would be able to compile all clif code, but 64 bit SIMD support is very incomplete both in the backends and in the interpreter itself. The only architectures that support it are AArch64 and RISC-V (And neither of those fully support it!).

I think X86_64 has never supported it, so yeah I would expect it to crash there.

gurry · 2023-08-14T11:28:40Z

Okay, makes sense. Thanks 👍

* Add support for vector in DataValueExt::int() Fixes bytecodealliance#6827 * Replace `if` with `match` * Enable interpreter test for simd ineg Issue bytecodealliance#6827 * Format code with `cargo fmt` and also improve comment

…time into feature/wasi-nn-preview-2 * 'feature/wasi-nn-preview-2' of github.com:geekbeast/wasmtime: Memcheck for Wasm guests in Wasmtime (bytecodealliance#6820) CI: upgrade to qemu 8.0.4. (bytecodealliance#6849) Sync wasi-cli with wit definitions in standards repo (bytecodealliance#6806) Rename `preview2::preview2` to `preview2::host` (bytecodealliance#6847) winch: Simplify the MacroAssembler and Assembler interfaces (bytecodealliance#6841) There are no files in `preview1` other than `mod.rs` (bytecodealliance#6845) Update stdio on Unix to fall back to worker threads (bytecodealliance#6833) Update RELEASES.md (bytecodealliance#6838) Minor documentation updates to docs/WASI-tutorial.md (bytecodealliance#6839) Add support for vector in DataValueExt::int() (bytecodealliance#6844)

Add support for vector in DataValueExt::int()

9ade5b5

Fixes bytecodealliance#6827

gurry requested a review from a team as a code owner August 14, 2023 06:52

gurry requested review from cfallin and removed request for a team August 14, 2023 06:52

gurry added 2 commits August 14, 2023 12:40

Replace if with match

339aaa4

Enable interpreter test for simd ineg

8c2b76c

Issue bytecodealliance#6827

Format code with cargo fmt

5fadae7

and also improve comment

github-actions bot added the cranelift Issues related to the Cranelift code generator label Aug 14, 2023

afonso360 approved these changes Aug 14, 2023

View reviewed changes

afonso360 added this pull request to the merge queue Aug 14, 2023

Merged via the queue into bytecodealliance:main with commit 76f1cb2 Aug 14, 2023

gurry deleted the issue-6827 branch August 16, 2023 02:27

afonso360 mentioned this pull request Aug 31, 2023

cranelift-interpreter: Fix SIMD shifts and rotates #6939

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for vector in DataValueExt::int()#6844

Add support for vector in DataValueExt::int()#6844
afonso360 merged 4 commits intobytecodealliance:mainfrom
gurry:issue-6827

gurry commented Aug 14, 2023 •

edited

Loading

Uh oh!

gurry commented Aug 14, 2023 •

edited

Loading

Uh oh!

gurry commented Aug 14, 2023 •

edited

Loading

Uh oh!

afonso360 left a comment •

edited

Loading

Uh oh!

gurry commented Aug 14, 2023

Uh oh!

afonso360 commented Aug 14, 2023 •

edited

Loading

Uh oh!

gurry commented Aug 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

gurry commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gurry commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gurry commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afonso360 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gurry commented Aug 14, 2023

Uh oh!

afonso360 commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gurry commented Aug 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gurry commented Aug 14, 2023 •

edited

Loading

gurry commented Aug 14, 2023 •

edited

Loading

gurry commented Aug 14, 2023 •

edited

Loading

afonso360 left a comment •

edited

Loading

afonso360 commented Aug 14, 2023 •

edited

Loading