[M68k] function epilogue pops stack too far

[TechnoElf]

The generated epilogue for functions that return structs adds 4 to the stack pointer despite there being no corresponding subtraction in the prologue.

A minimal example for this issue would be

define internal void @test(ptr sret([10 x i8]) align 2 %_0) unnamed_addr #1 {
start:
  ret void
}

which produces the following code (using llc -O0 -march=m68k):

        .p2align        1                               ; -- Begin function test
        .type   test,@function
test:                                   ; @test
        .cfi_startproc
; %bb.0:                                ; %start
        move.l  (4,%sp), %d0
        move.l  (%sp), %a1
        adda.l  #4, %sp
        move.l  %a1, (%sp)
        rts
.Lfunc_end2:
        .size   test, .Lfunc_end2-test
        .cfi_endproc
                                        ; -- End function
        .section        ".note.GNU-stack","",@progbits

The same issue occurs with longer functions that do actually use the stack.

[TechnoElf]

The following change produces the correct code:

--- a/llvm/lib/Target/M68k/M68kISelLowering.cpp
+++ b/llvm/lib/Target/M68k/M68kISelLowering.cpp
@@ -1046,8 +1048,8 @@ SDValue M68kTargetLowering::LowerFormalArguments(
   } else {
     MMFI->setBytesToPopOnReturn(0); // Callee pops nothing.
     // If this is an sret function, the return should pop the hidden pointer.
-    if (!canGuaranteeTCO(CCID) && argsAreStructReturn(Ins) == StackStructReturn)
-      MMFI->setBytesToPopOnReturn(4);
+    //if (!canGuaranteeTCO(CCID) && argsAreStructReturn(Ins) == StackStructReturn)
+    //  MMFI->setBytesToPopOnReturn(4);
   }

   MMFI->setArgumentStackSize(StackSize);

        .p2align        1                               ; -- Begin function test
        .type   test,@function
test:                                   ; @test
        .cfi_startproc
; %bb.0:                                ; %start
        move.l  (4,%sp), %d0
        rts
.Lfunc_end2:
        .size   test, .Lfunc_end2-test
        .cfi_endproc
                                        ; -- End function
        .section        ".note.GNU-stack","",@progbits

Is this fix correct or does this additional pop have a purpose that I missed?

[llvmbot]

@llvm/issue-subscribers-backend-m68k

Author: Janis Heims (TechnoElf)

The generated epilogue for functions that return structs adds 4 to the stack pointer despite there being no corresponding subtraction in the prologue. A minimal example for this issue would be ``` define internal void @test(ptr sret([10 x i8]) align 2 %_0) unnamed_addr #1 { start: ret void } ``` which produces the following code (using `llc -O0 -march=m68k`): ``` .p2align 1 ; -- Begin function test .type test,@function test: ; @test .cfi_startproc ; %bb.0: ; %start move.l (4,%sp), %d0 move.l (%sp), %a1 adda.l #4, %sp move.l %a1, (%sp) rts .Lfunc_end2: .size test, .Lfunc_end2-test .cfi_endproc ; -- End function .section ".note.GNU-stack","",@progbits ``` The same issue occurs with longer functions that do actually use the stack.

[TechnoElf]

What's the "hidden pointer" that's referred to in the comment in LowerFormalArguments and LowerCall? Is it the frame pointer?

[TechnoElf]

Setting ReserveCallFrame to false in M68kFrameLowering::eliminateCallFramePseudoInstr also seems to generate functional machine code. I guess hasReservedCallFrame should probably return false if the called function returns a struct using the stack?

Using the test from multiple-return.ll:

define { i32, i32, i32, i32 } @test0() {
start:
  ret { i32, i32, i32, i32 } { i32 13, i32 17, i32 19, i32 23 }
}

define void @test0_call() {
start:
  %0 = call { i32, i32, i32, i32 } @test0()
  ret void
}

.text
        .file   "multiple-return.ll"
        .globl  test0                           ; -- Begin function test0
        .p2align        1
        .type   test0,@function
test0:                                  ; @test0
        .cfi_startproc
; %bb.0:                                ; %start
        move.l  (4,%sp), %a0
        move.l  %a0, %d0
        move.l  #23, (12,%a0)
        move.l  #19, (8,%a0)
        move.l  #17, (4,%a0)
        move.l  #13, (%a0)
        move.l  (%sp), %a1
        adda.l  #4, %sp
        move.l  %a1, (%sp)
        rts
.Lfunc_end0:
        .size   test0, .Lfunc_end0-test0
        .cfi_endproc
                                        ; -- End function
        .globl  test0_call                      ; -- Begin function test0_call
        .p2align        1
        .type   test0_call,@function
test0_call:                             ; @test0_call
        .cfi_startproc
; %bb.0:                                ; %start
        suba.l  #20, %sp
        .cfi_def_cfa_offset -24
        suba.l  #8, %sp
        .cfi_adjust_cfa_offset 8
        move.l  %sp, %a0
        lea     (4,%sp), %a1
        move.l  %a1, (%a0)
        jsr     test0
        .cfi_adjust_cfa_offset -4
        adda.l  #4, %sp
        .cfi_adjust_cfa_offset -4
        adda.l  #20, %sp
        rts
.Lfunc_end1:
        .size   test0_call, .Lfunc_end1-test0_call
        .cfi_endproc
                                        ; -- End function