[AArch64] [Windows] Functions using SVE can fail "WinCFI not supported with SVE vectors"

[mstorsjo]

Compiling AArch64 code that uses SVE features, for a Windows target, can seem to work at first, but fail later when functions become more complex.

This has been observed in libaom (https://aomedia.googlesource.com/aom), since https://aomedia.googlesource.com/aom/+/04b91c17bf976b0616ab94fa6cd36892d47e9ce5%5E%21/, when compiled for an aarch64-mingw target.

When the SVE functions end up needing to back up and restore SVE vectors, they trigger the failed "WinCFI not supported with SVE vectors" assert.

This can be triggered with this reduced code snippet from libaom:

#include <arm_neon_sve_bridge.h>
int64x2_t a(int64x2_t acc, int16x8_t b, int16x8_t c) {
  return svget_neonq_s64(svdot_s64(svset_neonq_s64(svundef_s64(), acc),
                                   svset_neonq_s16(svundef_s16(), b),
                                   svset_neonq_s16(svundef_s16(), c)));
}
void e();
int16x8_t d(uint16x8x2_t g) {
  int16x8_t f[8];
  e();
  int16x8_t j = vextq_s16(g.val[0], g.val[1], 0);
  int64x2_t i = a(vdupq_n_s64(0), j, f[0]);
  int64x2_t l;
  int64x2_t k = vpaddq_s64(i, l);
  int64x2_t m;
  int32x4_t n = vcombine_s32(vmovn_s64(k), vmovn_s64(m));
  int32x4_t o;
  return vcombine_s16(vmovn_s32(n), vmovn_s32(o));
}

$ clang -target aarch64-windows-gnu -c repro.c -march=armv8-a+sve -O2
clang: ../lib/Target/AArch64/AArch64InstrInfo.cpp:5444: void llvm::emitFrameOffset(llvm::MachineBasicBlock&, llvm::MachineBasicBlock::iterator, const llvm::DebugLoc&, unsigned int, unsigned int, llvm::StackOffset, const llvm::TargetInstrInfo*, llvm::MachineInstr::MIFlag, bool, bool, bool*, bool, llvm::StackOffset, unsigned int): Assertion `!(NeedsWinCFI && (NumPredicateVectors || NumDataVectors)) && "WinCFI not supported with SVE vectors"' failed.

It can also be reproduced with a more targeted handwritten testcase:

#include <arm_sve.h>
void other(void);
void func(svfloat32_t a) {
  other();
}

$ clang -target aarch64-windows-gnu -c repro.c -march=armv8-a+sve
clang: ../lib/Target/AArch64/AArch64InstrInfo.cpp:5444: void llvm::emitFrameOffset(llvm::MachineBasicBlock&, llvm::MachineBasicBlock::iterator, const llvm::DebugLoc&, unsigned int, unsigned int, llvm::StackOffset, const llvm::TargetInstrInfo*, llvm::MachineInstr::MIFlag, bool, bool, bool*, bool, llvm::StackOffset, unsigned int): Assertion `!(NeedsWinCFI && (NumPredicateVectors || NumDataVectors)) && "WinCFI not supported with SVE vectors"' failed.

I guess there's not much we can do about this, until MS specifies SEH unwind opcodes for dealing with scalable vectors, or until they specify something that differs from AAPCS64 which scalable vector registers need to be preserved in the Windows calling convention.

CC @efriedma-quic

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: Martin Storsjö (mstorsjo)

Compiling AArch64 code that uses SVE features, for a Windows target, can seem to work at first, but fail later when functions become more complex.

This has been observed in libaom (https://aomedia.googlesource.com/aom), since https://aomedia.googlesource.com/aom/+/04b91c17bf976b0616ab94fa6cd36892d47e9ce5%5E%21/, when compiled for an aarch64-mingw target.

When the SVE functions end up needing to back up and restore SVE vectors, they trigger the failed "WinCFI not supported with SVE vectors" assert.

This can be triggered with this reduced code snippet from libaom:

#include <arm_neon_sve_bridge.h>
int64x2_t a(int64x2_t acc, int16x8_t b, int16x8_t c) {
  return svget_neonq_s64(svdot_s64(svset_neonq_s64(svundef_s64(), acc),
                                   svset_neonq_s16(svundef_s16(), b),
                                   svset_neonq_s16(svundef_s16(), c)));
}
void e();
int16x8_t d(uint16x8x2_t g) {
  int16x8_t f[8];
  e();
  int16x8_t j = vextq_s16(g.val[0], g.val[1], 0);
  int64x2_t i = a(vdupq_n_s64(0), j, f[0]);
  int64x2_t l;
  int64x2_t k = vpaddq_s64(i, l);
  int64x2_t m;
  int32x4_t n = vcombine_s32(vmovn_s64(k), vmovn_s64(m));
  int32x4_t o;
  return vcombine_s16(vmovn_s32(n), vmovn_s32(o));
}

$ clang -target aarch64-windows-gnu -c repro.c -march=armv8-a+sve -O2
clang: ../lib/Target/AArch64/AArch64InstrInfo.cpp:5444: void llvm::emitFrameOffset(llvm::MachineBasicBlock&, llvm::MachineBasicBlock::iterator, const llvm::DebugLoc&, unsigned int, unsigned int, llvm::StackOffset, const llvm::TargetInstrInfo*, llvm::MachineInstr::MIFlag, bool, bool, bool*, bool, llvm::StackOffset, unsigned int): Assertion `!(NeedsWinCFI && (NumPredicateVectors || NumDataVectors)) && "WinCFI not supported with SVE vectors"' failed.

It can also be reproduced with a more targeted handwritten testcase:

#include <arm_sve.h>
void other(void);
void func(svfloat32_t a) {
  other();
}

$ clang -target aarch64-windows-gnu -c repro.c -march=armv8-a+sve
clang: ../lib/Target/AArch64/AArch64InstrInfo.cpp:5444: void llvm::emitFrameOffset(llvm::MachineBasicBlock&, llvm::MachineBasicBlock::iterator, const llvm::DebugLoc&, unsigned int, unsigned int, llvm::StackOffset, const llvm::TargetInstrInfo*, llvm::MachineInstr::MIFlag, bool, bool, bool*, bool, llvm::StackOffset, unsigned int): Assertion `!(NeedsWinCFI && (NumPredicateVectors || NumDataVectors)) && "WinCFI not supported with SVE vectors"' failed.

I guess there's not much we can do about this, until MS specifies SEH unwind opcodes for dealing with scalable vectors, or until they specify something that differs from AAPCS64 which scalable vector registers need to be preserved in the Windows calling convention.

CC @efriedma-quic

[efriedma-quic]

AAPCS says "If a subroutine takes at least one argument in scalable vector registers or scalable predicate registers, or if it is a function that returns results in such registers, it must ensure that the entire contents of z8-z23 are preserved across the call. In other cases it need only preserve the low 64 bits of z8-z15." MSVC docs say we follow AAPCS rules for argument passing, and we can't preserve those registers without corresponding unwind codes... so we're stuck. There's no way to correctly emit a function with an argument of SVE type.

I guess in cases where we're only emitting the unwind info for debugging, we could add a hack to skip the SVE spills, but that's pretty ugly.

Either we need new unwind codes, or we need a different ABI on Windows targets. A different ABI isn't that hard to implement... but I don't really want to do that without some guidance from Microsoft.

CC @dpaoliello

[mstorsjo]

In case we don't get a good way forward for handling this, and more code in the wild will try to use SVE when the compiler supports it, we probably should error out in these cases in a more graceful way than a failed assert. Ideally even in a way that a project's build scripts can easily deduce that SVE isn't supported, before running into it while compiling the code. (OTOH, with asserts disabled, I guess it would compile just fine even if the unwind info doesn't match.) I wonder whether it makes sense to try to support compiling SVE in the limited cases as long as AAPCS doesn't say we need to back up those registers... Just disallowing all of SVE on Windows targets might be the safest alternative?

[mstorsjo]

Also, for reference - if compiling with a Clang that doesn't have asserts enabled, the error one hits is a different one:

error: Incorrect size for av1_highbd_warp_affine_sve epilogue: 48 bytes of instructions in range, but .seh directives corresponding to 44 bytes

[pmsjt]

Just as it is the case for the original Arm64 calling convention, Window's SVE calling convention does not add any new callee-saved registers. A corollary of this is that no new unwind opcodes are necessary for SVE.
This does not mean that all use of SVE is excused from saving/restoring and associated unwinding information. The lower 128 bits of SVE registers are overlayed on Neon registers. The lower 64-bits of V8 through V15 are callee-saved. This means that any function using SVE registers Z8 through Z15 must save the lower 64-bits of the associated Neon registers and decorate such operations with the associated unwind ops, just as if they were being used as Neon.

[efriedma-quic]

That mostly solves the problem, I guess.

The other issue is that there isn't any opcode corresponding to addvl, which you need if you have SVE stack temporaries and frame-pointer elimination. We can work around that by forcing a frame pointer, but it's a bit of a pain to implement.

[pmsjt]

The ADDVL instruction allocates an amount of stack that is only know at runtime. This is no different than the use of _alloca() and, yes, implies the use of a frame pointer.

[efriedma-quic]

LLVM normally uses a stack layout that looks something like this:

stack
caller arguments
callee-saves
SVE temporaries
non-SVE temporaries
end of stack (SP)

This is fine without a frame pointer, as long as you can represent it in the unwind info.

[pmsjt]

And that is still perfectly reasonable as long as the size is known a priori. Once the size is not known at compile time, it becomes harder to dismiss the frame pointer use. SVE has that about it. We could introduce a new opcode which is like alloc_sve - a cousin of alloc_small but with the argument being a multiple of vector size. But that raises multiple hurdles, such as having the unwinder know about the vector length even when processing stacks offline, or cross thread, like in a dump or garbage collection. Given that the use of frame pointers is strongly recommended, this sounds very unnecessary and borderline redundant.

[efriedma-quic]

The way the LLVM implementation is structured, forcing a frame pointer is a bit tricky to implement. We want to avoid adding frame pointers to every function (particularly small leaf functions)... but we don't know whether there are scalable spill slots until after register allocation. I'm sure we can come up with some solution, though. Maybe we can detect whether the code refers to any SVE registers before register allocation, and reserve the frame pointer register during register allocation if it does.

Do you have a timeline for updating the official documentation? I'd prefer not to point to comments on the issue tracker in the commit message...

[pmsjt]

How is _alloca() implemented?