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

build: add basic arm64 support #1028

Merged
merged 1 commit into from
Mar 2, 2015
Merged

Conversation

bnoordhuis
Copy link
Member

This commit adds basic arm64 support to the build. Building the bundled
openssl is disabled pending an upgrade to openssl 1.2. The currently
bundled version has some hand-rolled assembly that is 32 bits only.

The first commit is #1027.

It's not all roses but at least it compiles now.

R=@rvagg, /cc @shigeki @silverwind

@rvagg
Copy link
Member

rvagg commented Mar 2, 2015

Hah! I knew you couldn't resist a challenge if I put that armv8 slave there. Very nice work. Keen to see the openssl-1.2 issues sorted out.

Also, I've been wondering if we should go with the "aarch64" moniker since that appears to be the emerging name for this? "arm64" makes more sense to me but I can't say I've seen that used anywhere else.

@silverwind
Copy link
Contributor

I'm curious, what hardware are you testing this on?

@rvagg
Copy link
Member

rvagg commented Mar 2, 2015

http://www.linaro.org/leg/servercluster/

ARM have been lending a hand, they are keen to help this become a viable platform for ARMv8 servers. See the note in the last update: https://medium.com/node-js-javascript/io-js-week-of-february-17th-9422a589302a

@bnoordhuis
Copy link
Member Author

Also, I've been wondering if we should go with the "aarch64" moniker since that appears to be the emerging name for this? "arm64" makes more sense to me but I can't say I've seen that used anywhere else.

aarch64 is what shows up in the gcc triplet but I think in most other places it's called arm64. Off the top of my head: mainline linux, freebsd, debian, ubuntu and V8 itself, of course.

@rvagg
Copy link
Member

rvagg commented Mar 2, 2015

This from ARM:

ARMv8 architecture which introduces a 64-bit execution mode (known as AARCH64) and maintains backwards compatibility with ARMv7 32-bit code (via a 32-bit execution mode called AARCH32).

@jbergstroem
Copy link
Member

Perhaps note somewhere during configure phase that openssl support won't be build if you don't use a shared library?

@silverwind
Copy link
Contributor

arm64 seems to be the slightly more popular name, while aarch64 is technically more correct. My gut says call it arm64 for less confusion.

@bnoordhuis
Copy link
Member Author

@jbergstroem Added warning, PTAL.

@jbergstroem
Copy link
Member

@bnoordhuis LGTM :shipit:

@bnoordhuis
Copy link
Member Author

@jbergstroem Thanks! @rvagg Can I get a LGTM?

@rvagg
Copy link
Member

rvagg commented Mar 2, 2015

yes, you can get one: LGTM

This commit adds basic arm64 support to the build.  Building the bundled
openssl is disabled pending an upgrade to openssl 1.2, the currently
bundled version has some hand-rolled assembly that is 32 bits only.

PR-URL: nodejs#1028
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@bnoordhuis bnoordhuis closed this Mar 2, 2015
@bnoordhuis bnoordhuis deleted the fix-armv8-build branch March 2, 2015 23:32
@bnoordhuis bnoordhuis merged commit af0a848 into nodejs:v1.x Mar 2, 2015
@rvagg rvagg mentioned this pull request Mar 2, 2015
bnoordhuis added a commit that referenced this pull request Mar 2, 2015
This commit adds basic arm64 support to the build.  Building the bundled
openssl is disabled pending an upgrade to openssl 1.2, the currently
bundled version has some hand-rolled assembly that is 32 bits only.

PR-URL: #1028
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@shigeki
Copy link
Contributor

shigeki commented Mar 3, 2015

I'm okay to build without SSL on arm64 for now, but I'm curious that building with --openssl-no-asm on arm64 in the current iojs also leads to failures? I would like someone to try it because it helps my future work.

rvagg added a commit that referenced this pull request Mar 3, 2015
Notable changes:

* stream: Fixed problems for platforms without `writev()` support,
  particularly Windows. Changes introduced in 1.4.1, via
  #926, broke some
  functionality for these platforms, this has now been addressed.
  #1008 (Fedor Indutny)
* arm: We have the very beginnings of ARMv8 / ARM64 / AARCH64
  support. An upgrade to OpenSSL 1.0.2 is one requirement for full
  support. #1028
  (Ben Noordhuis)
* Add new collaborator: Julian Duque @julianduque
@shigeki
Copy link
Contributor

shigeki commented Mar 3, 2015

@rvagg Please run ci of https://github.com/shigeki/io.js/tree/arm64_no_asm to check if openssl_no_asm works well on arm64 .

@rvagg
Copy link
Member

rvagg commented Mar 3, 2015

@shigeki
Copy link
Contributor

shigeki commented Mar 3, 2015

@rvagg Thanks. Building on arm64 successful but several tests are failed with timeout due to spawnSync . Is this know issue?

@rvagg
Copy link
Member

rvagg commented Mar 3, 2015

@shigeki I don't believe so. See the armv7 machine which is mostly blue these days and I think the armv8 box is faster than the armv7 one.

@shigeki
Copy link
Contributor

shigeki commented Mar 3, 2015

@rvagg test-child-process-spawnsync was failed with TIMEOUT even in the previous job of https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/nodes=iojs-armv8-ubuntu1404/234/console . Several tls tests use openssl-cli command with spawnSync so they all were failed with TIMEOUT. I think there is something wrong with spawnSync on arm64.

@rvagg
Copy link
Member

rvagg commented Mar 3, 2015

that's firmly in @bnoordhuis's wheelhouse then

@shigeki
Copy link
Contributor

shigeki commented Mar 3, 2015

@rvagg I've just submitted a new issue of #1035 .

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

Successfully merging this pull request may close these issues.

5 participants