Stream: git-wasmtime

Topic: wasmtime / issue #5466 Cranelift: `iabs` not implemented ...

view this post on Zulip Wasmtime GitHub notifications bot (Dec 18 2022 at 12:18):

afonso360 labeled issue #5466:

:wave: Hey

.clif Test Case

test interpret
test run
target x86_64

function %iabs_i32(i32) -> i32 {
block0(v0: i32):
    v1 = iabs.i32 v0
    return v1
}
; run: %iabs_i32(1) == 1
; run: %iabs_i32(-1) == 1

Steps to Reproduce

Expected Results

The test to pass

Actual Results

The x86 backend has no lowerings for scalar types. The above test case is only for i32, but the same happens for other scalar types.

thread 'worker #0' panicked at 'should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`', cranelift/codegen/src/machinst/lower.rs:753:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ERROR cranelift_filetests::concurrent > FAIL: panicked in worker #0: should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`
FAIL ./lmao.clif: panicked in worker #0: should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`
1 tests
Error: 1 failure

Versions and Environment

Cranelift version or commit: main
Operating system: linux
Architecture: x86_64

view this post on Zulip Wasmtime GitHub notifications bot (Dec 18 2022 at 12:18):

afonso360 opened issue #5466:

:wave: Hey

.clif Test Case

test interpret
test run
target x86_64

function %iabs_i32(i32) -> i32 {
block0(v0: i32):
    v1 = iabs.i32 v0
    return v1
}
; run: %iabs_i32(1) == 1
; run: %iabs_i32(-1) == 1

Steps to Reproduce

Expected Results

The test to pass

Actual Results

The x86 backend has no lowerings for scalar types. The above test case is only for i32, but the same happens for other scalar types.

thread 'worker #0' panicked at 'should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`', cranelift/codegen/src/machinst/lower.rs:753:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ERROR cranelift_filetests::concurrent > FAIL: panicked in worker #0: should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`
FAIL ./lmao.clif: panicked in worker #0: should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`
1 tests
Error: 1 failure

Versions and Environment

Cranelift version or commit: main
Operating system: linux
Architecture: x86_64

view this post on Zulip Wasmtime GitHub notifications bot (Dec 18 2022 at 12:18):

afonso360 labeled issue #5466:

:wave: Hey

.clif Test Case

test interpret
test run
target x86_64

function %iabs_i32(i32) -> i32 {
block0(v0: i32):
    v1 = iabs.i32 v0
    return v1
}
; run: %iabs_i32(1) == 1
; run: %iabs_i32(-1) == 1

Steps to Reproduce

Expected Results

The test to pass

Actual Results

The x86 backend has no lowerings for scalar types. The above test case is only for i32, but the same happens for other scalar types.

thread 'worker #0' panicked at 'should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`', cranelift/codegen/src/machinst/lower.rs:753:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ERROR cranelift_filetests::concurrent > FAIL: panicked in worker #0: should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`
FAIL ./lmao.clif: panicked in worker #0: should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`
1 tests
Error: 1 failure

Versions and Environment

Cranelift version or commit: main
Operating system: linux
Architecture: x86_64

view this post on Zulip Wasmtime GitHub notifications bot (Dec 19 2022 at 20:27):

jameysharp commented on issue #5466:

@cfallin, @elliottt, and I discussed today which way we should fix this: by removing scalar types from the definition of iabs; or by legalizing iabs away before the backends; or by implementing this instruction on all backends.

We think that iabs ought to be implemented for all scalar types on all backends.

It's already implemented for scalar types on aarch64 (except I128: #5467) with a pair of instructions (compare and conditional negation: see decl abs in cranelift/codegen/src/isa/aarch64/inst.isle).

A similar lowering should work on x86, with a negate and a conditional move. The neg instruction sets the sign flag if the result is negative, so I think we get the right result with a cmove that takes the original value on CC.S or the result of negation otherwise. I believe register allocation will insert an additional mov instruction first to copy the original value, so it'll be three instructions total.

I can't decide whether this is a "good first issue" or not. It should be a small patch but the details of how we handle flags might take some explaining. I'm going to label it that way and see if somebody picks it up...

view this post on Zulip Wasmtime GitHub notifications bot (Dec 19 2022 at 20:27):

jameysharp labeled issue #5466:

:wave: Hey

.clif Test Case

test interpret
test run
target x86_64

function %iabs_i32(i32) -> i32 {
block0(v0: i32):
    v1 = iabs.i32 v0
    return v1
}
; run: %iabs_i32(1) == 1
; run: %iabs_i32(-1) == 1

Steps to Reproduce

Expected Results

The test to pass

Actual Results

The x86 backend has no lowerings for scalar types. The above test case is only for i32, but the same happens for other scalar types.

thread 'worker #0' panicked at 'should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`', cranelift/codegen/src/machinst/lower.rs:753:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ERROR cranelift_filetests::concurrent > FAIL: panicked in worker #0: should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`
FAIL ./lmao.clif: panicked in worker #0: should be implemented in ISLE: inst = `v1 = iabs.i32 v0`, type = `Some(types::I32)`
1 tests
Error: 1 failure

Versions and Environment

Cranelift version or commit: main
Operating system: linux
Architecture: x86_64

view this post on Zulip Wasmtime GitHub notifications bot (Jan 04 2023 at 13:52):

maekawatoshiki commented on issue #5466:

Hi, can I work on this issue? I'm new to contributing to Cranelift and looking for good first issues.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 04 2023 at 16:59):

jameysharp commented on issue #5466:

Welcome! Go ahead, and let us know if you have any questions!

Last updated: Nov 22 2024 at 16:03 UTC