[llvm] 899739c - Revert "[AArch64][GlobalISel] Lower formal arguments of AAPCS & ms_abi variadic functions."

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 13 01:37:47 PST 2022


Author: Martin Storsjö
Date: 2022-12-13T11:37:35+02:00
New Revision: 899739cdbd6646fa48b4b7ad251eb52143d909ba

URL: https://github.com/llvm/llvm-project/commit/899739cdbd6646fa48b4b7ad251eb52143d909ba
DIFF: https://github.com/llvm/llvm-project/commit/899739cdbd6646fa48b4b7ad251eb52143d909ba.diff

LOG: Revert "[AArch64][GlobalISel] Lower formal arguments of AAPCS & ms_abi variadic functions."

This reverts commit 56fd846f370adf16bea333b12637038ea2f3c225.

This commit regressed handling of functions with floats as arguments,
reproducible e.g. like this:

$ cat test.c
double func(double f) {
    return f;
}
$ clang -target aarch64-windows -S -o - test.c -fno-asynchronous-unwind-tables
func:
	sub	sp, sp, #16
	str	x0, [sp, #8]
	ldr	d0, [sp, #8]
	add	sp, sp, #16
	ret

Added: 
    

Modified: 
    llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
    llvm/lib/Target/AArch64/AArch64ISelLowering.h
    llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
    llvm/lib/Target/AArch64/GISel/AArch64CallLowering.h
    llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
    llvm/test/CodeGen/AArch64/aarch64_win64cc_vararg.ll

Removed: 
    llvm/test/CodeGen/AArch64/GlobalISel/aapcs_vararg_frame.ll


################################################################################
diff  --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 185242b4bb5b7..af4b98e75a801 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -139,17 +139,6 @@ static cl::opt<unsigned> MaxXors("aarch64-max-xors", cl::init(16), cl::Hidden,
 /// Value type used for condition codes.
 static const MVT MVT_CC = MVT::i32;
 
-static const MCPhysReg GPRArgRegs[] = {AArch64::X0, AArch64::X1, AArch64::X2,
-                                       AArch64::X3, AArch64::X4, AArch64::X5,
-                                       AArch64::X6, AArch64::X7};
-static const MCPhysReg FPRArgRegs[] = {AArch64::Q0, AArch64::Q1, AArch64::Q2,
-                                       AArch64::Q3, AArch64::Q4, AArch64::Q5,
-                                       AArch64::Q6, AArch64::Q7};
-
-const ArrayRef<MCPhysReg> llvm::AArch64::getGPRArgRegs() { return GPRArgRegs; }
-
-const ArrayRef<MCPhysReg> llvm::AArch64::getFPRArgRegs() { return FPRArgRegs; }
-
 static inline EVT getPackedSVEVectorVT(EVT VT) {
   switch (VT.getSimpleVT().SimpleTy) {
   default:
@@ -6573,8 +6562,10 @@ void AArch64TargetLowering::saveVarArgRegisters(CCState &CCInfo,
 
   SmallVector<SDValue, 8> MemOps;
 
-  auto GPRArgRegs = AArch64::getGPRArgRegs();
-  unsigned NumGPRArgRegs = GPRArgRegs.size();
+  static const MCPhysReg GPRArgRegs[] = { AArch64::X0, AArch64::X1, AArch64::X2,
+                                          AArch64::X3, AArch64::X4, AArch64::X5,
+                                          AArch64::X6, AArch64::X7 };
+  unsigned NumGPRArgRegs = std::size(GPRArgRegs);
   if (Subtarget->isWindowsArm64EC()) {
     // In the ARM64EC ABI, only x0-x3 are used to pass arguments to varargs
     // functions.
@@ -6624,8 +6615,10 @@ void AArch64TargetLowering::saveVarArgRegisters(CCState &CCInfo,
   FuncInfo->setVarArgsGPRSize(GPRSaveSize);
 
   if (Subtarget->hasFPARMv8() && !IsWin64) {
-    auto FPRArgRegs = AArch64::getFPRArgRegs();
-    const unsigned NumFPRArgRegs = FPRArgRegs.size();
+    static const MCPhysReg FPRArgRegs[] = {
+        AArch64::Q0, AArch64::Q1, AArch64::Q2, AArch64::Q3,
+        AArch64::Q4, AArch64::Q5, AArch64::Q6, AArch64::Q7};
+    static const unsigned NumFPRArgRegs = std::size(FPRArgRegs);
     unsigned FirstVariadicFPR = CCInfo.getFirstUnallocated(FPRArgRegs);
 
     unsigned FPRSaveSize = 16 * (NumFPRArgRegs - FirstVariadicFPR);

diff  --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.h b/llvm/lib/Target/AArch64/AArch64ISelLowering.h
index bbe78d9ea5ace..112e88535aae0 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.h
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.h
@@ -497,11 +497,6 @@ enum Rounding {
 
 // Bit position of rounding mode bits in FPCR.
 const unsigned RoundingBitsPos = 22;
-
-// Registers used to pass function arguments.
-const ArrayRef<MCPhysReg> getGPRArgRegs();
-const ArrayRef<MCPhysReg> getFPRArgRegs();
-
 } // namespace AArch64
 
 class AArch64Subtarget;

diff  --git a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
index 094048554b99b..8fe70c868d9b2 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -15,7 +15,6 @@
 #include "AArch64CallLowering.h"
 #include "AArch64ISelLowering.h"
 #include "AArch64MachineFunctionInfo.h"
-#include "AArch64RegisterInfo.h"
 #include "AArch64Subtarget.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
@@ -547,88 +546,6 @@ bool AArch64CallLowering::fallBackToDAGISel(const MachineFunction &MF) const {
   return false;
 }
 
-void AArch64CallLowering::saveVarArgRegisters(
-    MachineIRBuilder &MIRBuilder, CallLowering::IncomingValueHandler &Handler,
-    CCState &CCInfo) const {
-  auto GPRArgRegs = AArch64::getGPRArgRegs();
-  auto FPRArgRegs = AArch64::getFPRArgRegs();
-
-  MachineFunction &MF = MIRBuilder.getMF();
-  MachineRegisterInfo &MRI = MF.getRegInfo();
-  MachineFrameInfo &MFI = MF.getFrameInfo();
-  AArch64FunctionInfo *FuncInfo = MF.getInfo<AArch64FunctionInfo>();
-  auto &Subtarget = MF.getSubtarget<AArch64Subtarget>();
-  bool IsWin64CC =
-      Subtarget.isCallingConvWin64(CCInfo.getCallingConv());
-  const LLT p0 = LLT::pointer(0, 64);
-  const LLT s64 = LLT::scalar(64);
-
-  unsigned FirstVariadicGPR = CCInfo.getFirstUnallocated(GPRArgRegs);
-  unsigned NumVariadicGPRArgRegs = GPRArgRegs.size() - FirstVariadicGPR + 1;
-
-  unsigned GPRSaveSize = 8 * (GPRArgRegs.size() - FirstVariadicGPR);
-  int GPRIdx = 0;
-  if (GPRSaveSize != 0) {
-    if (IsWin64CC) {
-      GPRIdx = MFI.CreateFixedObject(GPRSaveSize,
-                                     -static_cast<int>(GPRSaveSize), false);
-    } else
-      GPRIdx = MFI.CreateStackObject(GPRSaveSize, Align(8), false);
-
-    auto FIN = MIRBuilder.buildFrameIndex(p0, GPRIdx);
-    auto Offset =
-        MIRBuilder.buildConstant(MRI.createGenericVirtualRegister(s64), 8);
-
-    for (unsigned i = FirstVariadicGPR; i < GPRArgRegs.size(); ++i) {
-      Register Val = MRI.createGenericVirtualRegister(s64);
-      Handler.assignValueToReg(
-          Val, GPRArgRegs[i],
-          CCValAssign::getReg(i + MF.getFunction().getNumOperands(), MVT::i64,
-                              GPRArgRegs[i], MVT::i64, CCValAssign::Full));
-      auto MPO = IsWin64CC ? MachinePointerInfo::getFixedStack(
-                               MF, GPRIdx, (i - FirstVariadicGPR) * 8)
-                         : MachinePointerInfo::getStack(MF, i * 8);
-      MIRBuilder.buildStore(Val, FIN, MPO, inferAlignFromPtrInfo(MF, MPO));
-
-      FIN = MIRBuilder.buildPtrAdd(MRI.createGenericVirtualRegister(p0),
-                                   FIN.getReg(0), Offset);
-    }
-  }
-  FuncInfo->setVarArgsGPRIndex(GPRIdx);
-  FuncInfo->setVarArgsGPRSize(GPRSaveSize);
-
-  if (Subtarget.hasFPARMv8() && !IsWin64CC) {
-    unsigned FirstVariadicFPR = CCInfo.getFirstUnallocated(FPRArgRegs);
-
-    unsigned FPRSaveSize = 16 * (FPRArgRegs.size() - FirstVariadicFPR);
-    int FPRIdx = 0;
-    if (FPRSaveSize != 0) {
-      FPRIdx = MFI.CreateStackObject(FPRSaveSize, Align(16), false);
-
-      auto FIN = MIRBuilder.buildFrameIndex(p0, FPRIdx);
-      auto Offset =
-          MIRBuilder.buildConstant(MRI.createGenericVirtualRegister(s64), 16);
-
-      for (unsigned i = FirstVariadicFPR; i < FPRArgRegs.size(); ++i) {
-        Register Val = MRI.createGenericVirtualRegister(LLT::scalar(128));
-        Handler.assignValueToReg(
-            Val, FPRArgRegs[i],
-            CCValAssign::getReg(
-                i + MF.getFunction().getNumOperands() + NumVariadicGPRArgRegs,
-                MVT::f128, FPRArgRegs[i], MVT::f128, CCValAssign::Full));
-
-        auto MPO = MachinePointerInfo::getStack(MF, i * 16);
-        MIRBuilder.buildStore(Val, FIN, MPO, inferAlignFromPtrInfo(MF, MPO));
-
-        FIN = MIRBuilder.buildPtrAdd(MRI.createGenericVirtualRegister(p0),
-                                     FIN.getReg(0), Offset);
-      }
-    }
-    FuncInfo->setVarArgsFPRIndex(FPRIdx);
-    FuncInfo->setVarArgsFPRSize(FPRSaveSize);
-  }
-}
-
 bool AArch64CallLowering::lowerFormalArguments(
     MachineIRBuilder &MIRBuilder, const Function &F,
     ArrayRef<ArrayRef<Register>> VRegs, FunctionLoweringInfo &FLI) const {
@@ -636,9 +553,6 @@ bool AArch64CallLowering::lowerFormalArguments(
   MachineBasicBlock &MBB = MIRBuilder.getMBB();
   MachineRegisterInfo &MRI = MF.getRegInfo();
   auto &DL = F.getParent()->getDataLayout();
-  auto &Subtarget = MF.getSubtarget<AArch64Subtarget>();
-  // TODO: Support Arm64EC
-  bool IsWin64 = Subtarget.isCallingConvWin64(F.getCallingConv()) && !Subtarget.isWindowsArm64EC();
 
   SmallVector<ArgInfo, 8> SplitArgs;
   SmallVector<std::pair<Register, Register>> BoolArgs;
@@ -684,14 +598,13 @@ bool AArch64CallLowering::lowerFormalArguments(
     MIRBuilder.setInstr(*MBB.begin());
 
   const AArch64TargetLowering &TLI = *getTLI<AArch64TargetLowering>();
-  CCAssignFn *AssignFn = TLI.CCAssignFnForCall(F.getCallingConv(), IsWin64);
+  CCAssignFn *AssignFn =
+      TLI.CCAssignFnForCall(F.getCallingConv(), /*IsVarArg=*/false);
 
   AArch64IncomingValueAssigner Assigner(AssignFn, AssignFn);
   FormalArgHandler Handler(MIRBuilder, MRI);
-  SmallVector<CCValAssign, 16> ArgLocs;
-  CCState CCInfo(F.getCallingConv(), F.isVarArg(), MF, ArgLocs, F.getContext());
-  if (!determineAssignments(Assigner, SplitArgs, CCInfo) ||
-      !handleAssignments(Handler, SplitArgs, CCInfo, ArgLocs, MIRBuilder))
+  if (!determineAndHandleAssignments(Handler, Assigner, SplitArgs, MIRBuilder,
+                                     F.getCallingConv(), F.isVarArg()))
     return false;
 
   if (!BoolArgs.empty()) {
@@ -709,14 +622,10 @@ bool AArch64CallLowering::lowerFormalArguments(
   AArch64FunctionInfo *FuncInfo = MF.getInfo<AArch64FunctionInfo>();
   uint64_t StackOffset = Assigner.StackOffset;
   if (F.isVarArg()) {
-    if ((!Subtarget.isTargetDarwin() && !Subtarget.isWindowsArm64EC()) || IsWin64) {
-      // The AAPCS variadic function ABI is identical to the non-variadic
-      // one. As a result there may be more arguments in registers and we should
-      // save them for future reference.
-      // Win64 variadic functions also pass arguments in registers, but all
-      // float arguments are passed in integer registers.
-      saveVarArgRegisters(MIRBuilder, Handler, CCInfo);
-    } else if (Subtarget.isWindowsArm64EC()) {
+    auto &Subtarget = MF.getSubtarget<AArch64Subtarget>();
+    if (!Subtarget.isTargetDarwin()) {
+        // FIXME: we need to reimplement saveVarArgsRegisters from
+      // AArch64ISelLowering.
       return false;
     }
 
@@ -748,6 +657,7 @@ bool AArch64CallLowering::lowerFormalArguments(
   // in this function later.
   FuncInfo->setBytesInStackArgArea(StackOffset);
 
+  auto &Subtarget = MF.getSubtarget<AArch64Subtarget>();
   if (Subtarget.hasCustomCallingConv())
     Subtarget.getRegisterInfo()->UpdateCustomCalleeSavedRegs(MF);
 

diff  --git a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.h b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.h
index 9ae175274d5d9..cbdf77f69a637 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.h
+++ b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.h
@@ -66,10 +66,6 @@ class AArch64CallLowering: public CallLowering {
   using MemHandler =
       std::function<void(MachineIRBuilder &, int, CCValAssign &)>;
 
-  void saveVarArgRegisters(MachineIRBuilder &MIRBuilder,
-                           CallLowering::IncomingValueHandler &Handler,
-                           CCState &CCInfo) const;
-
   bool lowerTailCall(MachineIRBuilder &MIRBuilder, CallLoweringInfo &Info,
                      SmallVectorImpl<ArgInfo> &OutArgs) const;
 

diff  --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
index 98acd4c02b224..4badb81a9b48c 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -1940,18 +1940,10 @@ bool AArch64InstructionSelector::selectVaStartDarwin(
 
   Register ArgsAddrReg = MRI.createVirtualRegister(&AArch64::GPR64RegClass);
 
-  int FrameIdx = FuncInfo->getVarArgsStackIndex();
-  if (MF.getSubtarget<AArch64Subtarget>().isCallingConvWin64(
-          MF.getFunction().getCallingConv())) {
-    FrameIdx = FuncInfo->getVarArgsGPRSize() > 0
-                   ? FuncInfo->getVarArgsGPRIndex()
-                   : FuncInfo->getVarArgsStackIndex();
-  }
-
   auto MIB =
       BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(AArch64::ADDXri))
           .addDef(ArgsAddrReg)
-          .addFrameIndex(FrameIdx)
+          .addFrameIndex(FuncInfo->getVarArgsStackIndex())
           .addImm(0)
           .addImm(0);
 

diff  --git a/llvm/test/CodeGen/AArch64/GlobalISel/aapcs_vararg_frame.ll b/llvm/test/CodeGen/AArch64/GlobalISel/aapcs_vararg_frame.ll
deleted file mode 100644
index 9df0f1b82c1b5..0000000000000
--- a/llvm/test/CodeGen/AArch64/GlobalISel/aapcs_vararg_frame.ll
+++ /dev/null
@@ -1,34 +0,0 @@
-; RUN: llc < %s --global-isel=0 -mtriple=aarch64-linux-gnu -mattr=+fp-armv8 | FileCheck %s
-; RUN: llc < %s --global-isel=1 -mtriple=aarch64-linux-gnu -mattr=+fp-armv8 | FileCheck %s --check-prefix=GISEL
-
-define void @va(i32 %count, half %f, ...) nounwind {
-; CHECK-LABEL: va:
-; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    sub sp, sp, #176
-; CHECK-NEXT:    stp x4, x5, [sp, #144]
-; CHECK-NEXT:    stp x2, x3, [sp, #128]
-; CHECK-NEXT:    str x1, [sp, #120]
-; CHECK-NEXT:    stp x6, x7, [sp, #160]
-; CHECK-NEXT:    stp q1, q2, [sp]
-; CHECK-NEXT:    stp q3, q4, [sp, #32]
-; CHECK-NEXT:    stp q5, q6, [sp, #64]
-; CHECK-NEXT:    str q7, [sp, #96]
-; CHECK-NEXT:    add sp, sp, #176
-; CHECK-NEXT:    ret
-;
-; GISEL-LABEL: va:
-; GISEL:       // %bb.0: // %entry
-; GISEL-NEXT:    sub sp, sp, #176
-; GISEL-NEXT:    stp x1, x2, [sp, #120]
-; GISEL-NEXT:    stp x3, x4, [sp, #136]
-; GISEL-NEXT:    stp x5, x6, [sp, #152]
-; GISEL-NEXT:    str x7, [sp, #168]
-; GISEL-NEXT:    stp q1, q2, [sp]
-; GISEL-NEXT:    stp q3, q4, [sp, #32]
-; GISEL-NEXT:    stp q5, q6, [sp, #64]
-; GISEL-NEXT:    str q7, [sp, #96]
-; GISEL-NEXT:    add sp, sp, #176
-; GISEL-NEXT:    ret
-entry:
-  ret void
-}

diff  --git a/llvm/test/CodeGen/AArch64/aarch64_win64cc_vararg.ll b/llvm/test/CodeGen/AArch64/aarch64_win64cc_vararg.ll
index 9856433df7dd8..50ec43d8862ac 100644
--- a/llvm/test/CodeGen/AArch64/aarch64_win64cc_vararg.ll
+++ b/llvm/test/CodeGen/AArch64/aarch64_win64cc_vararg.ll
@@ -1,6 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=aarch64-linux-gnu | FileCheck %s
-; RUN: llc < %s --global-isel=1 -mtriple=aarch64-apple-darwin | FileCheck %s --check-prefix=DARWIN
 
 define win64cc void @pass_va(i32 %count, ...) nounwind {
 ; CHECK-LABEL: pass_va:
@@ -18,12 +17,6 @@ define win64cc void @pass_va(i32 %count, ...) nounwind {
 ; CHECK-NEXT:    ldp x30, x18, [sp, #16] // 16-byte Folded Reload
 ; CHECK-NEXT:    add sp, sp, #96
 ; CHECK-NEXT:    ret
-;
-; DARWIN:       ; %bb.0: ; %entry
-; DARWIN-DAG:     stp x3, x4, [sp, #56]
-; DARWIN-DAG:     stp x1, x2, [sp, #40]
-; DARWIN-DAG:     stp x5, x6, [sp, #72]
-; DARWIN-DAG:     str x7, [sp, #88]
 entry:
   %ap = alloca i8*, align 8
   %ap1 = bitcast i8** %ap to i8*
@@ -47,16 +40,6 @@ define win64cc i8* @f9(i64 %a0, i64 %a1, i64 %a2, i64 %a3, i64 %a4, i64 %a5, i64
 ; CHECK-NEXT:    str x8, [sp, #8]
 ; CHECK-NEXT:    ldr x18, [sp], #16 // 8-byte Folded Reload
 ; CHECK-NEXT:    ret
-;
-; DARWIN-LABEL: _f9:
-; DARWIN:      ; %bb.0:                                ; %entry
-; DARWIN-NEXT:   str x18, [sp, #-16]!                ; 8-byte Folded Spill
-; DARWIN-NEXT:   add x8, sp, #8
-; DARWIN-NEXT:   add x9, sp, #24
-; DARWIN-NEXT:   str x9, [x8]
-; DARWIN-NEXT:   ldr x0, [sp, #8]
-; DARWIN-NEXT:   ldr x18, [sp], #16                  ; 8-byte Folded Reload
-; DARWIN-NEXT:   ret
 entry:
   %ap = alloca i8*, align 8
   %ap1 = bitcast i8** %ap to i8*
@@ -74,16 +57,6 @@ define win64cc i8* @f8(i64 %a0, i64 %a1, i64 %a2, i64 %a3, i64 %a4, i64 %a5, i64
 ; CHECK-NEXT:    str x8, [sp, #8]
 ; CHECK-NEXT:    ldr x18, [sp], #16 // 8-byte Folded Reload
 ; CHECK-NEXT:    ret
-;
-; DARWIN-LABEL: _f8:
-; DARWIN:      ; %bb.0:                                ; %entry
-; DARWIN-NEXT:   str x18, [sp, #-16]!                ; 8-byte Folded Spill
-; DARWIN-NEXT:   add x8, sp, #8
-; DARWIN-NEXT:   add x9, sp, #16
-; DARWIN-NEXT:   str x9, [x8]
-; DARWIN-NEXT:   ldr x0, [sp, #8]
-; DARWIN-NEXT:   ldr x18, [sp], #16                  ; 8-byte Folded Reload
-; DARWIN-NEXT:   ret
 entry:
   %ap = alloca i8*, align 8
   %ap1 = bitcast i8** %ap to i8*
@@ -102,17 +75,6 @@ define win64cc i8* @f7(i64 %a0, i64 %a1, i64 %a2, i64 %a3, i64 %a4, i64 %a5, i64
 ; CHECK-NEXT:    str x8, [sp, #8]
 ; CHECK-NEXT:    ldr x18, [sp], #32 // 8-byte Folded Reload
 ; CHECK-NEXT:    ret
-;
-; DARWIN-LABEL: _f7:
-; DARWIN:      ; %bb.0:                                ; %entry
-; DARWIN-NEXT:   str x18, [sp, #-32]!                ; 8-byte Folded Spill
-; DARWIN-NEXT:   add x8, sp, #8
-; DARWIN-NEXT:   add x9, sp, #24
-; DARWIN-NEXT:   str x7, [sp, #24]
-; DARWIN-NEXT:   str x9, [x8]
-; DARWIN-NEXT:   ldr x0, [sp, #8]
-; DARWIN-NEXT:   ldr x18, [sp], #32                  ; 8-byte Folded Reload
-; DARWIN-NEXT:   ret
 entry:
   %ap = alloca i8*, align 8
   %ap1 = bitcast i8** %ap to i8*


        


More information about the llvm-commits mailing list