[llvm-commits] PATCH: Change X86 function prologue/epilogue in case the stack is realigned

Alexey Samsonov samsonov at google.com
Thu Jul 12 12:54:50 PDT 2012


Thanks for your comments! I'll fix the patch and sent the updated version
tomorrow.

I also don't know a case where pushing the registers before the stack
alignment may cause any troubles (I just checked that gcc sometimes behaves
this way, but it is not Windows, and no xmm registers were involved there).
Do you think that we may get away with documenting
this concern in comments?

On Thu, Jul 12, 2012 at 10:04 PM, Chad Rosier <mcrosier at apple.com> wrote:

> Alexey,
> Is there a case where we would ever need the stack realigned before the
> pushes?  This came to mind prior to my comments below, but I didn't think
> it was possible.  Then Eric dropped by my office and made a similar
> comment.  That got me wondering, so I briefly chatted with Jakob.  He
> pointed out that in Windows there are vector callee-saved registers (xmm
> regs).  The only issue would be if these need to be saved and the alignment
> is below 16 bytes.  We weren't sure if this could ever happen, but just as
> a precaution I though I would comment.
>


>
>  Chad
>
>
> On Jul 12, 2012, at 9:53 AM, Chad Rosier wrote:
>
>
> On Jul 12, 2012, at 9:02 AM, Alexey Samsonov wrote:
>
> Hi!
>
> This patch is intended to fix the exception handling in case of realigned
> stack (see more details in recent discussion with mcrosier@ on r160002).
>
>
> This looks to be a better solution then the prior one we discussed, so I'm
> all for it.
> In fact, it will improve the performance in the case of stack realignment
> + VLAs because we don't restore the SP twice.
>
> Before this change, the prologue and epilogue looks like this:
> push %rbp
> mov %rsp, %rbp
> and $alignment %rsp   <--- align stack
> push %reg1   <----- save regs on stack
> push %reg2
> sub $size %rsp
> <...>
> add $size %rsp
> pop %reg2  <------- restore regs
> pop %reg1
> mov %rbp, %rsp <--- restore rsp
> pop %rbp
>
> The problem is: the register values are incorrectly restored in exception
> handling - emiited call frame information gives offsets of registers from
> frame pointer (%rbp),
> which is wrong, as their actual locations depend on the alignment result.
>
> I suggest to modify the prologue in the following way:
> push %rbp
> mov %rsp, %rbp
> push %reg1   <----- save regs on stack
> push %reg2
> and $alignment %rsp   <--- align stack
> sub $size %rsp
> <...>
> mov %rbp, %rsp
> sub $total_size_of_saved_regs, %rsp
> pop %reg2  <------- restore regs
> pop %reg1
> pop %rbp
>
> In this way the offsets from %rbp will be valid, and the EH won't be
> broken. (you may also see the emiited asm in the tests).
>
>
> Nice!
>
> Just a few comments on the patch:
> 1. For the new test case, pr11468.ll, would you mind adding CHECKs for the
> epilogue?  It just gives a more complete picture of what's going on.
>
> 2. In dynamic-allocas-VLAs.ll
>
> Because this is being removed from t4:
> -; CHECK: addq $[[STACKADJ]], %rsp
>
> Would you mind simplifying the earlier check:
>  ; CHECK: subq $[[STACKADJ:[0-9]+]], %rsp
>
> to this:
>  ; CHECK: subq ${{[0-9]+}}, %rsp
>
> And for this CHECK:
> +; CHECK: addq ${{[0-9]*}}, %rsp ## imm = 0xFFFFFFF0
>
> Use a + instead of a *; I assume we should see at least one digit.  Also,
> remove the ## comment unless you feel it's informative.
>
> The above applies to the t7 test case as well.
>
> And for t9 please removed the ## comments unless you feel it's informative.
>
> Looks good otherwise.
>
>  Chad
>
> Code review: http://codereview.appspot.com/6352105/
>
> --
> Alexey Samsonov, MSK
>
> <zdiff.pr11468>
>
>
>
>


-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120712/559ebd42/attachment.html>


More information about the llvm-commits mailing list