[ARM64] Disallowed scratch register usage when allocating stack for large frame sizes in B3::Air::generate()
When generating entrypoints for specific blocks in B3::Air::generate(), the necessary stack is allocated by subtracting the frame size value from the stack pointer register. http://trac.webkit.org/browser/webkit/trunk/Source/JavaScriptCore/b3/air/AirGenerate.cpp#L214 The titular problem occurs when the frame size is 4096 or larger, meaning it can't fit into a 12-bit immediate that can still be packed into a single add or sub instruction. In that case MacroAssembler64 tries to load the immediate into the data temp register, but triggers the m_allowScratchRegister assert since that's disallowed for the whole scope of B3::Air::generate(). http://trac.webkit.org/browser/webkit/trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h#L236
Created attachment 308845 [details] Patch
Comment on attachment 308845 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308845&action=review > Source/JavaScriptCore/b3/air/AirGenerate.cpp:221 > + AllowMacroScratchRegisterUsage allowScratch(jit); JF is doing the same thing in: https://bugs.webkit.org/show_bug.cgi?id=170215 I'm ok w/ having this be its own patch. A few comments: - As I suggested to JF, I think we should use AllowMacroScratchRegisterUsageIf allowScratch(isArm64()); - I think it's worth asserting that the ScratchRegister is not pinned, otherwise, this code would be wrong.
> - I think it's worth asserting that the ScratchRegister is not pinned, > otherwise, this code would be wrong. My patch does pin that register.
(In reply to JF Bastien from comment #4) > > - I think it's worth asserting that the ScratchRegister is not pinned, > > otherwise, this code would be wrong. > > My patch does pin that register. I feel like we kind of mean something different by your use of pinning vs the Wasm use of pinning. Perhaps it's not an interesting distinction though. What confuses me about our semantics of pinning is this: - When something claims to writePinned, we must execute it. We trust that it's updating the register as it sees fit. - When something clobbers a register, that means if we care about the value inside the register, we must take action to save its value across the thing clobbering it. What are the semantics when something claims to clobber a pinned register? Somebody who understands the semantics better I'm sure can explain to me what the expectation is in this scenario.
I'm fine with this being fixed by JF in bug #170215. Resolving as a duplicate of that bug. *** This bug has been marked as a duplicate of bug 170215 ***