Bug 124721

Summary: ARM64: Implement push/pop equivalents in LLInt
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch none

Michael Saboff
Reported 2013-11-21 09:07:14 PST
ARM64 doesn't allow operation to the stack point that aren't 16 byte aligned. Therefore push and pop need to be changed or abandoned. One possibility is push and pop a pair of registers using the store pair (STP) and load pair (LDP) instructions.
Attachments
Patch (5.57 KB, patch)
2013-11-21 16:56 PST, Michael Saboff
no flags
Geoffrey Garen
Comment 1 2013-11-21 09:29:03 PST
Which push/pop instructions do you have in mind? Are you saying that the stack can't be misaligned even for a moment, such that two 8 byte pushes in a row would crash?
Michael Saboff
Comment 2 2013-11-21 09:41:02 PST
(In reply to comment #1) > Which push/pop instructions do you have in mind? > > Are you saying that the stack can't be misaligned even for a moment, such that two 8 byte pushes in a row would crash? ARM64 doesn't have push or pop instructions. There are pre/post indexed memory ops. The stack does need to be 16 byte aligned. I don't know about whether temporary 8 byte alignment is fine. There are load / store pair instructions which we use in the JSC macro assembler for pushToSave() and popToRestore(). There are LLInt push / pop macro ops. They emit push and pop instructions, so I wondered if the clang assembler mapped those to the appropriate memory ops. Turns out the it doesn't. The current LLInt push / pop are currently unused, wrong and should result in a compile error. When implementing the LLInt callToJavaScript(), I added {push,pop}CalleeSaves macro ops that does the right thing for each platform. I followed after the push / pop macro op implementations. Trying to work through the ARM64 compile issues I found there aren't ARM64 pseudo ops for push / pop. I have reimplemented {push,pop}CalleeSaves using store / load pair and it appears to be working fine.
Michael Saboff
Comment 3 2013-11-21 16:56:59 PST
Michael Saboff
Comment 4 2013-11-21 17:09:32 PST
Michael Saboff
Comment 5 2013-11-21 17:10:03 PST
Reviewed in person by Phil Pizlo.
Geoffrey Garen
Comment 6 2013-11-21 17:11:02 PST
Does stp decrement the stack pointer register? Does ldp increment it? I'm worried that you're just storing/loading the same locations over and over again.
Michael Saboff
Comment 7 2013-11-21 17:24:04 PST
(In reply to comment #6) > Does stp decrement the stack pointer register? Does ldp increment it? I'm worried that you're just storing/loading the same locations over and over again. The stp is the pre-increment variety with an increment of -16. That is what the "[sp, #-16]!" syntax means. The ldp is the post-increment variety with an increment of 16. The syntax for post increment has the increment amount after the closing ']': "[sp], #16". A little confusing that's for sure.
Note You need to log in before you can comment on or make changes to this bug.