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

[Chad Rosier]

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>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120712/3f38da9a/attachment.html>