[PATCH] D68011: [RISCV] Split SP adjustment to reduce the offset of callee saved register spill and restore

[Shiva Chen via Phabricator via llvm-commits]

shiva0217 added a comment.

In D68011#1683996 <https://reviews.llvm.org/D68011#1683996>, @luismarques wrote:

> In D68011#1683772 <https://reviews.llvm.org/D68011#1683772>, @shiva0217 wrote:
>
> > Update patch
>
>
> @shiva0217 Thanks for updating the patch and addressing the previous concerns. This is looking quite good. I have just a couple more concerns before approving this:
>
> - The boundary condition for doing the two-stage sp adjustment probably needs adjustment. For instance, try this:
>
>   ``` int main() { char xx[2048-16]; foo(xx); } -- addi    sp, sp, -2032 sd      ra, 2024(sp) addi    sp, sp, -16 ... ```
>
>   I think that one would still fit a single-stage sp update. Related to that, where you have the code `if (!isInt<12>(StackSize) && (CSI.size() > 0))`, be sure that the `isInt<12>` check properly accounts for the `StackSize`, `CSI` size, `StackAlign` etc. Which brings me to the second point...
> - Please include tests showing that the boundary condition is correctly computed. For instance, one test where the stack size is just enough not to have to split the sp adjustment and another where the adjustment is needed.
>
>   Thanks!


Hi @luismarques,

Thanks for the boundary test case, I'll add the tests that one will split and the other will not.
The case will split in the prologue because in the epilogue +2048 will not fit in signed 12-bit.
The StackSize will contain the callee saved stack area and the StackSize has been aligned by `determineFrameLayout()`;
The condition `CSI.size() > 0` is to check that there exists a callee saved register spill.
GCC will split in the prologue and the epilogue in this case to simplify the split logic.

  main:
        add     sp,sp,-2032
        sw      ra,2028(sp)
        add     sp,sp,-16
        mv      a0,sp
        call    foo
        add     sp,sp,16
        lw      ra,2028(sp)
        li      a0,0
        add     sp,sp,2032
        jr      ra
        .size   main, .-main
        .ident  "GCC: 

Should we sync the GCC behaviour in this case?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68011/new/

https://reviews.llvm.org/D68011