Skip to content

Conversation

@dhecht
Copy link
Contributor

@dhecht dhecht commented Nov 11, 2025

af928c6

[JSC] GreedyRegAlloc: remove dead defs of remateralized spilled Tmps
https://bugs.webkit.org/show_bug.cgi?id=302335
rdar://164491860

Reviewed by Yusuke Suzuki.

If a Tmp has only one def and that def is an integer constant, then
the register allocator inserts rematerializations of that constant
at each use rather than loading from the spill slot. However, that
constant def instruction survives as does the store to the
spill slot.

The spill slot store was being removed by the stack slot allocation
pass, however the instruction that moves the constant to register
remains, and we have no pass that removes it.

So, let's add some smarts to the register allocator so that if this
constant def instruction is no longer needed:
1. The original def instruction is removed
2. The unspillable Tmp used to store that constant to the stack is never
   created.
3. The store spill instruction to stack is never inserted.

Points 1 & 2 are new, and 2 can be important as it can reduce register
pressure in situations where we already know we have pressure.

Note that we don't perform this when the Tmp has cold uses since cold uses
are not rematerialized. The cold arg most likely needs to be on the
stack anyway.

Testing: covered by existing JSC stress tests
Canonical link: https://commits.webkit.org/302864@main

388c45f

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win ✅ 🛠 ios-apple
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@dhecht dhecht requested a review from a team as a code owner November 11, 2025 18:20
@dhecht dhecht self-assigned this Nov 11, 2025
@dhecht dhecht added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Nov 11, 2025
@dhecht dhecht added the merge-queue Applied to send a pull request to merge-queue label Nov 11, 2025
https://bugs.webkit.org/show_bug.cgi?id=302335
rdar://164491860

Reviewed by Yusuke Suzuki.

If a Tmp has only one def and that def is an integer constant, then
the register allocator inserts rematerializations of that constant
at each use rather than loading from the spill slot. However, that
constant def instruction survives as does the store to the
spill slot.

The spill slot store was being removed by the stack slot allocation
pass, however the instruction that moves the constant to register
remains, and we have no pass that removes it.

So, let's add some smarts to the register allocator so that if this
constant def instruction is no longer needed:
1. The original def instruction is removed
2. The unspillable Tmp used to store that constant to the stack is never
   created.
3. The store spill instruction to stack is never inserted.

Points 1 & 2 are new, and 2 can be important as it can reduce register
pressure in situations where we already know we have pressure.

Note that we don't perform this when the Tmp has cold uses since cold uses
are not rematerialized. The cold arg most likely needs to be on the
stack anyway.

Testing: covered by existing JSC stress tests
Canonical link: https://commits.webkit.org/302864@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/JSC-GreedyRegAlloc-remove-dead-defs-of-remateralized-spilled-Tmps branch from 388c45f to af928c6 Compare November 11, 2025 21:35
@webkit-commit-queue
Copy link
Collaborator

Committed 302864@main (af928c6): https://commits.webkit.org/302864@main

Reviewed commits have been landed. Closing PR #53747 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit af928c6 into WebKit:main Nov 11, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants