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

i8Portable is not handle all cases and overcomplicated #221

Closed
MaxGraey opened this issue Dec 22, 2018 · 0 comments · Fixed by #264
Closed

i8Portable is not handle all cases and overcomplicated #221

MaxGraey opened this issue Dec 22, 2018 · 0 comments · Fixed by #264
Assignees
Labels
Core/Wasm Issues concerning the core library of Wasmboy enhancement New feature or request

Comments

@MaxGraey
Copy link
Contributor

I suggest replace i8Portable implementation which currently looks like this:

function checkBitOnByte(bitPosition, byte) {
  return (byte & (1 << bitPosition)) != 0;
}

function i8Portable(param) {
  // JS ints are all i32, therefore, get the sign bit, and then convert accordingly
  // Example: https://blog.michaelyin.info/convert-8bit-byte-to-signed-int/
  let response = param;
  if (checkBitOnByte(7, response)) {
    response = (256 - param) * -1;
  }

  return response;
}

And not handle this case for example:

i8Portable(260) // return 260 but should 4

To this:

function i8Portable(param) {
  return param << 24 >> 24;
}
@MaxGraey MaxGraey changed the title i8Portable is not handle all cases and overcompleacated i8Portable is not handle all cases and overcomplicated Jan 3, 2019
torch2424 added a commit that referenced this issue Feb 21, 2019
@torch2424 torch2424 self-assigned this Feb 21, 2019
@torch2424 torch2424 added enhancement New feature or request Core/Wasm Issues concerning the core library of Wasmboy labels Feb 21, 2019
torch2424 added a commit that referenced this issue Feb 22, 2019
* Implemented all changes from #230 and #221

* Fixed off by one mentioned in #216

* Wrapped modulo in i32Portable as mentioned in #216

* issue #207, Allowed making closure builds for debugging, and then tried
to match its inlining

* Removed the legacy api

* Updated the package-lock for the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core/Wasm Issues concerning the core library of Wasmboy enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants