Skip to content

[X86] Don't save/restore fp/bp around terminator #106462

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

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

weiguozhi
Copy link
Contributor

In function spillFPBP we already try to skip terminator, but there is a logic error, so when there is only terminator instruction in the MBB, it still tries to save/restore fp/bp around it if the terminator clobbers fp/bp, for example a tail call with ghc calling convention.

Now this patch really skips terminator even if it is the only instruction in the MBB.

In function spillFPBP we already try to skip terminator, but there is a
logic error, so when there is only terminator instruction in the MBB, it
still tries to save/restore fp/bp around it if the terminator clobbers
fp/bp, for example a tail call with ghc calling convention.

Now this patch really skips terminator even if it is the only
instruction in the MBB.
@weiguozhi weiguozhi requested a review from rnk August 28, 2024 22:04
@llvmbot
Copy link
Member

llvmbot commented Aug 28, 2024

@llvm/pr-subscribers-backend-x86

Author: None (weiguozhi)

Changes

In function spillFPBP we already try to skip terminator, but there is a logic error, so when there is only terminator instruction in the MBB, it still tries to save/restore fp/bp around it if the terminator clobbers fp/bp, for example a tail call with ghc calling convention.

Now this patch really skips terminator even if it is the only instruction in the MBB.


Full diff: https://github.com/llvm/llvm-project/pull/106462.diff

2 Files Affected:

  • (modified) llvm/lib/Target/X86/X86FrameLowering.cpp (+3-2)
  • (modified) llvm/test/CodeGen/X86/clobber_frame_ptr.ll (+43)
diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
index 43a3219f789c4a..53bf04e6eff25d 100644
--- a/llvm/lib/Target/X86/X86FrameLowering.cpp
+++ b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -4502,8 +4502,9 @@ void X86FrameLowering::spillFPBP(MachineFunction &MF) const {
     bool InsideEHLabels = false;
     auto MI = MBB.rbegin(), ME = MBB.rend();
     auto TermMI = MBB.getFirstTerminator();
-    if (TermMI != MBB.begin())
-      MI = *(std::prev(TermMI));
+    if (TermMI == MBB.begin())
+      continue;
+    MI = *(std::prev(TermMI));
 
     while (MI != ME) {
       // Skip frame setup/destroy instructions.
diff --git a/llvm/test/CodeGen/X86/clobber_frame_ptr.ll b/llvm/test/CodeGen/X86/clobber_frame_ptr.ll
index 55c2d791b66e76..f6b38839d13cc2 100644
--- a/llvm/test/CodeGen/X86/clobber_frame_ptr.ll
+++ b/llvm/test/CodeGen/X86/clobber_frame_ptr.ll
@@ -144,3 +144,46 @@ entry:
   call void @llvm.eh.sjlj.longjmp(ptr @buf)
   unreachable
 }
+
+declare ghccc void @tail()
+
+; We should not save/restore fp/bp around terminator.
+define ghccc void @test5() {
+; CHECK-LABEL: test5:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushq %rbp
+; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    .cfi_offset %rbp, -16
+; CHECK-NEXT:    movq %rsp, %rbp
+; CHECK-NEXT:    .cfi_def_cfa_register %rbp
+; CHECK-NEXT:    andq $-8, %rsp
+; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    testb %al, %al
+; CHECK-NEXT:    jne .LBB3_2
+; CHECK-NEXT:  # %bb.1: # %then
+; CHECK-NEXT:    movq $0, (%rax)
+; CHECK-NEXT:    movq %rbp, %rsp
+; CHECK-NEXT:    popq %rbp
+; CHECK-NEXT:    .cfi_def_cfa %rsp, 8
+; CHECK-NEXT:    retq
+; CHECK-NEXT:  .LBB3_2: # %else
+; CHECK-NEXT:    .cfi_def_cfa %rbp, 16
+; CHECK-NEXT:    movq %rbp, %rsp
+; CHECK-NEXT:    popq %rbp
+; CHECK-NEXT:    .cfi_def_cfa %rsp, 8
+; CHECK-NEXT:    jmp tail@PLT # TAILCALL
+entry:
+  br i1 undef, label %then, label %else
+
+then:
+  store i64 0, ptr undef
+  br label %exit
+
+else:
+  musttail call ghccc void @tail()
+  ret void
+
+exit:
+  ret void
+}
+

@weiguozhi
Copy link
Contributor Author

ping

@weiguozhi weiguozhi merged commit cdab6ff into llvm:main Sep 3, 2024
10 checks passed
@weiguozhi weiguozhi deleted the carrot-mbb-begin branch October 7, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants