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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
