[PATCH] D98802: [RISCV][WIP] Fix offset computation for RVV

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:882
+  if (RVVStackSize && !hasFP(MF) && Size % 8 != 0) {
+    RVFI->setRVVPadding(getStackAlign().value());
+  }
----------------
StephenFan wrote:
> StephenFan wrote:
> > If we can set the RVVPadding as 
> > ```
> > RVFI->setRVVPadding(alignTo(Size, getStackAlign().value()) -Size);
> > ```
> > or just align to 8, because we just want the RVV stack aligned to 8 bytes.
> > ```
> > RVFI->setRVVPadding(alignTo(Size, 8) - Size);
> > ```
> > ?
> > In this way, maybe we can save some stack space.
> If we can set the RVVPadding as 
> ```
> RVFI->setRVVPadding(alignTo(Size, getStackAlign().value()) -Size);
> ```
> or just align to 8, because we just want the RVV stack aligned to 8 bytes.
> ```
> RVFI->setRVVPadding(alignTo(Size, 8) - Size);
> ```
> ?
> In this way, maybe we can save some stack space.
> If we can set the RVVPadding as
>
> RVFI->setRVVPadding(alignTo(Size, getStackAlign().value()) -Size);

This is more compact, I'll use that, thanks!.

> or just align to 8, because we just want the RVV stack aligned to 8 bytes.

Unless I do something different with the RVVPadding I think I need to use `getStackAlign()`.

Consider RV32 and a single CSR: `Size` will be 4 so `alignTo(Size, 8) - Size` is 4. `StackSize` before adding `RVVPadding` is already aligned to `getStackAlign()` (that value is commonly 16, even in RV32). If I add 4 bytes to the StackSize then I can align the RVV to 8 bytes but `sp` won't be aligned to `getStackAlign()` anymore. Maybe I'm missing something?


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

https://reviews.llvm.org/D98802