Skip to content

Conversation

@BridgeAR
Copy link
Member

@BridgeARBridgeAR commented Dec 29, 2018

Original commit message:

[turbofan] Fix -0 check for subnormals. Previously we'd check `x` for -0 by testing `(1.0 / x) == -Infinity`, but this will yield the wrong results when `x` is a subnormal, i.e. really close to 0. In CSA we already perform bit checks to test for -0, so teach TurboFan to do the same for comparisons to -0 (via `Object.is`). We introduce a new NumberIsMinusZero simplified operator to handle the case where SimplifiedLowering already knows that the input is a number. Bug: chromium:903043, v8:6882 Change-Id: I0cb7c568029b461a92fc183104d5f359b4bfe7f4 Reviewed-on: https://chromium-review.googlesource.com/c/1328802 Commit-Queue: Benedikt Meurer <[email protected]> Reviewed-by: Sigurd Schneider <[email protected]> Cr-Commit-Position: refs/heads/master@{#57382} 

Refs: v8/v8@56f6a76

Refs: #25268

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Original commit message: [turbofan] Fix -0 check for subnormals. Previously we'd check `x` for -0 by testing `(1.0 / x) == -Infinity`, but this will yield the wrong results when `x` is a subnormal, i.e. really close to 0. In CSA we already perform bit checks to test for -0, so teach TurboFan to do the same for comparisons to -0 (via `Object.is`). We introduce a new NumberIsMinusZero simplified operator to handle the case where SimplifiedLowering already knows that the input is a number. Bug: chromium:903043, v8:6882 Change-Id: I0cb7c568029b461a92fc183104d5f359b4bfe7f4 Reviewed-on: https://chromium-review.googlesource.com/c/1328802 Commit-Queue: Benedikt Meurer <[email protected]> Reviewed-by: Sigurd Schneider <[email protected]> Cr-Commit-Position: refs/heads/master@{nodejs#57382} Refs: v8/v8@56f6a76
@nodejs-github-botnodejs-github-bot added build Issues and PRs related to build files or the CI. v11.x v8 engine Issues and PRs related to the V8 dependency. labels Dec 29, 2018
@BridgeAR
Copy link
MemberAuthor

Even though the commit lands cleanly, this relies on some other code. I do not have the time to dig into this @nodejs/v8 could you please check what is missing so this compiles on Node.js 11 (V8 7.1)?

@BridgeARBridgeAR mentioned this pull request Jan 16, 2019
4 tasks
@BridgeARBridgeAR deleted the fix-small-numbers branch January 20, 2020 11:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@BridgeAR@nodejs-github-bot