BBQJIT's ScratchScope should always lock preseverved registers #14941
Conversation
This file contains 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
|
EWS run on previous version of this PR (hash 252f393) |
webkit-ews-buildbot
added
the
merging-blocked
kmiller68
removed
the
merging-blocked
kmiller68
force-pushed
the
eng/BBQJITs-ScratchScope-should-always-lock-preseverved-registers
branch
from
252f393
to
7cbccc1
Compare
|
EWS run on previous version of this PR (hash 7cbccc1) |
kmiller68
commented
Jun 14, 2023
Comment on lines
10065
to
10083
| m_generator.m_fprLRU.lock(reg); | ||
| binding = RegisterBinding::scratch(); |
There was a problem hiding this comment.
I'll keep the old order before committing.
webkit-ews-buildbot
added
the
merging-blocked
kmiller68
removed
the
merging-blocked
kmiller68
force-pushed
the
eng/BBQJITs-ScratchScope-should-always-lock-preseverved-registers
branch
from
7cbccc1
to
2364132
Compare
|
EWS run on current version of this PR (hash 2364132) |
https://bugs.webkit.org/show_bug.cgi?id=258044 Reviewed by Yusuke Suzuki. Right now if a preserved location/register is already bound to a Location then we don't lock its slot. By not locking however, when we go to allocate any requested scratches we could choose one of the preserved registers to evict. This patch fixes this by always locking the preserved locations. Additionally, some places where we were using ScratchScope to lock nonArgumentNonPreservedGPR1 can now use ScratchScope to get a non-argument non-preserved register. This is done by preserving all argument GPRs then immediately releasing them via a new unbindPreserved method. * Source/JavaScriptCore/wasm/WasmBBQJIT.cpp: (JSC::Wasm::BBQJIT::addIf): (JSC::Wasm::BBQJIT::emitIndirectCall): (JSC::Wasm::BBQJIT::addCallIndirect): (JSC::Wasm::BBQJIT::addCallRef): (JSC::Wasm::BBQJIT::ScratchScope::ScratchScope): (JSC::Wasm::BBQJIT::ScratchScope::~ScratchScope): (JSC::Wasm::BBQJIT::ScratchScope::unbindEarly): (JSC::Wasm::BBQJIT::ScratchScope::unbindScratches): (JSC::Wasm::BBQJIT::ScratchScope::unbindPreserved): (JSC::Wasm::BBQJIT::ScratchScope::gpr const): (JSC::Wasm::BBQJIT::ScratchScope::fpr const): (JSC::Wasm::BBQJIT::ScratchScope::unbind): Deleted. Canonical link: https://commits.webkit.org/265159@main
webkit-commit-queue
force-pushed
the
eng/BBQJITs-ScratchScope-should-always-lock-preseverved-registers
branch
from
2364132
to
3465968
Compare
|
Committed 265159@main (3465968): https://commits.webkit.org/265159@main Reviewed commits have been landed. Closing PR #14941 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
3465968
2364132
🛠 wincairo🧪 gtk-wk2🛠 🧪 jsc🧪 api-ios🧪 mac-wk2🛠 tv🧪 mac-AS-debug-wk2🛠 tv-sim🧪 jsc-armv7-tests🛠 watch