FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

build: fix V8 build with pointer compression by targos · Pull Request #39664 · nodejs/node · GitHub

/ node Public

build: fix V8 build with pointer compression - #39664

Closed
targos wants to merge 1 commit into
nodejs:masterfrom
targos:fix-pointer-compression
ConversationCommits (1)ChecksFiles changed
Closed

build: fix V8 build with pointer compression#39664
targos wants to merge 1 commit into
nodejs:masterfrom
targos:fix-pointer-compression

Conversation

targos commented Aug 5, 2021

Copy link
Copy Markdown
Member

targos commented Aug 5, 2021

Copy link
Copy Markdown
Member Author

@richardlau @asciidisco

The build succeeds and all tests pass, but it has a lot of warnings like this one:

[2057/3383] CXX obj/deps/v8/src/strings/v8_base_without_compiler.string-stream.o
In file included from ../../deps/v8/src/strings/string-stream.cc:11:
In file included from ../../deps/v8/src/objects/js-array-inl.h:10:
In file included from ../../deps/v8/src/objects/objects-inl.h:22:
In file included from ../../deps/v8/src/heap/heap-write-barrier-inl.h:15:
In file included from ../../deps/v8/src/objects/compressed-slots-inl.h:10:
../../deps/v8/src/common/ptr-compr-inl.h:25:35: warning: requested alignment must be 536870912 bytes or smaller; maximum alignment assumed [-Wbuiltin-assume-aligned-alignment]
  ret = reinterpret_cast<Address>(V8_ASSUME_ALIGNED(
                                  ^~~~~~~~~~~~~~~~~~
../../deps/v8/include/v8config.h:379:3: note: expanded from macro 'V8_ASSUME_ALIGNED'
  __builtin_assume_aligned((ptr), (alignment))
  ^                               ~~~~~~~~~~~
1 warning generated.

targos commented Aug 5, 2021

Copy link
Copy Markdown
Member Author

I'm porting this part of BUILD.gn:

node/deps/v8/BUILD.gn

Lines 615 to 622 in 58316e2

if (v8_enable_pointer_compression) {
enabled_external_v8_defines += [ "V8_COMPRESS_POINTERS" ]
if (v8_enable_pointer_compression_shared_cage) {
enabled_external_v8_defines += [ "V8_COMPRESS_POINTERS_IN_SHARED_CAGE" ]
} else {
enabled_external_v8_defines += [ "V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE" ]
}
}

Setting V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE unconditionally because we haven't ported the v8_enable_pointer_compression_shared_cage option to gyp.

targos added build Issues and PRs related to build files or the CI. dont-land-on-v12.x v8 engine Issues and PRs related to the V8 dependency. labels Aug 5, 2021
nodejs-github-bot added needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Aug 5, 2021
targos mentioned this pull request Aug 5, 2021
Closed
targos added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 5, 2021
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 5, 2021

nodejs-github-bot commented Aug 5, 2021

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Aug 6, 2021 •
edited by richardlau
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/39488/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/4194/

nodejs-github-bot commented Aug 7, 2021

Copy link
Copy Markdown
Collaborator

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

targos added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 7, 2021
github-actions Bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 7, 2021

github-actions Bot commented Aug 7, 2021

Copy link
Copy Markdown
Contributor
Commit Queue failed
- Loading data for nodejs/node/pull/39664
✔  Done loading data for nodejs/node/pull/39664
----------------------------------- PR info ------------------------------------
Title      build: fix V8 build with pointer compression (#39664)
Author     Michaël Zasso  (@targos)
Branch     targos:fix-pointer-compression -> nodejs:master
Labels     build, v8 engine, tools, needs-ci, dont-land-on-v12.x, dont-land-on-v14.x
Commits    1
 - build: fix V8 build with pointer compression
Committers 1
 - Michaël Zasso 
PR-URL: https://github.com/nodejs/node/pull/39664
Reviewed-By: Richard Lau 
Reviewed-By: James M Snell 
Reviewed-By: Colin Ihrig 
Reviewed-By: Mary Marchini 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/39664
Reviewed-By: Richard Lau 
Reviewed-By: James M Snell 
Reviewed-By: Colin Ihrig 
Reviewed-By: Mary Marchini 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 05 Aug 2021 14:31:48 GMT
   ✔  Approvals: 4
   ✔  - Richard Lau (@richardlau): https://github.com/nodejs/node/pull/39664#pullrequestreview-723459767
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/39664#pullrequestreview-723554439
   ✔  - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/39664#pullrequestreview-724757725
   ✔  - Mary Marchini (@mmarchini) (TSC): https://github.com/nodejs/node/pull/39664#pullrequestreview-724801993
   ✖  This PR needs to wait 1 more hours to land
   ✖  Last GitHub CI failed
   ℹ  Last Full PR CI on 2021-08-07T04:30:37Z: https://ci.nodejs.org/job/node-test-pull-request/39492/
   ℹ  Last V8 CI on 2021-08-06T16:30:55Z: https://ci.nodejs.org/job/node-test-commit-v8-linux/4194/
- Querying data for job/node-test-pull-request/39492/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/1108009843

targos added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Aug 7, 2021
github-actions Bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 7, 2021

github-actions Bot commented Aug 7, 2021

Copy link
Copy Markdown
Contributor
Commit Queue failed
- Loading data for nodejs/node/pull/39664
✔  Done loading data for nodejs/node/pull/39664
----------------------------------- PR info ------------------------------------
Title      build: fix V8 build with pointer compression (#39664)
Author     Michaël Zasso  (@targos)
Branch     targos:fix-pointer-compression -> nodejs:master
Labels     build, v8 engine, tools, needs-ci, dont-land-on-v12.x, dont-land-on-v14.x
Commits    1
 - build: fix V8 build with pointer compression
Committers 1
 - Michaël Zasso 
PR-URL: https://github.com/nodejs/node/pull/39664
Reviewed-By: Richard Lau 
Reviewed-By: James M Snell 
Reviewed-By: Colin Ihrig 
Reviewed-By: Mary Marchini 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/39664
Reviewed-By: Richard Lau 
Reviewed-By: James M Snell 
Reviewed-By: Colin Ihrig 
Reviewed-By: Mary Marchini 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 05 Aug 2021 14:31:48 GMT
   ✔  Approvals: 4
   ✔  - Richard Lau (@richardlau): https://github.com/nodejs/node/pull/39664#pullrequestreview-723459767
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/39664#pullrequestreview-723554439
   ✔  - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/39664#pullrequestreview-724757725
   ✔  - Mary Marchini (@mmarchini) (TSC): https://github.com/nodejs/node/pull/39664#pullrequestreview-724801993
   ✖  Last GitHub CI failed
   ℹ  Last Full PR CI on 2021-08-07T13:25:47Z: https://ci.nodejs.org/job/node-test-pull-request/39492/
   ℹ  Last V8 CI on 2021-08-07T13:25:47Z: https://ci.nodejs.org/job/node-test-commit-v8-linux/4194/
- Querying data for job/node-test-pull-request/39492/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/1108229017

targos added a commit that referenced this pull request Aug 7, 2021
build: fix V8 build with pointer compression
df25424 
Refs: nodejs/TSC#790 (comment)

PR-URL: #39664
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>

targos commented Aug 7, 2021

Copy link
Copy Markdown
Member Author

Landed in df25424

targos closed this Aug 7, 2021
targos deleted the fix-pointer-compression branch August 7, 2021 18:27
richardlau mentioned this pull request Aug 13, 2021
Merged
danielleadams pushed a commit that referenced this pull request Aug 16, 2021
build: fix V8 build with pointer compression
cf028df 
Refs: nodejs/TSC#790 (comment)

PR-URL: #39664
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
danielleadams mentioned this pull request Aug 16, 2021
Merged
targos removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 5, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
cjihrig cjihrig approved these changes
richardlau richardlau approved these changes
+1 more reviewer
mmarchini mmarchini approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. 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.

6 participants

Back | FazBrowse Home | New Git URL