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

src: disable fast math on arm #1398

Merged
merged 2 commits into from
Apr 11, 2015
Merged

Conversation

bnoordhuis
Copy link
Member

The "fast" implementation of Math.exp() and Math.tanh() emits ARMv7
movt/movw instructions on ARMv6 (notably, the original Raspberry Pi.)

Disable fast math for now. The adventurous can enable it again with
the --fast_math switch.

Refs: #1376
V8-Bug: https://code.google.com/p/v8/issues/detail?id=4019

R=@silverwind?

https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/489/

@mscdex mscdex added confirmed-bug Issues with confirmed bugs. v8 engine Issues and PRs related to the V8 dependency. labels Apr 11, 2015
@silverwind
Copy link
Contributor

Compiling

@silverwind
Copy link
Contributor

LGTM, hope this gets fixed soon in v8. Verified it works on my pi.

The "fast" implementation of Math.exp() and Math.tanh() emits ARMv7
movt/movw instructions on ARMv6 (notably, the original Raspberry Pi.)

Disable fast math for now.  The adventurous can enable it again with
the --fast_math switch.

PR-URL: nodejs#1398
Refs: nodejs#1376
Reviewed-By: Roman Reiss <me@silverwind.io>
V8-Bug: https://code.google.com/p/v8/issues/detail?id=4019
Don't disable fast math on ARMv7, only ARMv6.  I hope I got all the
v6 subarchs.

PR-URL: nodejs#1398
Reviewed-By: Roman Reiss <me@silverwind.io>
@bnoordhuis bnoordhuis closed this Apr 11, 2015
@bnoordhuis bnoordhuis deleted the arm-fast-math branch April 11, 2015 19:05
@bnoordhuis bnoordhuis merged commit a4d8847 into nodejs:v1.x Apr 11, 2015
rvagg added a commit that referenced this pull request Apr 14, 2015
Notable changes:

* C++ API: Fedor Indutny contributed a feature to V8 which has been
  backported to the V8 bundled in io.js. SealHandleScope allows a C++
  add-on author to seal a HandleScope to prevent further, unintended
  allocations within it. Currently only enabled for debug builds of
  io.js. This feature helped detect the leak in #1075 and is now
  activated on the root HandleScope in io.js. (Fedor Indutny) #1395.
* ARM: This release includes significant work to improve the state of
  ARM support for builds and tests. The io.js CI cluster's ARMv6,
  ARMv7 and ARMv8 build servers are now all (mostly) reporting passing
  builds and tests.
  - ARMv8 64-bit (AARCH64) is now properly supported, including a
    backported fix in libuv that was mistakenly detecting the
    existence of `epoll_wait()`. (Ben Noordhuis) #1365.
  - ARMv6: #1376 reported a problem with Math.exp() on ARMv6 (incl
    Raspberry Pi). The culprit is erroneous codegen for ARMv6 when
    using the "fast math" feature of V8. --nofast_math has been turned
    on for all ARMv6 variants by default to avoid this, fast math can
    be turned back on with --fast_math. (Ben Noordhuis) #1398.
  - Tests: timeouts have been tuned specifically for slower platforms,
    detected as ARMv6 and ARMv7. (Roman Reiss) #1366.
* npm: Upgrade npm to 2.7.6. See the release notes
  (https://github.com/npm/npm/releases/tag/v2.7.6) for details.
targos added a commit to targos/node that referenced this pull request Aug 31, 2015
Ref: nodejs#1376
Ref: nodejs#1398
Issue fixed in V8: https://chromium.googlesource.com/v8/v8/+/81703350bbb9923d211fe5b79e90bd458b0916e2
V8-Bug: https://code.google.com/p/v8/issues/detail?id=4019

PR-URL: nodejs#2592
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
targos added a commit that referenced this pull request Sep 3, 2015
Ref: #1376
Ref: #1398
Issue fixed in V8: https://chromium.googlesource.com/v8/v8/+/81703350bbb9923d211fe5b79e90bd458b0916e2
V8-Bug: https://code.google.com/p/v8/issues/detail?id=4019

PR-URL: #2592
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
rvagg pushed a commit to rvagg/io.js that referenced this pull request Sep 4, 2015
Ref: nodejs#1376
Ref: nodejs#1398
Issue fixed in V8: https://chromium.googlesource.com/v8/v8/+/81703350bbb9923d211fe5b79e90bd458b0916e2
V8-Bug: https://code.google.com/p/v8/issues/detail?id=4019

PR-URL: nodejs#2592
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
targos added a commit that referenced this pull request Sep 6, 2015
Ref: #1376
Ref: #1398
Issue fixed in V8: https://chromium.googlesource.com/v8/v8/+/81703350bbb9923d211fe5b79e90bd458b0916e2
V8-Bug: https://code.google.com/p/v8/issues/detail?id=4019

PR-URL: #2592
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
targos added a commit that referenced this pull request Sep 6, 2015
Ref: #1376
Ref: #1398
Issue fixed in V8: https://chromium.googlesource.com/v8/v8/+/81703350bbb9923d211fe5b79e90bd458b0916e2
V8-Bug: https://code.google.com/p/v8/issues/detail?id=4019

PR-URL: #2592
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
targos added a commit that referenced this pull request Sep 6, 2015
Ref: #1376
Ref: #1398
Issue fixed in V8: https://chromium.googlesource.com/v8/v8/+/81703350bbb9923d211fe5b79e90bd458b0916e2
V8-Bug: https://code.google.com/p/v8/issues/detail?id=4019

PR-URL: #2592
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants