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

Mon May 23 23:20:35 PDT 2022

frasercrmck added a comment.

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

> A couple of themes, but nothing actionable.
>
> Theme 1 - We have confirmed we have untested code.  We know this code used to differ between two different cases of using SP as the base register.  Its not clear if this is correct, or not.  This implies both lack of test coverage, and (probably) lack of understanding.
>
> Theme 2 - This is very complicated code, and the changes are broad enough to be hard to track.
>
> I am not objecting to this landing.  It's probably more correct than what's in tree.  I'm just saying that even with this, we probably have uncaught issues here.  (Or at least, the code structure doesn't convince me that we *don't*.)  I am a strong proponent of reducing code complexity until it can't be simplified further as a means to flesh out bugs.

Yeah, good points. I hope we can do further work on this code to simplify it and increase readability so we can reduce the barrier to entry somewhat. I don't have any plans for Theme 2, really, but for 1) I am going to root out the untested code and work out whether it's a lack of coverage or whether it's logically impossible to reach.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125787