[PATCH] D97111: [RISCV] change rvv frame layout

luxufan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 20 21:53:19 PST 2021


StephenFan added a comment.

In D97111#2576958 <https://reviews.llvm.org/D97111#2576958>, @rogfer01 wrote:

> Hi @StephenFan. I wonder if we want to do this only when we index via `sp`.
>
> My understanding is that the emergency slot will be close enough when we index via `fp`/`bp` in the previous layout. In the previous layout, when indexing a scalar via `sp` we have to cross the RVV part (which needs an extra register) to reach the scalar registers (where the emergency slot will be located). IIUC, your approach it fixes the `sp` case but complicates the `fp`/`bp` case to access the emergency slot because now we need to jump over RVV.
>
> My suggestion means we want to have the RVV vectors as far from the frame pointer as possible (the one being used `sp`/`fp`/`bp`), so scalars are still straightforward to access even in the presence of RVV. This means different layouts when indexing with `sp` and  when indexing with `fp`/`bp`. In summary: your new layout for `sp` but the previous one for `fp`/`bp`. Does this make sense?

Make sense to me. I didn't consider the case that needs `fp` to access the emergency spill slot. In my patch, It seems that make use of the `bp` register is necessary when variable-sized local variables and rvv stack  objects exist at the same time. Because if uses `fp` to access the emergency spill slot needs an extra register. In the original patch,  make use of the `fp` register is necessary when rvv stack objects exist. I prefer the current frame layout, Because it only needs a `bp` register when variable-sized local variables and rvv stack objects exist. Do you agree it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97111



More information about the llvm-commits mailing list