Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[RISCV] Fix incorrect calculation of stack size used by the libcall with the ABI ilp32e and lp64e. #110455

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[RISCV] Fix incorrect calculation of stack size used by the libcall with the ABI ilp32e and lp64e. #110455

wants to merge 1 commit into from
+240 −227

Conversation

tclin914
Copy link
Contributor

@tclin914 tclin914 commented Sep 30, 2024 •
edited
Loading

The purpose of this patch is to fix incorrect calculation of stack size used by the libcall with the ABI ilp32e and lp64e. It's always 12 bytes for ilp32e and 24 bytes for lp64e.

@ShivaChen @tclin914
[RISCV] Fix incorrect calculation of stack size used by the libcall w…
cbfb3d9 
…ith the ABI ilp32e and lp64e.

The purpose of this patch is to fix incorrect calculation of stack size
used by the libcall with the ABI ilp32e and lp64e. It's always 12 bytes
for ilp32e and 24 byte for lp64e.
@llvmbot
Copy link
Member

llvmbot commented Sep 30, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Jim Lin (tclin914)

Changes

The purpose of this patch is to fix incorrect calculation of stack size used by the libcall with the ABI ilp32e and lp64e. It's always 12 bytes for ilp32e and 24 bytes for lp64e.

Patch is 60.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/110455.diff

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVFrameLowering.cpp (+25-12)
  • (modified) llvm/test/CodeGen/RISCV/calling-conv-ilp32e.ll (+215-215)
diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
index 22824b77c37dd6..44d73c3ca57f1c 100644
--- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -168,7 +168,8 @@ static void emitSCSEpilogue(MachineFunction &MF, MachineBasicBlock &MBB,
 // restored by the libcall, except it is zero-indexed - ID 0 corresponds to a
 // single register.
 static int getLibCallID(const MachineFunction &MF,
-                        const std::vector<CalleeSavedInfo> &CSI) {
+                        const std::vector<CalleeSavedInfo> &CSI,
+                        RISCVABI::ABI ABI) {
   const auto *RVFI = MF.getInfo<RISCVMachineFunctionInfo>();
 
   if (CSI.empty() || !RVFI->useSaveRestoreLibCalls(MF))
@@ -184,6 +185,14 @@ static int getLibCallID(const MachineFunction &MF,
   if (MaxReg == RISCV::NoRegister)
     return -1;
 
+  // The libcall always save/restore ra/s0/s1 with the ABI ilp32e and lp64e.
+  if (ABI == RISCVABI::ABI_ILP32E || ABI == RISCVABI::ABI_LP64E) {
+    assert(
+        (MaxReg == RISCV::X1 || MaxReg == RISCV::X8 || MaxReg == RISCV::X9) &&
+        "Invalid MaxReg for ABI ilp32e and lp64e");
+    MaxReg = RISCV::X9;
+  }
+
   switch (MaxReg) {
   default:
     llvm_unreachable("Something has gone wrong!");
@@ -205,9 +214,9 @@ static int getLibCallID(const MachineFunction &MF,
 
 // Get the name of the libcall used for spilling callee saved registers.
 // If this function will not use save/restore libcalls, then return a nullptr.
-static const char *
-getSpillLibCallName(const MachineFunction &MF,
-                    const std::vector<CalleeSavedInfo> &CSI) {
+static const char *getSpillLibCallName(const MachineFunction &MF,
+                                       const std::vector<CalleeSavedInfo> &CSI,
+                                       RISCVABI::ABI ABI) {
   static const char *const SpillLibCalls[] = {
     "__riscv_save_0",
     "__riscv_save_1",
@@ -224,7 +233,7 @@ getSpillLibCallName(const MachineFunction &MF,
     "__riscv_save_12"
   };
 
-  int LibCallID = getLibCallID(MF, CSI);
+  int LibCallID = getLibCallID(MF, CSI, ABI);
   if (LibCallID == -1)
     return nullptr;
   return SpillLibCalls[LibCallID];
@@ -234,7 +243,8 @@ getSpillLibCallName(const MachineFunction &MF,
 // If this function will not use save/restore libcalls, then return a nullptr.
 static const char *
 getRestoreLibCallName(const MachineFunction &MF,
-                      const std::vector<CalleeSavedInfo> &CSI) {
+                      const std::vector<CalleeSavedInfo> &CSI,
+                      RISCVABI::ABI ABI) {
   static const char *const RestoreLibCalls[] = {
     "__riscv_restore_0",
     "__riscv_restore_1",
@@ -251,7 +261,7 @@ getRestoreLibCallName(const MachineFunction &MF,
     "__riscv_restore_12"
   };
 
-  int LibCallID = getLibCallID(MF, CSI);
+  int LibCallID = getLibCallID(MF, CSI, ABI);
   if (LibCallID == -1)
     return nullptr;
   return RestoreLibCalls[LibCallID];
@@ -573,8 +583,9 @@ void RISCVFrameLowering::emitPrologue(MachineFunction &MF,
   // For negative frame indices, the offset from the frame pointer will differ
   // depending on which of these groups the frame index applies to.
   // The following calculates the correct offset knowing the number of callee
-  // saved registers spilt by the two methods.
-  if (int LibCallRegs = getLibCallID(MF, MFI.getCalleeSavedInfo()) + 1) {
+  // saved registers spilt by the two methods.STI.getTargetABI()
+  if (int LibCallRegs =
+          getLibCallID(MF, MFI.getCalleeSavedInfo(), STI.getTargetABI()) + 1) {
     // Calculate the size of the frame managed by the libcall. The stack
     // alignment of these libcalls should be the same as how we set it in
     // getABIStackAlignment.
@@ -1463,7 +1474,7 @@ bool RISCVFrameLowering::assignCalleeSavedSpillSlots(
   if (RVFI->isPushable(MF)) {
     if (int64_t PushSize = RVFI->getRVPushStackSize())
       MFI.CreateFixedSpillStackObject(PushSize, -PushSize);
-  } else if (int LibCallRegs = getLibCallID(MF, CSI) + 1) {
+  } else if (int LibCallRegs = getLibCallID(MF, CSI, STI.getTargetABI()) + 1) {
     int64_t LibCallFrameSize =
         alignTo((STI.getXLen() / 8) * LibCallRegs, getStackAlign());
     MFI.CreateFixedSpillStackObject(LibCallFrameSize, -LibCallFrameSize);
@@ -1500,7 +1511,8 @@ bool RISCVFrameLowering::spillCalleeSavedRegisters(
       for (unsigned i = 0; i < PushedRegNum; i++)
         PushBuilder.addUse(FixedCSRFIMap[i].first, RegState::Implicit);
     }
-  } else if (const char *SpillLibCall = getSpillLibCallName(*MF, CSI)) {
+  } else if (const char *SpillLibCall =
+                 getSpillLibCallName(*MF, CSI, STI.getTargetABI())) {
     // Add spill libcall via non-callee-saved register t0.
     BuildMI(MBB, MI, DL, TII.get(RISCV::PseudoCALLReg), RISCV::X5)
         .addExternalSymbol(SpillLibCall, RISCVII::MO_CALL)
@@ -1625,7 +1637,8 @@ bool RISCVFrameLowering::restoreCalleeSavedRegisters(
         PopBuilder.addDef(FixedCSRFIMap[i].first, RegState::ImplicitDefine);
     }
   } else {
-    const char *RestoreLibCall = getRestoreLibCallName(*MF, CSI);
+    const char *RestoreLibCall =
+        getRestoreLibCallName(*MF, CSI, STI.getTargetABI());
     if (RestoreLibCall) {
       // Add restore libcall via tail call.
       MachineBasicBlock::iterator NewMI =
diff --git a/llvm/test/CodeGen/RISCV/calling-conv-ilp32e.ll b/llvm/test/CodeGen/RISCV/calling-conv-ilp32e.ll
index d08cf577b1bdd3..e7c3e91702491a 100644
--- a/llvm/test/CodeGen/RISCV/calling-conv-ilp32e.ll
+++ b/llvm/test/CodeGen/RISCV/calling-conv-ilp32e.ll
@@ -54,15 +54,15 @@ define i32 @callee_float_in_regs(i32 %a, float %b) {
 ;
 ; ILP32E-FPELIM-SAVE-RESTORE-LABEL: callee_float_in_regs:
 ; ILP32E-FPELIM-SAVE-RESTORE:       # %bb.0:
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_1
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 8
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 12
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_offset s0, -8
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    mv s0, a0
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    mv a0, a1
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call __fixsfsi
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    add a0, s0, a0
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_1
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
 ;
 ; ILP32E-WITHFP-SAVE-RESTORE-LABEL: callee_float_in_regs:
 ; ILP32E-WITHFP-SAVE-RESTORE:       # %bb.0:
@@ -117,26 +117,26 @@ define i32 @caller_float_in_regs() {
 ;
 ; ILP32E-FPELIM-SAVE-RESTORE-LABEL: caller_float_in_regs:
 ; ILP32E-FPELIM-SAVE-RESTORE:       # %bb.0:
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_0
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 4
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 12
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    li a0, 1
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    lui a1, 262144
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call callee_float_in_regs
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_0
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
 ;
 ; ILP32E-WITHFP-SAVE-RESTORE-LABEL: caller_float_in_regs:
 ; ILP32E-WITHFP-SAVE-RESTORE:       # %bb.0:
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_1
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset s0, -8
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa s0, 0
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    li a0, 1
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    lui a1, 262144
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call callee_float_in_regs
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_1
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
   %1 = call i32 @callee_float_in_regs(i32 1, float 2.0)
   ret i32 %1
 }
@@ -176,16 +176,16 @@ define i32 @callee_float_on_stack(i64 %a, i64 %b, i64 %c, i64 %d, float %e) {
 ;
 ; ILP32E-WITHFP-SAVE-RESTORE-LABEL: callee_float_on_stack:
 ; ILP32E-WITHFP-SAVE-RESTORE:       # %bb.0:
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_1
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset s0, -8
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa s0, 0
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    lw a0, 8(s0)
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    lw a1, 0(s0)
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    add a0, a1, a0
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_1
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
   %1 = trunc i64 %d to i32
   %2 = bitcast float %e to i32
   %3 = add i32 %1, %2
@@ -244,9 +244,9 @@ define i32 @caller_float_on_stack() {
 ;
 ; ILP32E-FPELIM-SAVE-RESTORE-LABEL: caller_float_on_stack:
 ; ILP32E-FPELIM-SAVE-RESTORE:       # %bb.0:
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_0
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    addi sp, sp, -12
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 16
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 24
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    lui a0, 264704
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    sw a0, 8(sp)
@@ -261,16 +261,16 @@ define i32 @caller_float_on_stack() {
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    li a5, 0
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call callee_float_on_stack
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    addi sp, sp, 12
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_0
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
 ;
 ; ILP32E-WITHFP-SAVE-RESTORE-LABEL: caller_float_on_stack:
 ; ILP32E-WITHFP-SAVE-RESTORE:       # %bb.0:
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_1
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi sp, sp, -12
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 20
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 24
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset s0, -8
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 20
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 24
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa s0, 0
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    lui a0, 264704
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    sw a0, 8(sp)
@@ -285,7 +285,7 @@ define i32 @caller_float_on_stack() {
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    li a5, 0
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call callee_float_on_stack
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi sp, sp, 12
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_1
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
   %1 = call i32 @callee_float_on_stack(i64 1, i64 2, i64 3, i64 4, float 5.0)
   ret i32 %1
 }
@@ -319,14 +319,14 @@ define float @callee_tiny_scalar_ret() {
 ;
 ; ILP32E-WITHFP-SAVE-RESTORE-LABEL: callee_tiny_scalar_ret:
 ; ILP32E-WITHFP-SAVE-RESTORE:       # %bb.0:
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_1
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset s0, -8
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa s0, 0
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    lui a0, 260096
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_1
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
   ret float 1.0
 }
 
@@ -360,22 +360,22 @@ define i32 @caller_tiny_scalar_ret() {
 ;
 ; ILP32E-FPELIM-SAVE-RESTORE-LABEL: caller_tiny_scalar_ret:
 ; ILP32E-FPELIM-SAVE-RESTORE:       # %bb.0:
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_0
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 4
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 12
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call callee_tiny_scalar_ret
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_0
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
 ;
 ; ILP32E-WITHFP-SAVE-RESTORE-LABEL: caller_tiny_scalar_ret:
 ; ILP32E-WITHFP-SAVE-RESTORE:       # %bb.0:
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_1
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset s0, -8
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa s0, 0
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call callee_tiny_scalar_ret
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_1
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
   %1 = call float @callee_tiny_scalar_ret()
   %2 = bitcast float %1 to i32
   ret i32 %2
@@ -428,8 +428,8 @@ define i32 @callee_double_in_regs(i32 %a, double %b) {
 ;
 ; ILP32E-FPELIM-SAVE-RESTORE-LABEL: callee_double_in_regs:
 ; ILP32E-FPELIM-SAVE-RESTORE:       # %bb.0:
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_1
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 8
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 12
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_offset s0, -8
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    mv s0, a0
@@ -437,7 +437,7 @@ define i32 @callee_double_in_regs(i32 %a, double %b) {
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    mv a1, a2
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call __fixdfsi
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    add a0, s0, a0
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_1
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
 ;
 ; ILP32E-WITHFP-SAVE-RESTORE-LABEL: callee_double_in_regs:
 ; ILP32E-WITHFP-SAVE-RESTORE:       # %bb.0:
@@ -495,28 +495,28 @@ define i32 @caller_double_in_regs() {
 ;
 ; ILP32E-FPELIM-SAVE-RESTORE-LABEL: caller_double_in_regs:
 ; ILP32E-FPELIM-SAVE-RESTORE:       # %bb.0:
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_0
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 4
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 12
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    li a0, 1
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    lui a2, 262144
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    li a1, 0
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call callee_double_in_regs
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_0
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
 ;
 ; ILP32E-WITHFP-SAVE-RESTORE-LABEL: caller_double_in_regs:
 ; ILP32E-WITHFP-SAVE-RESTORE:       # %bb.0:
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_1
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset s0, -8
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa s0, 0
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    li a0, 1
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    lui a2, 262144
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    li a1, 0
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call callee_double_in_regs
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_1
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
   %1 = call i32 @callee_double_in_regs(i32 1, double 2.0)
   ret i32 %1
 }
@@ -584,11 +584,11 @@ define i32 @callee_aligned_stack(i32 %a, i32 %b, fp128 %c, i32 %d, i32 %e, i64 %
 ;
 ; ILP32E-WITHFP-SAVE-RESTORE-LABEL: callee_aligned_stack:
 ; ILP32E-WITHFP-SAVE-RESTORE:       # %bb.0:
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_1
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset s0, -8
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 8
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi s0, sp, 12
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa s0, 0
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    lw a0, 0(a2)
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    lw a1, 12(s0)
@@ -601,7 +601,7 @@ define i32 @callee_aligned_stack(i32 %a, i32 %b, fp128 %c, i32 %d, i32 %e, i64 %
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    add a0, a0, a1
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    add a4, a5, a4
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    add a0, a0, a4
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_1
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
   %1 = bitcast fp128 %c to i128
   %2 = trunc i128 %1 to i32
   %3 = add i32 %2, %g
@@ -730,8 +730,8 @@ define void @caller_aligned_stack() {
 ;
 ; ILP32E-FPELIM-SAVE-RESTORE-LABEL: caller_aligned_stack:
 ; ILP32E-FPELIM-SAVE-RESTORE:       # %bb.0:
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_1
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    addi sp, sp, -56
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    addi sp, sp, -52
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 64
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    .cfi_offset s0, -8
@@ -777,13 +777,13 @@ define void @caller_aligned_stack() {
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    sw a6, 32(sp)
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    call callee_aligned_stack
 ; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    addi sp, s0, -64
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    addi sp, sp, 56
-; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_1
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    addi sp, sp, 52
+; ILP32E-FPELIM-SAVE-RESTORE-NEXT:    tail __riscv_restore_2
 ;
 ; ILP32E-WITHFP-SAVE-RESTORE-LABEL: caller_aligned_stack:
 ; ILP32E-WITHFP-SAVE-RESTORE:       # %bb.0:
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_1
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi sp, sp, -56
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call t0, __riscv_save_2
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi sp, sp, -52
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_def_cfa_offset 64
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset ra, -4
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    .cfi_offset s0, -8
@@ -829,8 +829,8 @@ define void @caller_aligned_stack() {
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    sw a6, 32(sp)
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    call callee_aligned_stack
 ; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi sp, s0, -64
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi sp, sp, 56
-; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    tail __riscv_restore_1
+; ILP32E-WITHFP-SAVE-RESTORE-NEXT:    addi sp, sp, 52
+...
[truncated]

@lenary
Copy link
Member

lenary commented Oct 8, 2024

Sorry for taking some time to get back to this. I was away last week.

Please can you explain a bit more what you mean by "It's always 12 bytes for ilp32e and 24 bytes for lp64e" - I think you mean the current implementation of the save/restore routines always pushes 12/24 bytes, rather than grouping by the stack alignment (as is done for the other ABIs).

Correcting the save/restore routines for the E abis has been on my list to get to, but I've not managed to write up the (toolchain conventions) document changes I think are needed: riscv-non-isa/riscv-toolchain-conventions#35 - it is clear to me the current CFI information emitted by LLVM does not match the current compiler-rt implementation.

I'll also note I have a patch that matches the proposed direction, which tries to minimise stack usage: #95398 - which I think kito and I are in favour of - and this PR would mean the improvement from that other PR would have no effect, and we'd always use the larger stack amounts.

Other related changes and discussions, relating to recent work, which might have useful information in them:

@tclin914
Copy link
Contributor Author

tclin914 commented Oct 9, 2024

Sorry that I didn't know the correction of the save/restore routines for E is on progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@topperc topperc Awaiting requested review from topperc

@wangpc-pp wangpc-pp Awaiting requested review from wangpc-pp

@lenary lenary Awaiting requested review from lenary

@kito-cheng kito-cheng Awaiting requested review from kito-cheng

Assignees
No one assigned
Labels
backend:RISC-V
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tclin914 @llvmbot @lenary @ShivaChen