Summary: | [Wasm] REGRESSION(r256665): Wasm->JS call IC needs to save memory size register | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tadeu Zagallo <tzagallo> | ||||
Component: | JavaScriptCore | Assignee: | Tadeu Zagallo <tzagallo> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, ews-watchlist, keith_miller, Lawrence.j, mark.lam, msaboff, saam, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Tadeu Zagallo
2020-02-17 11:12:05 PST
Created attachment 390944 [details]
Patch
Comment on attachment 390944 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390944&action=review > Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:165 > + // Pessimistically save callee saves in BoundsChecking mode since the LLInt always bounds checks > + return Wasm::PinnedRegisterInfo::get().toSave(Wasm::MemoryMode::BoundsChecking); When would we ever not want to save the sizeRegister? Why not get rid of the memoryMode condition and always save the register? From gripping the code, I don't see a case where toSave() is ever called with anything other than MemoryMode::BoundsChecking. (In reply to Mark Lam from comment #3) > > Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:165 > > + // Pessimistically save callee saves in BoundsChecking mode since the LLInt always bounds checks > > + return Wasm::PinnedRegisterInfo::get().toSave(Wasm::MemoryMode::BoundsChecking); > > When would we ever not want to save the sizeRegister? Why not get rid of > the memoryMode condition and always save the register? From gripping the > code, I don't see a case where toSave() is ever called with anything other > than MemoryMode::BoundsChecking. /gripping/grepping/ Comment on attachment 390944 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390944&action=review r=me >> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:165 >> + return Wasm::PinnedRegisterInfo::get().toSave(Wasm::MemoryMode::BoundsChecking); > > When would we ever not want to save the sizeRegister? Why not get rid of the memoryMode condition and always save the register? From gripping the code, I don't see a case where toSave() is ever called with anything other than MemoryMode::BoundsChecking. I'm wrong, there's still one place where we may not pass MemoryMode::BoundsChecking. *** Bug 207843 has been marked as a duplicate of this bug. *** Comment on attachment 390944 [details] Patch Clearing flags on attachment: 390944 Committed r256766: <https://trac.webkit.org/changeset/256766> All reviewed patches have been landed. Closing bug. |