[PATCH] D143812: [X86][Win64] Avoid statepoints prior to SEH epilogue

[Markus Böck via Phabricator via llvm-commits]

zero9178 created this revision.
zero9178 added reviewers: rnk, hans, mstorsjo.
Herald added subscribers: pengfei, hiraditya.
Herald added a project: All.
zero9178 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This patchs purpose is very similar to https://reviews.llvm.org/D119644

The gist of the issue is that SEH unwinding has certain invariants around call instructions. One of those is that a call instruction must not be immediately followed by the function epilogue. Failing to do so leads to Windows' unwinder not recognizing the frame and skipping it when unwinding the stack.

LLVM ensures this invariant by inserting a noop after a call prior to an epilogue. The implementation however, makes the unfortunate assumption that pseudo instructions may not be calls, leading to statepoints being skipped and no noop being inserted.
This patch fixes that issue by only skipping over pseudo instructions that aren't calls.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143812

Files:
  llvm/lib/Target/X86/X86MCInstLower.cpp
  llvm/test/CodeGen/X86/win64-seh-epilogue-statepoint.ll


Index: llvm/test/CodeGen/X86/win64-seh-epilogue-statepoint.ll
===================================================================
--- llvm/test/CodeGen/X86/win64-seh-epilogue-statepoint.ll
+++ llvm/test/CodeGen/X86/win64-seh-epilogue-statepoint.ll
@@ -9,6 +9,7 @@
 ; CHECK-NEXT:    .seh_endprologue
 ; CHECK-NEXT:    callq bar
 ; CHECK-NEXT:  .Ltmp0:
+; CHECK-NEXT:    nop
 ; CHECK-NEXT:    addq $40, %rsp
 ; CHECK-NEXT:    retq
 ; CHECK-NEXT:    .seh_endproc
Index: llvm/lib/Target/X86/X86MCInstLower.cpp
===================================================================
--- llvm/lib/Target/X86/X86MCInstLower.cpp
+++ llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -2714,9 +2714,10 @@
     for (MBBI = PrevCrossBBInst(MBBI);
          MBBI != MachineBasicBlock::const_iterator();
          MBBI = PrevCrossBBInst(MBBI)) {
-      // Conservatively assume that pseudo instructions don't emit code and keep
-      // looking for a call. We may emit an unnecessary nop in some cases.
-      if (!MBBI->isPseudo()) {
+      // Pseudo instructions that aren't a call are assumed to not emit any
+      // code. If they do, we worst case generate unnecessary noops after a
+      // call.
+      if (MBBI->isCall() || !MBBI->isPseudo()) {
         if (MBBI->isCall())
           EmitAndCountInstruction(MCInstBuilder(X86::NOOP));
         break;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D143812.496681.patch
Type: text/x-patch
Size: 1337 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230211/efa05aef/attachment.bin>