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

Luís Marques via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 26 04:51:52 PDT 2019


luismarques added a comment.

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!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68011





More information about the llvm-commits mailing list