llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
26.95k stars 11.04k forks source link

[AArch64] Wrong code for variadic argument with preserve_none calling convention #95093

Open ostannard opened 1 month ago

ostannard commented 1 month ago

When using the preserve_none calling convention attribute, we generate incorrect code for variadic arguments:

#include <stdarg.h>

__attribute__((preserve_none))
int F1(int P5, ...) {
  va_list vl;
  va_start(vl, P5);
  int P6 = va_arg(vl, int);
  va_end(vl);
  return P6;
}
$ /work/llvm/install/bin/clang --target=aarch64--none-elf -c test.c -O1 -o - -S
        .text
        .file   "test.c"
        .globl  F1                              // -- Begin function F1
        .p2align        2
        .type   F1,@function
F1:                                     // @F1
// %bb.0:                               // %entry
        sub     sp, sp, #224
        mov     x8, #-64                        // =0xffffffffffffffc0
        mov     x9, sp
        add     x10, sp, #128
        movk    x8, #65408, lsl #32
        add     x9, x9, #128
        stp     x0, x1, [sp, #128]
        stp     x9, x8, [sp, #208]
        add     x9, x10, #64
        add     x10, sp, #224
        mov     x8, #-64                        // =0xffffffffffffffc0
        stp     x2, x3, [sp, #144]
        stp     x4, x5, [sp, #160]
        stp     x6, x7, [sp, #176]
        stp     q0, q1, [sp]
        stp     q2, q3, [sp, #32]
        stp     q4, q5, [sp, #64]
        stp     q6, q7, [sp, #96]
        stp     x10, x9, [sp, #192]
        tbz     w8, #31, .LBB0_3
// %bb.1:                               // %vaarg.maybe_reg
        add     w9, w8, #8
        cmn     w8, #8
        str     w9, [sp, #216]
        b.gt    .LBB0_3
// %bb.2:                               // %vaarg.in_reg
        ldr     x9, [sp, #200]
        add     x8, x9, x8
        b       .LBB0_4
.LBB0_3:                                // %vaarg.on_stack
        ldr     x8, [sp, #192]
        add     x9, x8, #8
        str     x9, [sp, #192]
.LBB0_4:                                // %vaarg.end
        ldr     w0, [x8]
        add     sp, sp, #224
        ret
.Lfunc_end0:
        .size   F1, .Lfunc_end0-F1
                                        // -- End function
        .ident  "clang version 19.0.0git (git@github.com:llvm/llvm-project.git 79ce70b8033815b6abd3a9a5cc2335de70f1aaab)"
        .section        ".note.GNU-stack","",@progbits
        .addrsig

The two constants of -64 in the assembly correspond to the gr_offs field in the va_list, so this ends up loading the argument from the saved copy of x0, which the first (non-variadic) argument was passed in, instead of x1.

Without the preserve_none attribute, that constant is -56, so the argument is correctly loaded from x1:

$ /work/llvm/install/bin/clang --target=aarch64--none-elf -c test.c -O1 -o - -S
        .text
        .file   "test.c"
        .globl  F1                              // -- Begin function F1
        .p2align        2
        .type   F1,@function
F1:                                     // @F1
// %bb.0:                               // %entry
        sub     sp, sp, #224
        mov     x8, #-56                        // =0xffffffffffffffc8
        mov     x9, sp
        add     x10, sp, #136
        movk    x8, #65408, lsl #32
        add     x9, x9, #128
        stp     x1, x2, [sp, #136]
        stp     x9, x8, [sp, #208]
        add     x9, x10, #56
        add     x10, sp, #224
        mov     x8, #-56                        // =0xffffffffffffffc8
        stp     x3, x4, [sp, #152]
        stp     x5, x6, [sp, #168]
        stp     q0, q1, [sp]
        stp     q2, q3, [sp, #32]
        stp     q4, q5, [sp, #64]
        stp     q6, q7, [sp, #96]
        str     x9, [sp, #200]
        stp     x7, x10, [sp, #184]
        tbz     w8, #31, .LBB0_3
// %bb.1:                               // %vaarg.maybe_reg
        add     w9, w8, #8
        cmn     w8, #8
        str     w9, [sp, #216]
        b.gt    .LBB0_3
// %bb.2:                               // %vaarg.in_reg
        ldr     x9, [sp, #200]
        add     x8, x9, x8
        b       .LBB0_4
.LBB0_3:                                // %vaarg.on_stack
        ldr     x8, [sp, #192]
        add     x9, x8, #8
        str     x9, [sp, #192]
.LBB0_4:                                // %vaarg.end
        ldr     w0, [x8]
        add     sp, sp, #224
        ret
.Lfunc_end0:
        .size   F1, .Lfunc_end0-F1
                                        // -- End function
        .ident  "clang version 19.0.0git (git@github.com:llvm/llvm-project.git 79ce70b8033815b6abd3a9a5cc2335de70f1aaab)"
        .section        ".note.GNU-stack","",@progbits
        .addrsig
llvmbot commented 1 month ago

@llvm/issue-subscribers-backend-aarch64

Author: Oliver Stannard (ostannard)

When using the `preserve_none` calling convention attribute, we generate incorrect code for variadic arguments: ```c #include <stdarg.h> __attribute__((preserve_none)) int F1(int P5, ...) { va_list vl; va_start(vl, P5); int P6 = va_arg(vl, int); va_end(vl); return P6; } ``` ``` $ /work/llvm/install/bin/clang --target=aarch64--none-elf -c test.c -O1 -o - -S .text .file "test.c" .globl F1 // -- Begin function F1 .p2align 2 .type F1,@function F1: // @F1 // %bb.0: // %entry sub sp, sp, #224 mov x8, #-64 // =0xffffffffffffffc0 mov x9, sp add x10, sp, #128 movk x8, #65408, lsl #32 add x9, x9, #128 stp x0, x1, [sp, #128] stp x9, x8, [sp, #208] add x9, x10, #64 add x10, sp, #224 mov x8, #-64 // =0xffffffffffffffc0 stp x2, x3, [sp, #144] stp x4, x5, [sp, #160] stp x6, x7, [sp, #176] stp q0, q1, [sp] stp q2, q3, [sp, #32] stp q4, q5, [sp, #64] stp q6, q7, [sp, #96] stp x10, x9, [sp, #192] tbz w8, #31, .LBB0_3 // %bb.1: // %vaarg.maybe_reg add w9, w8, #8 cmn w8, #8 str w9, [sp, #216] b.gt .LBB0_3 // %bb.2: // %vaarg.in_reg ldr x9, [sp, #200] add x8, x9, x8 b .LBB0_4 .LBB0_3: // %vaarg.on_stack ldr x8, [sp, #192] add x9, x8, #8 str x9, [sp, #192] .LBB0_4: // %vaarg.end ldr w0, [x8] add sp, sp, #224 ret .Lfunc_end0: .size F1, .Lfunc_end0-F1 // -- End function .ident "clang version 19.0.0git (git@github.com:llvm/llvm-project.git 79ce70b8033815b6abd3a9a5cc2335de70f1aaab)" .section ".note.GNU-stack","",@progbits .addrsig ``` The two constants of `-64` in the assembly correspond to the `gr_offs` field in the `va_list`, so this ends up loading the argument from the saved copy of `x0`, which the first (non-variadic) argument was passed in, instead of `x1`. Without the `preserve_none` attribute, that constant is `-56`, so the argument is correctly loaded from `x1`: ``` $ /work/llvm/install/bin/clang --target=aarch64--none-elf -c test.c -O1 -o - -S .text .file "test.c" .globl F1 // -- Begin function F1 .p2align 2 .type F1,@function F1: // @F1 // %bb.0: // %entry sub sp, sp, #224 mov x8, #-56 // =0xffffffffffffffc8 mov x9, sp add x10, sp, #136 movk x8, #65408, lsl #32 add x9, x9, #128 stp x1, x2, [sp, #136] stp x9, x8, [sp, #208] add x9, x10, #56 add x10, sp, #224 mov x8, #-56 // =0xffffffffffffffc8 stp x3, x4, [sp, #152] stp x5, x6, [sp, #168] stp q0, q1, [sp] stp q2, q3, [sp, #32] stp q4, q5, [sp, #64] stp q6, q7, [sp, #96] str x9, [sp, #200] stp x7, x10, [sp, #184] tbz w8, #31, .LBB0_3 // %bb.1: // %vaarg.maybe_reg add w9, w8, #8 cmn w8, #8 str w9, [sp, #216] b.gt .LBB0_3 // %bb.2: // %vaarg.in_reg ldr x9, [sp, #200] add x8, x9, x8 b .LBB0_4 .LBB0_3: // %vaarg.on_stack ldr x8, [sp, #192] add x9, x8, #8 str x9, [sp, #192] .LBB0_4: // %vaarg.end ldr w0, [x8] add sp, sp, #224 ret .Lfunc_end0: .size F1, .Lfunc_end0-F1 // -- End function .ident "clang version 19.0.0git (git@github.com:llvm/llvm-project.git 79ce70b8033815b6abd3a9a5cc2335de70f1aaab)" .section ".note.GNU-stack","",@progbits .addrsig ```
ostannard commented 1 month ago

CC @antangelo, who added preserve_none support in #91046.

antangelo commented 3 weeks ago

The issue seems to be that AArch64TargetLowering::saveVarArgRegisters assumes that GPR arguments are only passed in X0-X7, in that order. preserve_none (as well as ghccc) will use the callee save registers X20-X28 before X0-X7, so the implementation incorrectly assumes that the variadic arguments passed in registers start at X0. Changing the register assignment order to begin with X0-X7 produces correct code for up to 8 supplied arguments (after which it breaks due to the above assumption being incorrect again).

I haven't looked as deeply into how this behaves on the X86 implementation, but it seems to be broken in a similar way.

To properly support varargs for preserve_none, saveVarArgRegisters will need to be reworked to follow the register assignment ordering from tablegen.

efriedma-quic commented 2 weeks ago

Can you just make varargs preserve_nonecc pass arguments the same way as the C calling convention? We could theoretically make va_start work the way you're suggesting, I guess (unlike the x86 va_list, the AAPCS64 va_list doesn't hardcode the number of registers), but it seems like a lot of complexity for little benefit.