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: Wrong results in x86_64 and s390x when shifting values of type i16 and i8 #3075

Closed
afonso360 opened this issue Jul 10, 2021 · 1 comment
Labels
bug Incorrect behavior in the current implementation that needs fixing cranelift:area:s390x Issues related to Cranelift's s390x backend cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator

Comments

@afonso360
Copy link
Contributor

Hey, In #3074 we introduce a number of shift tests, some of those found issues with the shift instructions that all backends have.

.clif Test Case

test run
target x86_64 machinst
target s390x

function %ishl_i16_i64(i16, i64) -> i16 {
block0(v0: i16, v1: i64):
    v2 = ishl.i16 v0, v1
    return v2
}
; run: %ishl_i16_i64(0x0000, 0) == 0x0000
; run: %ishl_i16_i64(0x0000, 1) == 0x0000
; run: %ishl_i16_i64(0x000f, 0) == 0x000f
; run: %ishl_i16_i64(0x000f, 4) == 0x00f0
; This fails for the values below
; run: %ishl_i16_i64(0x0004, 16) == 0x0004
; run: %ishl_i16_i64(0x0004, 17) == 0x0008
; run: %ishl_i16_i64(0x0004, 18) == 0x0010

Note, this is a sample CLIF, we found errors for all shifts with destination types of i16 and i8. A full test suite is introduced in #3074, in the files:

  • cranelift/filetests/filetests/runtests/i128-shifts-small-types.clif
  • cranelift/filetests/filetests/runtests/shifts-small-types.clif

Expected Results and Actual Results

For shift amounts larger than the size of the type, a wrapping behaviour should occur, such that i32 << 32 is equivalent to performing a i32 << 0. And this is what happens for i32 and i64 types, however for i16 and i8 this does not happen
and the result is always 0.

Versions and Environment

Cranelift version or commit: main
Architecture: x86_64 and s390x

@afonso360 afonso360 added bug Incorrect behavior in the current implementation that needs fixing cranelift Issues related to the Cranelift code generator labels Jul 10, 2021
@cfallin cfallin added cranelift:area:x64 Issues related to x64 codegen cranelift:area:s390x Issues related to Cranelift's s390x backend labels May 4, 2022
@afonso360
Copy link
Contributor Author

This was fixed in #4752 for x86 and #3702 for s390x.

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:area:s390x Issues related to Cranelift's s390x backend cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

No branches or pull requests

2 participants