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.

Sign up for GitHub

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

Jump to bottom

conf: linux large pages LLVM lld linkage support #28938

Closed
wants to merge 1 commit into from
Closed

conf: linux large pages LLVM lld linkage support #28938

wants to merge 1 commit into from

Conversation

devnexen
Copy link
Contributor

@devnexen devnexen commented Aug 2, 2019 •
edited
Loading

The custom linker script is compatible with GNU ld only.
As such, providin a new expliciting option to redirect to
a different one. lld seems unable to migrate this
large section w/o segfaulting so providing only the
base address anchor for now.

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. labels Aug 2, 2019
bnoordhuis
bnoordhuis approved these changes Aug 6, 2019
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM but can you keep lines <= 80 columns? In the .gypi you can write it like this:

['OS=="linux" and '
 'target_arch=="x64" and '
 'node_use_large_pages=="true" and '
 'node_use_large_pages_script_lld=="false"', {
  # ...
}

@bnoordhuis
Copy link
Member

@devnexen Are you a collaborator yet? If not, I'd be happy to nominate you.

@devnexen
build: linux large pages LLVM lld linkage support
46e39f8 
The custom linker script is compatible with GNU ld only.
As such, providin a new expliciting option to redirect to
a different one. lld seems unable to migrate this
large section w/o segfaulting so providing only the
base address anchor for now.
@devnexen
Copy link
Contributor Author

devnexen commented Aug 6, 2019

Not yet indeed :)

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/24867/

@Trott
Copy link
Member

Trott commented Aug 20, 2019

Landed in 0d7acfa

@Trott Trott closed this Aug 20, 2019
Trott pushed a commit that referenced this pull request Aug 20, 2019
@devnexen @Trott
build: enable linux large pages LLVM lld linkage support
0d7acfa 
The custom linker script is compatible with GNU ld only.
As such, providin a new expliciting option to redirect to
a different one. lld seems unable to migrate this
large section w/o segfaulting so providing only the
base address anchor for now.

PR-URL: #28938
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos pushed a commit that referenced this pull request Aug 20, 2019
@devnexen @targos
build: enable linux large pages LLVM lld linkage support
f42eb01 
The custom linker script is compatible with GNU ld only.
As such, providin a new expliciting option to redirect to
a different one. lld seems unable to migrate this
large section w/o segfaulting so providing only the
base address anchor for now.

PR-URL: #28938
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@targos targos mentioned this pull request Aug 20, 2019
Merged
This was referenced Aug 20, 2019
Closed
Closed
Closed
Closed
Closed
Closed
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Mar 6, 2020
@devnexen
build: enable linux large pages LLVM lld linkage support
86419d8 
The custom linker script is compatible with GNU ld only.
As such, providin a new expliciting option to redirect to
a different one. lld seems unable to migrate this
large section w/o segfaulting so providing only the
base address anchor for now.

PR-URL: nodejs#28938
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@devnexen @bnoordhuis @nodejs-github-bot @Trott