Skip to content

[v19.x] deps: V8: cherry-pick 9ec4e9095a25#47535

Closed
kleisauke wants to merge 1 commit intonodejs:v19.x-stagingfrom
kleisauke:cherrypick-v8-9ec4e9095a25-v19
Closed

[v19.x] deps: V8: cherry-pick 9ec4e9095a25#47535
kleisauke wants to merge 1 commit intonodejs:v19.x-stagingfrom
kleisauke:cherrypick-v8-9ec4e9095a25-v19

Conversation

@kleisauke
Copy link

@kleisauke kleisauke commented Apr 13, 2023

Original commit message:

[turbofan] Fix 32-to-64 bit spill slot moves

Other optimizations can create a situation where it is valid to treat a
stack slot as either 32-bit (which is what its value was created as) or
64-bit value (to which it was implicitly zero-extended). So when moving
such a value to a register, we cannot use a 32-bit move instruction just
because the source was annotated as such; we must also take the target
slot's representation into account.

Fixed: chromium:1407594
Bug: chromium:1356461
Change-Id: I00d850c11a020b055e90f6107b604cdd267d9b6c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4197349
Auto-Submit: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85501}

Refs: v8/v8@9ec4e90
Refs: #46446 (comment)
Refs: #47092 (comment)

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 13, 2023

Review requested:

  • @nodejs/gyp
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v19.x v8 engine Issues and PRs related to the V8 dependency. labels Apr 13, 2023
@kleisauke kleisauke mentioned this pull request Apr 13, 2023
Closed
@kleisauke
Copy link
Author

kleisauke commented Apr 13, 2023

For reference, this was fixed in v18.16.0 with PR #47092. The upcoming v20.0.0 doesn't need this as V8 has been updated to 11.3 (f226350).

@kleisauke kleisauke mentioned this pull request Apr 13, 2023
Closed
5 tasks
@richardlau
Copy link
Member

richardlau commented Apr 13, 2023

I've just landed #47239 on v19.x-staging to fix our V8 CI for Node.js 19.x (nodejs/build#3221). Could you rebase onto the current v19.x-staging to pick up that change, then I'll kick off the CI runs? v8_embedder_string in common.gypi will need to be bumped by 1 as #47239 also bumped it up.

@kleisauke
deps: V8: cherry-pick 9ec4e9095a25
00fd5ee 
Original commit message:

    [turbofan] Fix 32-to-64 bit spill slot moves

    Other optimizations can create a situation where it is valid to treat a
    stack slot as either 32-bit (which is what its value was created as) or
    64-bit value (to which it was implicitly zero-extended). So when moving
    such a value to a register, we cannot use a 32-bit move instruction just
    because the source was annotated as such; we must also take the target
    slot's representation into account.

    Fixed: chromium:1407594
    Bug: chromium:1356461
    Change-Id: I00d850c11a020b055e90f6107b604cdd267d9b6c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4197349
    Auto-Submit: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Maya Lekova <mslekova@chromium.org>
    Commit-Queue: Maya Lekova <mslekova@chromium.org>
    Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#85501}

Refs: v8/v8@9ec4e90
@kleisauke kleisauke force-pushed the cherrypick-v8-9ec4e9095a25-v19 branch from 1b6d976 to 00fd5ee Compare April 13, 2023 12:19
@kleisauke
Copy link
Author

kleisauke commented Apr 13, 2023

Rebased and bumped the v8_embedder_string as requested.

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 13, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 13, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 13, 2023

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 13, 2023

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/5309/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 13, 2023

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/5309/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 13, 2023

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/5309/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 13, 2023

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 13, 2023

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 13, 2023

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

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 14, 2023
@github-actions github-actions bot mentioned this pull request Apr 15, 2023
Open
33 tasks
targos pushed a commit that referenced this pull request May 1, 2023
@kleisauke @targos
deps: V8: cherry-pick 9ec4e9095a25
8283313 
Original commit message:

    [turbofan] Fix 32-to-64 bit spill slot moves

    Other optimizations can create a situation where it is valid to treat a
    stack slot as either 32-bit (which is what its value was created as) or
    64-bit value (to which it was implicitly zero-extended). So when moving
    such a value to a register, we cannot use a 32-bit move instruction just
    because the source was annotated as such; we must also take the target
    slot's representation into account.

    Fixed: chromium:1407594
    Bug: chromium:1356461
    Change-Id: I00d850c11a020b055e90f6107b604cdd267d9b6c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4197349
    Auto-Submit: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Maya Lekova <mslekova@chromium.org>
    Commit-Queue: Maya Lekova <mslekova@chromium.org>
    Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#85501}

Refs: v8/v8@9ec4e90
PR-URL: #47535
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
@targos
Copy link
Member

targos commented May 1, 2023

Landed in 8283313

@targos targos closed this May 1, 2023
@kleisauke kleisauke deleted the cherrypick-v8-9ec4e9095a25-v19 branch May 1, 2023 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gengjiawen gengjiawen gengjiawen approved these changes

@richardlau richardlau richardlau approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. 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.

5 participants

@kleisauke @nodejs-github-bot @richardlau @targos @gengjiawen