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

Back-port v8 changes for ppc64, Aix platform (Node 8 backport) #23718

Closed
wants to merge 6 commits into from
Closed

Back-port v8 changes for ppc64, Aix platform (Node 8 backport) #23718

wants to merge 6 commits into from

Conversation

V-for-Vasili
Copy link
Contributor

@V-for-Vasili V-for-Vasili commented Oct 17, 2018 •
edited
Loading

Back-port v8 changes for ppc64, Aix platform
Changes included:

ppc64, aix: Pass CallFrequency object by const reference to avoid value copy error.
Bug: v8:8193
GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61976
V8 Revision: d2e0166ded485df126c765b9196f7edd1ce50f82
Link: https://chromium-review.googlesource.com/c/v8/v8/+/1228633

ppc64, aix: Fixes to V8 GN build process on aix platform
(Only changes to src/base/debug/stack_trace_posix.cc included)
V8 Revision: 6bc4bfea655723b6e71280e71a3dae07c37d7b44
Link: https://chromium-review.googlesource.com/c/v8/v8/+/927484

Vasili Skurydzin added 4 commits October 16, 2018 13:42
deps: cherry-pick 6bc4bfe from V8 upstream
3bee318 
(Only changes to src/base/debug/stack_trace_posix.cc included)

Original commit message:
  Fixes to V8 GN build process on aix platform

  src/base/debug/stack_trace_posix.cc: suppressed unused function warnings
  for functions DemangleSymbols, OutputPointer(in order to compile with
  -Werror flag)

  test/cctest/test-isolate-independent-builtins.cc: corrections to make
  ByteInText test case compatible with aix. (affects aix only)

  Change-Id: I49e45e63545404c77aaed3f51b26557f6f03455e
  Reviewed-on: https://chromium-review.googlesource.com/927484
  Reviewed-by: Jakob Gruber <jgruber@chromium.org>
  Reviewed-by: Michael Achenbach <machenbach@chromium.org>
  Commit-Queue: Jakob Gruber <jgruber@chromium.org>
  Cr-Commit-Position: refs/heads/master@{nodejs#52071}
Fix gyp build on Aix platform. (Changes not currently present in Master)
78db777 
deps/v8/gypfiles/standalone.gypi:
pass -Wno-nonnull-compare, -Wno-address, -Wno-unused-function to gcc on AIX in order
to compile with -Werror flag
deps: cherry-pick d2e0166 from V8 upstream
a8900e6 
  Original commit message:

  ppc64, aix: Pass CallFrequency object by const reference to avoid value copy error.

  Bug: v8:8193
  GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61976

  Change-Id: I0d4efca4da03ef82651325e15ddf2160022bc8de
  Reviewed-on: https://chromium-review.googlesource.com/1228633
  Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
  Reviewed-by: Daniel Clifford <danno@chromium.org>
  Reviewed-by: Junliang Yan <jyan@ca.ibm.com>
  Commit-Queue: Junliang Yan <jyan@ca.ibm.com>
  Cr-Commit-Position: refs/heads/master@{nodejs#56275}
increasing v8 patch level in deps/v8/include/v8-version.h
6289428
@nodejs-github-bot nodejs-github-bot added v8 engine Issues and PRs related to the V8 dependency. v8.x labels Oct 17, 2018
@V-for-Vasili
Copy link
Contributor Author

cc @nodejs/v8 @mhdawson @refack
Please take a look.

Do not pass -Wno-nonnull-compare, -Wno-address, -Wno-unused-function …
152910c 
…to GCC on Aix (not required to build with gcc 4.8.5).
@V-for-Vasili
Copy link
Contributor Author

@mhdawson Removed the unnecessary gcc flags from the PR

Supperss -Wnonnull-compare, -Waddress warnings for deps/v8/testing/gt…
7b71b7c 
…est project;

Suppress unused function warnings in deps/v8/src/compiler/store-store-elimination.cc,
deps/v8/src/conversions.cc in order to compile with newer (>4.8.5) gcc on Aix.
@V-for-Vasili
Copy link
Contributor Author

@mhdawson Uploaded the v8/testing/gtest.gyp patch, PTAL

@mhdawson
Copy link
Member

@V-for-Vasili I think this needs to be update so that we just have the final commits which should land. I believe we should have:

deps: cherry-pick 6bc4bfe from V8 upstream
deps: cherry-pick d2e0166 from V8 upstream
increasing v8 patch level in deps/v8/include/v8-version.h…

and then 1 commit for
Do not pass -Wno-nonnull-compare, -Wno-address, -Wno-unused-function …
Supperss -Wnonnull-compare, -Waddress warnings for deps/v8/testing/gt… …
deps: cherry-pick 6bc4bfe from V8 upstream

My understanding is that because of the checkypicks we'll end up landing as multiple commits and by getting this PR into that shape we'll be able to ensure they land correctly.

Otherwise the final content of all of the commits taken together looks good so I'll approve once the commits are refactored into the right shape. If you want/need any help doing that just let me know.

@V-for-Vasili V-for-Vasili mentioned this pull request Oct 29, 2018
Closed
@V-for-Vasili
Copy link
Contributor Author

@mhdawson done, please take a look: #23958
Closing this one.

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

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@V-for-Vasili @mhdawson @jasnell @nodejs-github-bot