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.

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

Cranelift: srem.8 and srem.i16 not supported #2826

Closed
bjorn3 opened this issue Apr 12, 2021 · 3 comments
Closed

Cranelift: srem.8 and srem.i16 not supported #2826

bjorn3 opened this issue Apr 12, 2021 · 3 comments
Labels
bug Incorrect behavior in the current implementation that needs fixing cranelift Issues related to the Cranelift code generator

Comments

@bjorn3
Copy link
Contributor

bjorn3 commented Apr 12, 2021

.clif Test Case

function u0:42() -> i8, i8 system_v {
block0:
    v0 = iconst.i8 0
    v21 = iconst.i32 0
    v36 = iconst.i8 0
    brz v21, block10
    jump block6

block6:
    trap user0

block10:
    v27 = srem.i8 v0, v36
    trap user0
}

Steps to Reproduce

  • Compile for x86_64

Expected Results

Compiles

Actual Results

thread 'main' panicked at 'assertion failed: dst_size.is_one_of(&[OperandSize::Size32, OperandSize::Size64])', cranelift/codegen/src/isa/x64/inst/mod.rs:666:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/panicking.rs:92:14
   2: core::panicking::panic
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/panicking.rs:50:5
   3: cranelift_codegen::isa::x64::inst::Inst::imm
             at ./codegen/src/isa/x64/inst/mod.rs:666:9
   4: cranelift_codegen::isa::x64::inst::emit::emit
             at ./codegen/src/isa/x64/inst/emit.rs:492:32
   5: <cranelift_codegen::isa::x64::inst::Inst as cranelift_codegen::machinst::MachInstEmit>::emit
             at ./codegen/src/isa/x64/inst/mod.rs:2804:9
   6: cranelift_codegen::machinst::vcode::VCode<I>::emit
             at ./codegen/src/machinst/vcode.rs:531:17
   7: <cranelift_codegen::isa::x64::X64Backend as cranelift_codegen::machinst::MachBackend>::compile_function
             at ./codegen/src/isa/x64/mod.rs:63:22
   8: cranelift_codegen::context::Context::compile
             at ./codegen/src/context.rs:197:26
   9: cranelift_codegen::context::Context::compile_and_emit
             at ./codegen/src/context.rs:137:20

Versions and Environment

Cranelift version or commit: 67cc42d

Operating system: N/A

Architecture: x86_64

Extra Info

Likely introduced in #2763.

I also saw another error when trying to update Cranelift, but I haven't found a standalone repro yet:

thread 'rustc' panicked at 'not implemented: integer store of type: i16', /home/bjorn/.cargo/git/checkouts/wasmtime-41807828cb3a7a7e/67cc42d/cranelift/codegen/src/isa/x64/inst/mod.rs:1134:26
stack backtrace:
   0: rust_begin_unwind
             at /rustc/c051c5ddda79f45fad196ca3a4690251e377d043/library/std/src/panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/c051c5ddda79f45fad196ca3a4690251e377d043/library/core/src/panicking.rs:92:14
   2: cranelift_codegen::isa::x64::inst::Inst::store
   3: cranelift_codegen::machinst::abi_impl::gen_store_stack_multi
   4: <cranelift_codegen::machinst::abi_impl::ABICalleeImpl<M> as cranelift_codegen::machinst::abi::ABICallee>::gen_spill
   5: regalloc::inst_stream::add_spills_reloads_and_moves
   6: regalloc::inst_stream::edit_inst_stream
   7: regalloc::bt_main::alloc_main
   8: regalloc::allocate_registers_with_opts
   9: cranelift_codegen::machinst::compile::compile
  10: <cranelift_codegen::isa::x64::X64Backend as cranelift_codegen::machinst::MachBackend>::compile_function
  11: cranelift_codegen::context::Context::compile
  12: <cranelift_object::backend::ObjectModule as cranelift_module::module::Module>::define_function
  13: rustc_session::utils::<impl rustc_session::session::Session>::time
  14: rustc_codegen_cranelift::base::codegen_fn
  15: rustc_codegen_cranelift::driver::aot::module_codegen
  16: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task
  17: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
  18: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
  19: rustc_session::utils::<impl rustc_session::session::Session>::time
  20: rustc_codegen_cranelift::driver::time
  21: rustc_codegen_cranelift::driver::aot::run_aot
  22: rustc_codegen_cranelift::driver::codegen_crate
  23: <rustc_codegen_cranelift::CraneliftCodegenBackend as rustc_codegen_ssa::traits::backend::CodegenBackend>::codegen_crate
  24: rustc_interface::passes::QueryContext::enter
  25: rustc_interface::queries::Queries::ongoing_codegen
  26: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  27: rustc_span::with_source_map
  28: rustc_interface::interface::create_compiler_and_run
  29: scoped_tls::ScopedKey<T>::set
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

error: internal compiler error: unexpected panic

error: Unrecognized option: 'clif'

Instance { def: Item(WithOptConstParam { did: DefId(0:581 ~ core[8231]::num::dec2flt::rawfp::big_to_fp), const_param_did: None }), substs: [] } _ZN4core3num7dec2flt5rawfp9big_to_fp17h0ff476ec949ff22dE
@bjorn3 bjorn3 added bug Incorrect behavior in the current implementation that needs fixing cranelift Issues related to the Cranelift code generator labels Apr 12, 2021
@cfallin
Copy link
Member

cfallin commented Apr 12, 2021

Thanks for the report! I can take a look at some point, but I'm deep in a bunch of other stuff right now, so if you have some time and are willing to look at the lowering (I suspect it may just be a matter of using a narrower form), I'd be grateful and would be happy to review.

The regalloc issue I think may have come from a recent PR from @abrown that touched Inst::store; @abrown would you be willing to take a look to make sure spills/reloads of narrow values are handled properly?

@abrown
Copy link
Contributor

abrown commented Apr 13, 2021

Yeah, I can take a look.

abrown added a commit to abrown/wasmtime that referenced this issue Apr 13, 2021
Previously, `Inst::store` only understood a subset of the scalar types, which resulted in failures seen in bytecodealliance#2826. This change allows `Inst::store` to generate instructions for all scalar widths (`8 | 16 | 32 | 64`) since all of these are supported in the emission code of `Inst::MovRM`.
@bjorn3
Copy link
Contributor Author

bjorn3 commented Apr 14, 2021

Fixed by #2828 and #2833.

@bjorn3 bjorn3 closed this as completed Apr 14, 2021
mchesser pushed a commit to mchesser/wasmtime that referenced this issue May 24, 2021
Previously, `Inst::store` only understood a subset of the scalar types, which resulted in failures seen in bytecodealliance#2826. This change allows `Inst::store` to generate instructions for all scalar widths (`8 | 16 | 32 | 64`) since all of these are supported in the emission code of `Inst::MovRM`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

No branches or pull requests

3 participants