Skip to content

Rotation shift builtins for small typed integers incorrectly implemented #1492

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

Closed
MaxGraey opened this issue Oct 8, 2020 · 1 comment · Fixed by #1494
Closed

Rotation shift builtins for small typed integers incorrectly implemented #1492

MaxGraey opened this issue Oct 8, 2020 · 1 comment · Fixed by #1494

Comments

@MaxGraey
Copy link
Member

MaxGraey commented Oct 8, 2020

It seems we have wrong polyfills of rotr / rotl for small integers like u16 / u8. Here how it compile now:

function rotL16(a: u16, b: u16): u16 {
  return rotl<u16>(a, b);
}

current result:

(func $rotL16 (param $0 i32) (param $1 i32) (result i32)
  local.get $0
  i32.const 65535
  i32.and
  local.get $1
  i32.rotl
  i32.const 65535
  i32.and
 )

what I expected:

(func $rotL16 (param $0 i32) (param $1 i32) (result i32)
  local.get $0
  local.get $1
  i32.const 15
  i32.and 
  i32.shl 
  local.get $0
  i32.const 0
  local.get $1
  i32.sub 
  i32.const 15
  i32.and 
  i32.shr_u
  i32.or  
  i32.const 65535
  i32.and
)

which equivalent to:

function rotl(a: u16, b: u16): u16 {
  return (a << (b & 15)) | (a >>> (-b & 15));
  // rotr<16>(x, y):
  //   return (x >>> (y & 15)) | (x << (-y & 15));
}
@MaxGraey MaxGraey changed the title Rotation shift builtins for small numbers incorrectly implemented Rotation shift builtins for small integers incorrectly implemented Oct 8, 2020
@MaxGraey MaxGraey changed the title Rotation shift builtins for small integers incorrectly implemented Rotation shift builtins for small typed integers incorrectly implemented Oct 10, 2020
@github-actions
Copy link

🎉 This issue has been resolved in version 0.15.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant