[PATCH] D125787: [RISCV] Fix RVV stack frame alignment bugs

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 23 09:52:49 PDT 2022


reames added a comment.

In D125787#3528414 <https://reviews.llvm.org/D125787#3528414>, @reames wrote:

> Thinking about what this code does, I think we've got two pieces:
>
> - Step 1 - Pick a base register (FP, SP, or BP).  There's some legality aspects in this selection (we can't index across either dynamic realign or var sized objects since we don't know their sizes), but it's also a profitability decision.
> - Step 2 - Given choice from 1, compute offset expression.
>
> I'm wondering if it makes sense to separate the code out this way.  If we had a function which only did step 2, then having the asserts to make sure we didn't try to cross var sized objects becomes easy.  It also becomes easy to ensure that e.g. all the SP offset paths stay in sync.
>
> Thoughts?

I prototyped this structure, and all of the tests in tree continued to pass.

Given this change has been LGTMed, this is not a blocker, but I think this code organization does make sense as a post cleanup.

I am not particular confident in the correctness of this code as it stands.  Either the existing code, or this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125787



More information about the llvm-commits mailing list