[PATCH] D40805: [RISCV] Support for varargs

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:701
+    // If saving an odd-number of registers, create an extra stack slot to
+    // ensure even-numbered registers are always 2*XLEN-aligned.
+    if (Idx % 2) {
----------------
asb wrote:
> xiangzhai wrote:
> > efriedma wrote:
> > > xiangzhai wrote:
> > > > efriedma wrote:
> > > > > Could you clarify what "ensure even-numbered registers are always 2*XLEN-aligned" means?  The varargs save area is always right next to any arguments passed on the stack, so I'm not sure what exactly you need to align.
> > > > Hi Eli,
> > > > 
> > > > I am reviewing [Compiler Principle](https://en.wikipedia.org/wiki/Compilers:_Principles,_Techniques,_and_Tools) chapter 7.2 Stack Allocation, then consider about this question, because Strict Alignment? just like GCC https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00011.html Please teach me, thanks a lot!
> > > > 
> > > > Regards,
> > > > Leslie Zhai 
> > > Yes, in general, some things need to be aligned, for a variety of reasons (including the issue you noted).  I'm not questioning that part.
> > > 
> > > The question is more about the stack layout.  Why do we need to allocate an extra object here to ensure alignment?  What stack objects are we ensuring the alignment of?
> > Perhaps I need to review Compiler Principle's chapter 7.2.4 `VarArg in stack` and 7.3.5 Access Link more carefully :)
> This is a requirement of the calling convention, see [here](https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#integer-calling-convention). I believe the intention is to ensure that when the vararg registers are written to the stack they are done so with correct alignment, meaning va_arg can assume correct alignment regardless of whether a parameter was originally passed in an argument register or the stack. Assigning 'aligned' registers is no use if a2 is unaligned on the stack.
I'm still not following; even-numbered registers are always 2*XLEN-aligned because the vararg save area must be adjacent to arguments passed on the stack. Like so:

| stack arg
| stack arg (XLEN*2 aligned)
| saved a7
| saved a6 (XLEN*2 aligned)
| saved a5
| saved a4 (XLEN*2 aligned)
| saved a3
| saved a2 (XLEN*2 aligned)
| saved a1
| saved a0 (XLEN*2 aligned)

This is the one and only layout, no matter how many argregs we save.  If some of these arguments are fixed, you can skip saving them (and use the space for other stack objects, or just let the space extend beyond the stack pointer), but the layout doesn't fundamentally change.  For example, if we save five registers (because there are three fixed registers), the layout is something like this:

| stack arg
| stack arg (XLEN*2 aligned)
| saved a7
| saved a6 (XLEN*2 aligned)
| saved a5
| saved a4 (XLEN*2 aligned)
| saved a3 (va_start points here)
| PADDING  (XLEN*2 aligned) (frame pointer points here?)
| (unused space)
| (unused space) (XLEN*2 aligned)

Assuming my model is right, what purpose does the padding serve?  The only effect I can see is that it changes the alignment of the frame pointer (which the comment doesn't mention at all).


https://reviews.llvm.org/D40805