Skip to content

Conversation

@bnoordhuis
Copy link
Member

R=@nodejs/v8

I threw in the build: remove -f{data,function}-sections flags commit because otherwise I'd have to rebuild twice. :-) See #6056 for background.

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

@bnoordhuisbnoordhuis added the v8 engine Issues and PRs related to the V8 dependency. label Apr 6, 2016
@trevnorris
Copy link
Contributor

LGTM

We don't link with `--gc-sections` because it's unreliable with some toolchains, so all these flags do is make the compiler generate slightly worse code. Drop them. Refs: nodejs#6056 PR-URL: nodejs#6077 Reviewed-By: Trevor Norris <[email protected]>
@bnoordhuisbnoordhuis deleted the upgrade-v8 branch April 7, 2016 11:16
@bnoordhuisbnoordhuis merged commit 71544c5 into nodejs:masterApr 7, 2016
@MylesBorins
Copy link
Contributor

@bnoordhuis safe to assume this does not make sense for lts?

@bnoordhuis
Copy link
MemberAuthor

71544c5 could be cherry-picked but it's not critical.

@MylesBorins
Copy link
Contributor

@bnoordhuis this is not landing cleanly on v5.x would you be able to backport so we can include this in 5.10.2

MylesBorins pushed a commit that referenced this pull request Apr 19, 2016
We don't link with `--gc-sections` because it's unreliable with some toolchains, so all these flags do is make the compiler generate slightly worse code. Drop them. Refs: #6056 PR-URL: #6077 Reviewed-By: Trevor Norris <[email protected]>
@bnoordhuis
Copy link
MemberAuthor

@thealphanerd When you say 'this', do you mean the V8 upgrade or the other commit?

@MylesBorins
Copy link
Contributor

The v8 upgrade. The other commit landed without problem

@bnoordhuis
Copy link
MemberAuthor

The V8 upgrade shouldn't go into v4 or v5. There are perhaps some individual fixes we could cherry-pick.

MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
We don't link with `--gc-sections` because it's unreliable with some toolchains, so all these flags do is make the compiler generate slightly worse code. Drop them. Refs: #6056 PR-URL: #6077 Reviewed-By: Trevor Norris <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Apr 20, 2016
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
We don't link with `--gc-sections` because it's unreliable with some toolchains, so all these flags do is make the compiler generate slightly worse code. Drop them. Refs: #6056 PR-URL: #6077 Reviewed-By: Trevor Norris <[email protected]>
This was referenced Apr 21, 2016
@MylesBorins
Copy link
Contributor

@bnoordhuis if you think any parts of this should be backported to v4.x-staging could you please open a PR

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@bnoordhuis@trevnorris@MylesBorins