[PATCH] D70401: [WIP][RISCV] Implement ilp32e ABI

[Sam Elliott via Phabricator via cfe-commits]

lenary marked 3 inline comments as done.
lenary added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1525
   unsigned TwoXLenInBytes = (2 * XLen) / 8;
   if (!IsFixed && ArgFlags.getOrigAlign() == TwoXLenInBytes &&
       DL.getTypeAllocSize(OrigTy) == TwoXLenInBytes) {
----------------
shiva0217 wrote:
> lenary wrote:
> > shiva0217 wrote:
> > > The variadic argument for ilp32e doesn't need to align to even register. We could also add a test line in vararg.ll.
> > I'm not sure I agree with this interpretation of the psABI. The [[ https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#ilp32e-calling-convention | ILP32E Section ]] makes no exception for variadic arguments, and the base calling convention is only defined in relation to `XLEN`, not in terms of stack alignment.
> > 
> > I will add a test to `vararg.ll` so the behaviour is at least tested. 
> It seems to be the current GCC behavior and the following case could observe that double will not align to even pair.
>   #include <stdarg.h>
>   void va_double (int n, ...) {
>     va_list args;
>     va_start (args, n);
>     if (va_arg (args, double) != 2.0)
>       abort ();
>     va_end (args);
>    }
>    int main (int a) {
>     va_double (1, 2.0);
>     return a;
>    }
> 
> In a second thought, it seems that non-fixed double arguments may generate incorrect code, even with align even pair.
> For ilp32 or lp64 ABI with feature D, stack alignment will be 16, so even pair can make sure when pushing/popping the non-fixed double to/from the stack, it will be 8-byte alignment. For ilp32e with 4-byte alignment, even pair can not guarantee the double will be pushed to stack with 8-byte alignment.
Ah, I see the issue.

It's not clear that choosing to spill to a register pair where the first register is a multiple of 4 would solve the problem either, right? The problem is that we actually need to realign the spill slots for these register pairs.

I'm not sure how we achieve this. I will investigate further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401