Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Add support for building on ARM #25641

Closed
wants to merge 1 commit into from
Closed

Add support for building on ARM #25641

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jul 7, 2015

This patch was sourced from a FreeBSD PR

Source: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=201227
Author: Mikael Urankar

This patch was sourced from a FreeBSD PR

Source: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=201227
Author: Mikael Urankar
@@ -50,6 +50,35 @@ bool CpuFeatures::initialized_ = false;
unsigned CpuFeatures::supported_ = 0;
unsigned CpuFeatures::found_by_runtime_probing_ = 0;

#ifdef __arm__

bool OS::ArmCpuHasFeature(CpuFeature feature) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why defining these functions in the ARM assembler for all OSes? Wouldn't that change break ARM support on platforms other than the one for which it was specifically written?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've screwed up this part of the patch... These changes should go in deps/v8/src/platform-freebsd.cc

@misterdjules
Copy link

@feld Thank you for your interest in running node v0.10 on ARMv6 and FreeBSD. I added a few comments, mostly so that I can understand some of the details in this patch.

Let's see what this patch would look like at the end of the review process. At that time we'll be in a better place to evaluate whether it could land in this repository or should be kept as a patch in FreeBSD's ports.

@misterdjules
Copy link

@mikaelu Thank you very much for the feedback! 👍 Any comment on #25641 (comment) and #25641 (comment)?

@ghost
Copy link
Author

ghost commented Jul 8, 2015

good collaboration guys 👍

@jasnell
Copy link
Member

jasnell commented Aug 27, 2015

@misterdjules ... is this something we're going to land in v0.10?

@rvagg
Copy link
Member

rvagg commented Sep 12, 2015

@nodejs/lts & @nodejs/build I'm inclined to say we shouldn't land this given that v4+ has proper ARM support and adding it back to v0.10 is just adding additional maintenance burden, thoughts?

@jasnell
Copy link
Member

jasnell commented Sep 12, 2015

Agreed
On Sep 11, 2015 5:56 PM, "Rod Vagg" notifications@github.com wrote:

@nodejs/lts https://github.com/orgs/nodejs/teams/lts & @nodejs/build
https://github.com/orgs/nodejs/teams/build I'm inclined to say we
shouldn't land this given that v4+ has proper ARM support and adding it
back to v0.10 is just adding additional maintenance burden, thoughts?


Reply to this email directly or view it on GitHub
#25641 (comment)
.

@cjihrig
Copy link

cjihrig commented Sep 12, 2015

I'd say this isn't appropriate for LTS or maintenance releases.

@rvagg
Copy link
Member

rvagg commented Sep 12, 2015

done

sorry @feld, hopefully you're sorted on v4.x now though

@rvagg rvagg closed this Sep 12, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants