llvm / llvm/llvm-project

[AArch64][GISel] Incorrect alignment for packed HVA struct passed on stack with global isel

Open
#186,152 1 comment 0 reactions 0 assignees View on GitHub
backend:AArch64 generated by fuzzer llvm:globalisel
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

This code is miscompiled when targeting AArch64 at -O0. I think that the bug is in `main`, and only occurs when global isel is used (default at -O0).
https://godbolt.org/z/T4MabefYz
```test.c
#include
#include
#include

struct __attribute((packed)) S120 {
uint16x8_t M0;
};

__attribute((noinline))
void f(double a0, double a1, double a2, double a3, double a4, double a5,
double a6, double a7, double a8, ...) {
va_list vl;
va_start(vl, a8);
struct S120 p = va_arg(vl, struct S120);

assert(vgetq_lane_u16(p.M0, 0) == 42);
}

int main(void) {
f(0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, (struct S120){vdupq_n_u16(42)});
return 0;
}
```

Generated code for main with global isel:
```asm
$ /work/llvm/build/bin/clang --target=aarch64-none-elf -O0 -S test.c -o -
...
main: // @main
// %bb.0: // %entry
sub sp, sp, #128
stp x29, x30, [sp, #112] // 16-byte Folded Spill
add x29, sp, #112
mov w8, wzr
str w8, [sp, #40] // 4-byte Spill
stur wzr, [x29, #-52]
sub x8, x29, #2
mov w9, #42 // =0x2a
sturh w9, [x29, #-2]
ldur h0, [x29, #-2]
// kill: def $q0 killed $h0
ld1 { v0.h }[1], [x8]
ld1 { v0.h }[2], [x8]
ld1 { v0.h }[3], [x8]
ld1 { v0.h }[4], [x8]
ld1 { v0.h }[5], [x8]
ld1 { v0.h }[6], [x8]
ld1 { v0.h }[7], [x8]
stur q0, [x29, #-48]
ldur q0, [x29, #-48]
stur q0, [x29, #-32]
ldur q0, [x29, #-32]
stur q0, [sp, #44]
ldur q0, [sp, #44]
mov x8, sp
movi d7, #0000000000000000
str d7, [x8]
str q0, [x8, #16] // BUG: arg passed at SP+16
fmov d0, d7
fmov d1, d7
fmov d2, d7
fmov d3, d7
fmov d4, d7
fmov d5, d7
fmov d6, d7
bl f
ldr w0, [sp, #40] // 4-byte Reload
ldp x29, x30, [sp, #112] // 16-byte Folded Reload
add sp, sp, #128
ret
...
```

Generated code with DAG isel:
```asm
$ /work/llvm/build/bin/clang --target=aarch64-none-elf -O0 -S test.c -o - -fno-global-isel
main: // @main
// %bb.0: // %entry
sub sp, sp, #112
stp x29, x30, [sp, #96] // 16-byte Folded Spill
add x29, sp, #96
mov w8, wzr
str w8, [sp, #44]
mov w8, #42 // =0x2a
sturh w8, [x29, #-2]
sub x8, x29, #2
ld1r { v0.8h }, [x8]
str q0, [sp, #48]
ldr q0, [sp, #48]
stur q0, [x29, #-32]
ldur q0, [x29, #-32]
stur q0, [sp, #28]
ldur q0, [sp, #28]
mov x9, sp
stur q0, [x9, #8] // Arg correctly passed at SP+8
mov x8, xzr
str x8, [x9]
movi d7, #0000000000000000
fmov d0, d7
fmov d1, d7
fmov d2, d7
fmov d3, d7
fmov d4, d7
fmov d5, d7
fmov d6, d7
bl f
mov w0, wzr
ldp x29, x30, [sp, #96] // 16-byte Folded Reload
add sp, sp, #112
ret
```

The difference between these two is that DAG isel passes the `S120` argument at SP+8, but global isel passes it at SP+16. I think DAG isel is correct here, as the alignment of the struct is 1 byte, but gets rounded up to 8 bytes (not 16) by the AAPCS64 rules:
> For a Composite Type, the alignment of the copy will have 8-byte alignment if its natural alignment is ≤ 8 and 16-byte alignment if its natural alignment is ≥ 16.

In both cases, the va_arg code generated in the callee does not do any re-alignment before reading the argument from the stack, so it is read from SP+8.

The IR passes the argument as `[1 x <8 x i16>] alignstack(8)`, but the `alignstack` attribute appears to be ignored by global isel.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.