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: cannot generate relocation against libcall FloorF64, when has_sse41=false #5563

Closed
zeldovich opened this issue Jan 12, 2023 · 1 comment · Fixed by #5567
Closed
Labels
bug Incorrect behavior in the current implementation that needs fixing cranelift Issues related to the Cranelift code generator wasmtime:platform-support Related to supporting a new platform in Wasmtime

Comments

@zeldovich
Copy link

.wat Test Case

(module
  (type (;0;) (func (param f64) (result f64)))
  (func $f (type 0) (param f64) (result f64)
    local.get 0
    f64.floor))

Steps to Reproduce

  • wasmtime run --wasm-features=-simd --cranelift-set has_sse41=false testcase.wat

Expected Results

I would expect wasmtime / cranelift to be able to compile this code on a machine with has_sse41=false. (In particular, this shows up in a QEMU VM.)

Actual Results

I get an error:

thread 'main' panicked at 'not implemented: cannot generate relocation against libcall FloorF64', crates/cranelift/src/obj.rs:155:21

Versions and Environment

Cranelift version or commit: 4.0.0

Operating system: Linux 6.1.3

Architecture: x86_64; the problem shows up in a QEMU VM where has_sse41=false, but can also be observed outside of a QEMU VM by explicitly passing has_sse41=false like above.

@zeldovich zeldovich added bug Incorrect behavior in the current implementation that needs fixing cranelift Issues related to the Cranelift code generator labels Jan 12, 2023
@alexcrichton alexcrichton added the wasmtime:platform-support Related to supporting a new platform in Wasmtime label Jan 12, 2023
@alexcrichton
Copy link
Member

Thanks for the report! I believe this is a bug where it was mistakenly assumed that SSE4.2 would always be available, but that's only for when simd is enabled, not for the general baseline. This I think used to work in Wasmtime some time ago but I think I broke it with refactorings a year or so ago.

That being said this seems reasonable to me to support, but supporting this will require a bit of careful planning. The main trickiness here is that this would be the first-of-its-kind to be a relocation that needs to be resolved when an object is loaded. Currently Wasmtime has no relocations to support at that time which improves the load time of objects. Supporting this will be somewhat nontrivial but I still think is reasonable to implement.

alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 12, 2023
Long ago Wasmtime used to have logic for resolving relocations
post-compilation for libcalls which I ended up removing during
refactorings last year. As bytecodealliance#5563 points out, however, it's possible to
get Wasmtime to panic by disabling SSE features which forces Cranelift
to use libcalls for some floating-point operations instead. Note that
this also requires disabling SIMD because SIMD support has a baseline of
SSE 4.2.

This commit pulls back the old implementations of various libcalls and
reimplements logic necessary to have them work on CPUs without SSE 4.2

Closes bytecodealliance#5563
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 wasmtime:platform-support Related to supporting a new platform in Wasmtime
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants