Skip to content

[AArch64][SVE] Remove isSVECC() in favour of changing the calling convention #152742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1356,8 +1356,7 @@ void AArch64AsmPrinter::emitFunctionEntryLabel() {
if (TT.isOSBinFormatELF() &&
(MF->getFunction().getCallingConv() == CallingConv::AArch64_VectorCall ||
MF->getFunction().getCallingConv() ==
CallingConv::AArch64_SVE_VectorCall ||
MF->getInfo<AArch64FunctionInfo>()->isSVECC())) {
CallingConv::AArch64_SVE_VectorCall)) {
auto *TS =
static_cast<AArch64TargetStreamer *>(OutStreamer->getTargetStreamer());
TS->emitDirectiveVariantPCS(CurrentFnSym);
Expand Down
10 changes: 6 additions & 4 deletions llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,10 +339,10 @@ static bool requiresSaveVG(const MachineFunction &MF);
// on the stack. This function is safe to be called before callee-saves or
// object offsets have been determined.
static bool isLikelyToHaveSVEStack(MachineFunction &MF) {
auto *AFI = MF.getInfo<AArch64FunctionInfo>();
if (AFI->isSVECC())
if (MF.getFunction().getCallingConv() == CallingConv::AArch64_SVE_VectorCall)
return true;

auto *AFI = MF.getInfo<AArch64FunctionInfo>();
if (AFI->hasCalculatedStackSizeSVE())
return bool(getSVEStackSize(MF));

Expand Down Expand Up @@ -3121,12 +3121,14 @@ static unsigned getPrologueDeath(MachineFunction &MF, unsigned Reg) {
static bool produceCompactUnwindFrame(MachineFunction &MF) {
const AArch64Subtarget &Subtarget = MF.getSubtarget<AArch64Subtarget>();
AttributeList Attrs = MF.getFunction().getAttributes();
AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
return Subtarget.isTargetMachO() &&
!(Subtarget.getTargetLowering()->supportSwiftError() &&
Attrs.hasAttrSomewhere(Attribute::SwiftError)) &&
MF.getFunction().getCallingConv() != CallingConv::SwiftTail &&
!requiresSaveVG(MF) && !AFI->isSVECC();
MF.getFunction().getCallingConv() !=
CallingConv::AArch64_SVE_VectorCall &&

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: whitespace

!requiresSaveVG(MF);
}

static bool invalidateWindowsRegisterPairing(unsigned Reg1, unsigned Reg2,
Expand Down
49 changes: 31 additions & 18 deletions llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/SmallVectorExtras.h"
Expand Down Expand Up @@ -7837,24 +7838,44 @@ SDValue AArch64TargetLowering::LowerFormalArguments(
const SmallVectorImpl<ISD::InputArg> &Ins, const SDLoc &DL,
SelectionDAG &DAG, SmallVectorImpl<SDValue> &InVals) const {
MachineFunction &MF = DAG.getMachineFunction();
const Function &F = MF.getFunction();
Function &F = MF.getFunction();
MachineFrameInfo &MFI = MF.getFrameInfo();
bool IsWin64 =
Subtarget->isCallingConvWin64(F.getCallingConv(), F.isVarArg());
bool StackViaX4 = CallConv == CallingConv::ARM64EC_Thunk_X64 ||
(isVarArg && Subtarget->isWindowsArm64EC());
AArch64FunctionInfo *FuncInfo = MF.getInfo<AArch64FunctionInfo>();

SmallVector<ISD::OutputArg, 4> Outs;
GetReturnInfo(CallConv, F.getReturnType(), F.getAttributes(), Outs,
DAG.getTargetLoweringInfo(), MF.getDataLayout());
if (any_of(Outs, [](ISD::OutputArg &Out){ return Out.VT.isScalableVector(); }))
FuncInfo->setIsSVECC(true);

// Assign locations to all of the incoming arguments.
SmallVector<CCValAssign, 16> ArgLocs;
CCState CCInfo(CallConv, isVarArg, MF, ArgLocs, *DAG.getContext());

// This logic is consistent with AArch64TargetLowering::LowerCall.
// The `ShouldUpgradeToSVECC` flag can be when analyzing arguments.
bool ShouldUpgradeToSVECC = false;
auto _ = make_scope_exit([&] {
if (CallConv != CallingConv::C && CallConv != CallingConv::Fast)
return;

if (!ShouldUpgradeToSVECC) {
// If the flag was not set, check if the return value requires the SVE CC.
SmallVector<ISD::OutputArg, 4> Outs;
GetReturnInfo(CallConv, F.getReturnType(), F.getAttributes(), Outs,
DAG.getTargetLoweringInfo(), MF.getDataLayout());
ShouldUpgradeToSVECC = any_of(
Outs, [](ISD::OutputArg &Out) { return Out.VT.isScalableVector(); });
}

if (!ShouldUpgradeToSVECC)
return;

if (isVarArg)
report_fatal_error("Passing/returning SVE types to variadic functions "
"is currently not supported");
Comment on lines +7872 to +7874
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass them to variadic functions, but not as variadic arguments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I was confused by the wording of our current errors which say "Passing SVE types to variadic functions currently not supported" (which it appears to check it use is as a variadic argument).


F.setCallingConv(CallingConv::AArch64_SVE_VectorCall);
});

// At this point, Ins[].VT may already be promoted to i32. To correctly
// handle passing i8 as i8 instead of i32 on stack, we pass in both i32 and
// i8 to CC_AArch64_AAPCS with i32 being ValVT and i8 being LocVT.
Expand Down Expand Up @@ -7942,14 +7963,14 @@ SDValue AArch64TargetLowering::LowerFormalArguments(
RC = &AArch64::FPR128RegClass;
else if (RegVT.isScalableVector() &&
RegVT.getVectorElementType() == MVT::i1) {
FuncInfo->setIsSVECC(true);
RC = &AArch64::PPRRegClass;
ShouldUpgradeToSVECC = true;
} else if (RegVT == MVT::aarch64svcount) {
FuncInfo->setIsSVECC(true);
RC = &AArch64::PPRRegClass;
ShouldUpgradeToSVECC = true;
} else if (RegVT.isScalableVector()) {
FuncInfo->setIsSVECC(true);
RC = &AArch64::ZPRRegClass;
ShouldUpgradeToSVECC = true;
} else
llvm_unreachable("RegVT not supported by FORMAL_ARGUMENTS Lowering");

Expand Down Expand Up @@ -8597,14 +8618,6 @@ bool AArch64TargetLowering::isEligibleForTailCallOptimization(
CallAttrs.caller().hasStreamingBody())
return false;

// Functions using the C or Fast calling convention that have an SVE signature
// preserve more registers and should assume the SVE_VectorCall CC.
// The check for matching callee-saved regs will determine whether it is
// eligible for TCO.
if ((CallerCC == CallingConv::C || CallerCC == CallingConv::Fast) &&
MF.getInfo<AArch64FunctionInfo>()->isSVECC())
CallerCC = CallingConv::AArch64_SVE_VectorCall;

bool CCMatch = CallerCC == CalleeCC;

// When using the Windows calling convention on a non-windows OS, we want
Expand Down
7 changes: 0 additions & 7 deletions llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,6 @@ class AArch64FunctionInfo final : public MachineFunctionInfo {

bool IsMTETagged = false;

/// The function has Scalable Vector or Scalable Predicate register argument
/// or return type
bool IsSVECC = false;

/// The frame-index for the TPIDR2 object used for lazy saves.
TPIDR2Object TPIDR2;

Expand Down Expand Up @@ -280,9 +276,6 @@ class AArch64FunctionInfo final : public MachineFunctionInfo {
int64_t getStreamingVGIdx() const { return StreamingVGIdx; };
void setStreamingVGIdx(unsigned FrameIdx) { StreamingVGIdx = FrameIdx; };

bool isSVECC() const { return IsSVECC; };
void setIsSVECC(bool s) { IsSVECC = s; };

TPIDR2Object &getTPIDR2Obj() { return TPIDR2; }

void initializeBaseYamlFields(const yaml::AArch64FunctionInfo &YamlMFI);
Expand Down
9 changes: 1 addition & 8 deletions llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,6 @@ AArch64RegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
if (MF->getFunction().getCallingConv() ==
CallingConv::AArch64_SVE_VectorCall)
return CSR_Win_AArch64_SVE_AAPCS_SaveList;
if (MF->getInfo<AArch64FunctionInfo>()->isSVECC())
return CSR_Win_AArch64_SVE_AAPCS_SaveList;
return CSR_Win_AArch64_AAPCS_SaveList;
}
if (MF->getFunction().getCallingConv() == CallingConv::AArch64_VectorCall)
Expand Down Expand Up @@ -148,8 +146,6 @@ AArch64RegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
// This is for OSes other than Windows; Windows is a separate case further
// above.
return CSR_AArch64_AAPCS_X18_SaveList;
if (MF->getInfo<AArch64FunctionInfo>()->isSVECC())
return CSR_AArch64_SVE_AAPCS_SaveList;
return CSR_AArch64_AAPCS_SaveList;
}

Expand All @@ -165,8 +161,7 @@ AArch64RegisterInfo::getDarwinCalleeSavedRegs(const MachineFunction *MF) const {
if (MF->getFunction().getCallingConv() == CallingConv::AArch64_VectorCall)
return CSR_Darwin_AArch64_AAVPCS_SaveList;
if (MF->getFunction().getCallingConv() == CallingConv::AArch64_SVE_VectorCall)
report_fatal_error(
"Calling convention SVE_VectorCall is unsupported on Darwin.");
return CSR_Darwin_AArch64_SVE_AAPCS_SaveList;
if (MF->getFunction().getCallingConv() ==
CallingConv::AArch64_SME_ABI_Support_Routines_PreserveMost_From_X0)
report_fatal_error(
Expand Down Expand Up @@ -205,8 +200,6 @@ AArch64RegisterInfo::getDarwinCalleeSavedRegs(const MachineFunction *MF) const {
return CSR_Darwin_AArch64_RT_AllRegs_SaveList;
if (MF->getFunction().getCallingConv() == CallingConv::Win64)
return CSR_Darwin_AArch64_AAPCS_Win64_SaveList;
if (MF->getInfo<AArch64FunctionInfo>()->isSVECC())
return CSR_Darwin_AArch64_SVE_AAPCS_SaveList;
return CSR_Darwin_AArch64_AAPCS_SaveList;
}

Expand Down
2 changes: 0 additions & 2 deletions llvm/test/CodeGen/AArch64/arm64-darwin-cc.ll
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
; RUN: sed -e "s,CC,cfguard_checkcc,g" %s | not --crash llc -mtriple=arm64-apple-darwin -o - 2>&1 | FileCheck %s --check-prefix=CFGUARD
; RUN: sed -e "s,CC,aarch64_sve_vector_pcs,g" %s | not --crash llc -mtriple=arm64-apple-darwin -o - 2>&1 | FileCheck %s --check-prefix=SVE_VECTOR_PCS

define CC void @f0() {
unreachable
}

; CFGUARD: Calling convention CFGuard_Check is unsupported on Darwin.
; SVE_VECTOR_PCS: Calling convention SVE_VectorCall is unsupported on Darwin.
113 changes: 21 additions & 92 deletions llvm/test/CodeGen/AArch64/win-sve.ll
Original file line number Diff line number Diff line change
Expand Up @@ -784,8 +784,6 @@ define void @f6(<vscale x 2 x i64> %x, [8 x i64] %pad, i64 %n9) personality ptr
; CHECK-NEXT: .seh_proc f6
; CHECK-NEXT: .seh_handler __CxxFrameHandler3, @unwind, @except
; CHECK-NEXT: // %bb.0:
; CHECK-NEXT: sub sp, sp, #16
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough about SEH to tell if this change is significant (to me, it seems insignificant). I also don't know if this case is fully supported.

; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: addvl sp, sp, #-18
; CHECK-NEXT: .seh_allocz 18
; CHECK-NEXT: str p4, [sp] // 2-byte Folded Spill
Expand Down Expand Up @@ -853,21 +851,21 @@ define void @f6(<vscale x 2 x i64> %x, [8 x i64] %pad, i64 %n9) personality ptr
; CHECK-NEXT: add x29, sp, #16
; CHECK-NEXT: .seh_add_fp 16
; CHECK-NEXT: .seh_endprologue
; CHECK-NEXT: sub sp, sp, #64
; CHECK-NEXT: sub sp, sp, #80
; CHECK-NEXT: mov x0, #-2 // =0xfffffffffffffffe
; CHECK-NEXT: addvl x8, x29, #18
; CHECK-NEXT: mov x19, sp
; CHECK-NEXT: stur x0, [x8, #16]
; CHECK-NEXT: stur x0, [x8]
; CHECK-NEXT: addvl x8, x29, #18
; CHECK-NEXT: ldr x1, [x8, #32]
; CHECK-NEXT: .Ltmp0:
; CHECK-NEXT: ldr x1, [x8, #16]
; CHECK-NEXT: .Ltmp0: // EH_LABEL
; CHECK-NEXT: add x0, x19, #0
; CHECK-NEXT: bl g6
; CHECK-NEXT: .Ltmp1:
; CHECK-NEXT: .Ltmp1: // EH_LABEL
; CHECK-NEXT: // %bb.1: // %invoke.cont
; CHECK-NEXT: .seh_startepilogue
; CHECK-NEXT: add sp, sp, #64
; CHECK-NEXT: .seh_stackalloc 64
; CHECK-NEXT: add sp, sp, #80
; CHECK-NEXT: .seh_stackalloc 80
; CHECK-NEXT: ldp x29, x30, [sp, #16] // 16-byte Folded Reload
; CHECK-NEXT: .seh_save_fplr 16
; CHECK-NEXT: ldr x28, [sp, #8] // 8-byte Folded Reload
Expand Down Expand Up @@ -932,12 +930,8 @@ define void @f6(<vscale x 2 x i64> %x, [8 x i64] %pad, i64 %n9) personality ptr
; CHECK-NEXT: .seh_save_preg p14, 10
; CHECK-NEXT: ldr p15, [sp, #11, mul vl] // 2-byte Folded Reload
; CHECK-NEXT: .seh_save_preg p15, 11
; CHECK-NEXT: add sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: addvl sp, sp, #18
; CHECK-NEXT: .seh_allocz 18
; CHECK-NEXT: add sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: .seh_endepilogue
; CHECK-NEXT: ret
; CHECK-NEXT: .seh_endfunclet
Expand Down Expand Up @@ -1160,64 +1154,6 @@ define void @f8(<vscale x 2 x i64> %v) {
ret void
}

define void @f9(<vscale x 2 x i64> %v, ...) {
; CHECK-LABEL: f9:
; CHECK: .seh_proc f9
; CHECK-NEXT: // %bb.0:
; CHECK-NEXT: sub sp, sp, #64
; CHECK-NEXT: .seh_stackalloc 64
; CHECK-NEXT: addvl sp, sp, #-1
; CHECK-NEXT: .seh_allocz 1
; CHECK-NEXT: str z8, [sp] // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_zreg z8, 0
; CHECK-NEXT: str x30, [sp, #-16]! // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg_x x30, 16
; CHECK-NEXT: .seh_endprologue
; CHECK-NEXT: addvl x8, sp, #1
; CHECK-NEXT: add x9, sp, #8
; CHECK-NEXT: str x2, [x8, #32]
; CHECK-NEXT: addvl x8, sp, #1
; CHECK-NEXT: str x0, [x8, #16]
; CHECK-NEXT: addvl x8, sp, #1
; CHECK-NEXT: str x1, [x8, #24]
; CHECK-NEXT: addvl x8, sp, #1
; CHECK-NEXT: str x3, [x8, #40]
; CHECK-NEXT: addvl x8, sp, #1
; CHECK-NEXT: str x4, [x8, #48]
; CHECK-NEXT: addvl x8, sp, #1
; CHECK-NEXT: str x5, [x8, #56]
; CHECK-NEXT: addvl x8, sp, #1
; CHECK-NEXT: str x6, [x8, #64]
; CHECK-NEXT: addvl x8, sp, #1
; CHECK-NEXT: str x7, [x8, #72]
; CHECK-NEXT: add x8, sp, #16
; CHECK-NEXT: addvl x8, x8, #1
; CHECK-NEXT: str x8, [sp, #8]
; CHECK-NEXT: //APP
; CHECK-NEXT: //NO_APP
; CHECK-NEXT: .seh_startepilogue
; CHECK-NEXT: ldr x30, [sp] // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg x30, 0
; CHECK-NEXT: add sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: ldr z8, [sp] // 16-byte Folded Reload
; CHECK-NEXT: .seh_save_zreg z8, 0
; CHECK-NEXT: add sp, sp, #64
; CHECK-NEXT: .seh_stackalloc 64
; CHECK-NEXT: addvl sp, sp, #1
; CHECK-NEXT: .seh_allocz 1
; CHECK-NEXT: add sp, sp, #64
; CHECK-NEXT: .seh_stackalloc 64
; CHECK-NEXT: .seh_endepilogue
; CHECK-NEXT: ret
; CHECK-NEXT: .seh_endfunclet
; CHECK-NEXT: .seh_endproc
%va_list = alloca ptr
call void @llvm.va_start.p0(ptr %va_list)
call void asm "", "r,~{d8},~{memory}"(ptr %va_list)
ret void
}

declare void @g10(ptr,ptr)
define void @f10(i64 %n, <vscale x 2 x i64> %x) "frame-pointer"="all" {
; CHECK-LABEL: f10:
Expand Down Expand Up @@ -1546,40 +1482,33 @@ define tailcc void @f15(double %d, <vscale x 4 x i32> %vs, [9 x i64], i32 %i) {
; CHECK-LABEL: f15:
; CHECK: .seh_proc f15
; CHECK-NEXT: // %bb.0:
; CHECK-NEXT: addvl sp, sp, #-1
Copy link
Member Author

@MacDue MacDue Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is due to the explicit tailcc on the function. I'm not 100% sure on the correct behaviour here, LowerCall only uses the SVE CC if the original CC is C or Fast (which the new logic matches). https://reviews.llvm.org/D84869 suggests this is not a safe CC for SVE.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LangRef says that the calling convention requires the prototype of all callees to exactly match the prototype of the function definition. So I guess adding tailcall on this function is not correct.

; CHECK-NEXT: .seh_allocz 1
; CHECK-NEXT: str z8, [sp] // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_zreg z8, 0
; CHECK-NEXT: str x28, [sp, #-16]! // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg_x x28, 16
; CHECK-NEXT: str x28, [sp, #-32]! // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg_x x28, 32
; CHECK-NEXT: str x30, [sp, #8] // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg x30, 8
; CHECK-NEXT: sub sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: str d8, [sp, #16] // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_freg d8, 16
; CHECK-NEXT: addvl sp, sp, #-1
; CHECK-NEXT: .seh_allocz 1
; CHECK-NEXT: .seh_endprologue
; CHECK-NEXT: addvl x8, sp, #2
; CHECK-NEXT: addvl x8, sp, #1
; CHECK-NEXT: addvl x9, sp, #1
; CHECK-NEXT: //APP
; CHECK-NEXT: //NO_APP
; CHECK-NEXT: stp d0, d0, [sp, #8]
; CHECK-NEXT: ldr w8, [x8, #104]
; CHECK-NEXT: str w8, [sp, #8]
; CHECK-NEXT: str d0, [x9, #24]
; CHECK-NEXT: addvl x9, sp, #1
; CHECK-NEXT: str d0, [sp]
; CHECK-NEXT: str w8, [x9, #24]
; CHECK-NEXT: .seh_startepilogue
; CHECK-NEXT: addvl sp, sp, #1
; CHECK-NEXT: .seh_allocz 1
; CHECK-NEXT: add sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: ldr d8, [sp, #16] // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_freg d8, 16
; CHECK-NEXT: ldr x30, [sp, #8] // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg x30, 8
; CHECK-NEXT: ldr x28, [sp] // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg x28, 0
; CHECK-NEXT: add sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: ldr z8, [sp] // 16-byte Folded Reload
; CHECK-NEXT: .seh_save_zreg z8, 0
; CHECK-NEXT: addvl sp, sp, #1
; CHECK-NEXT: .seh_allocz 1
; CHECK-NEXT: ldr x28, [sp], #32 // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg_x x28, 32
; CHECK-NEXT: add sp, sp, #80
; CHECK-NEXT: .seh_stackalloc 80
; CHECK-NEXT: .seh_endepilogue
Expand Down