[llvm] 65b2128 - Avoid emitting unreachable SP adjustments after `throw`
Reid Kleckner via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 6 13:33:51 PST 2020
Author: Reid Kleckner
Date: 2020-03-06T13:33:45-08:00
New Revision: 65b21282c710afe9c275778820c6e3c1cf46734b
URL: https://github.com/llvm/llvm-project/commit/65b21282c710afe9c275778820c6e3c1cf46734b
DIFF: https://github.com/llvm/llvm-project/commit/65b21282c710afe9c275778820c6e3c1cf46734b.diff
LOG: Avoid emitting unreachable SP adjustments after `throw`
In 172eee9c, we tried to avoid these by modelling the callee as
internally resetting the stack pointer.
However, for the majority of functions with reserved stack frames, this
would lead LLVM to emit extra SP adjustments to undo the callee's
internal adjustment. This lead us to fix the problem further on down the
pipeline in eliminateCallFramePseudoInstr. In 5b79e603d3b7a2994, I added
use a heuristic to try to detect when the adjustment would be
unreachable.
This heuristic is imperfect, and when exception handling is involved, it
fails to fire. The new test is an example of this. Simply throwing an
exception with an active cleanup emits dead SP adjustments after the
throw. Not only are they dead, but if they were executed, they would be
incorrect, so they are confusing.
This change essentially reverts 172eee9c and makes the 5b79e603d3b7a2994
heuristic responsible for preventing unreachable stack adjustments. This
means we may emit unreachable stack adjustments for functions using EH
with unreserved call frames, but that is not very many these days. Back
in 2016 when this change was added, we were focused on 32-bit, which we
observed to have fewer reserved frames.
Fixes PR45064
Reviewed By: hans
Differential Revision: https://reviews.llvm.org/D75712
Added:
Modified:
llvm/lib/Target/X86/X86FrameLowering.cpp
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/test/CodeGen/X86/noreturn-call-win64.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
index 9d7d5b7a6f18..bc88401797c9 100644
--- a/llvm/lib/Target/X86/X86FrameLowering.cpp
+++ b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -3009,6 +3009,12 @@ eliminateCallFramePseudoInstr(MachineFunction &MF, MachineBasicBlock &MBB,
I = MBB.erase(I);
auto InsertPos = skipDebugInstructionsForward(I, MBB.end());
+ // Try to avoid emitting dead SP adjustments if the block end is unreachable,
+ // typically because the function is marked noreturn (abort, throw,
+ // assert_fail, etc).
+ if (isDestroy && blockEndIsUnreachable(MBB, I))
+ return I;
+
if (!reserveCallFrame) {
// If the stack pointer can be changed after prologue, turn the
// adjcallstackup instruction into a 'sub ESP, <amt>' and the
@@ -3091,13 +3097,7 @@ eliminateCallFramePseudoInstr(MachineFunction &MF, MachineBasicBlock &MBB,
return I;
}
- if (isDestroy && InternalAmt && !blockEndIsUnreachable(MBB, I)) {
- // If we are performing frame pointer elimination and if the callee pops
- // something off the stack pointer, add it back. We do this until we have
- // more advanced stack pointer tracking ability.
- // We are not tracking the stack pointer adjustment by the callee, so make
- // sure we restore the stack pointer immediately after the call, there may
- // be spill code inserted between the CALL and ADJCALLSTACKUP instructions.
+ if (InternalAmt) {
MachineBasicBlock::iterator CI = I;
MachineBasicBlock::iterator B = MBB.begin();
while (CI != B && !std::prev(CI)->isCall())
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 6b327699a61f..9610706e3b87 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -4362,12 +4362,6 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
else
NumBytesForCalleeToPop = 0; // Callee pops nothing.
- if (CLI.DoesNotReturn && !getTargetMachine().Options.TrapUnreachable) {
- // No need to reset the stack after the call if the call doesn't return. To
- // make the MI verify, we'll pretend the callee does it for us.
- NumBytesForCalleeToPop = NumBytes;
- }
-
// Returns a flag for retval copy to use.
if (!IsSibcall) {
Chain = DAG.getCALLSEQ_END(Chain,
diff --git a/llvm/test/CodeGen/X86/noreturn-call-win64.ll b/llvm/test/CodeGen/X86/noreturn-call-win64.ll
index 6289eef6bb48..7f9dcc0c9bb9 100644
--- a/llvm/test/CodeGen/X86/noreturn-call-win64.ll
+++ b/llvm/test/CodeGen/X86/noreturn-call-win64.ll
@@ -1,5 +1,8 @@
; RUN: llc < %s -mtriple=x86_64-windows-msvc | FileCheck %s
+%struct.MakeCleanup = type { i8 }
+%eh.ThrowInfo = type { i32, i32, i32, i32 }
+
; Function Attrs: noinline nounwind optnone uwtable
define dso_local i32 @foo() {
entry:
@@ -51,3 +54,58 @@ declare dso_local i32 @cond()
declare dso_local void @abort1() noreturn
declare dso_local void @abort2() noreturn
declare dso_local void @abort3() noreturn
+
+define dso_local void @throw_exception() uwtable personality i32 (...)* @__CxxFrameHandler3 {
+entry:
+ %o = alloca %struct.MakeCleanup, align 1
+ %call = invoke i32 @cond()
+ to label %invoke.cont unwind label %ehcleanup
+
+invoke.cont: ; preds = %entry
+ %cmp1 = icmp eq i32 0, %call
+ br i1 %cmp1, label %if.then, label %if.end
+
+if.then: ; preds = %invoke.cont
+ invoke void @_CxxThrowException(i8* null, %eh.ThrowInfo* null)
+ to label %unreachable unwind label %ehcleanup
+
+if.end: ; preds = %invoke.cont
+ %call2 = invoke i32 @cond()
+ to label %invoke.cont1 unwind label %ehcleanup
+
+invoke.cont1: ; preds = %if.end
+ %cmp2 = icmp eq i32 0, %call2
+ br i1 %cmp2, label %if.then3, label %if.end4
+
+if.then3: ; preds = %invoke.cont1
+ invoke void @_CxxThrowException(i8* null, %eh.ThrowInfo* null)
+ to label %unreachable unwind label %ehcleanup
+
+if.end4: ; preds = %invoke.cont1
+ call void @"??1MakeCleanup@@QEAA at XZ"(%struct.MakeCleanup* nonnull %o)
+ ret void
+
+ehcleanup: ; preds = %if.then3, %if.end, %if.then, %entry
+ %cp = cleanuppad within none []
+ call void @"??1MakeCleanup@@QEAA at XZ"(%struct.MakeCleanup* nonnull %o) [ "funclet"(token %cp) ]
+ cleanupret from %cp unwind to caller
+
+unreachable: ; preds = %if.then3, %if.then
+ unreachable
+}
+
+declare dso_local i32 @__CxxFrameHandler3(...)
+declare dso_local void @_CxxThrowException(i8*, %eh.ThrowInfo*)
+declare dso_local void @"??1MakeCleanup@@QEAA at XZ"(%struct.MakeCleanup*)
+
+; CHECK-LABEL: throw_exception:
+; CHECK: callq cond
+; CHECK: je
+; CHECK: callq cond
+; CHECK: je
+; CHECK: retq
+; CHECK: callq _CxxThrowException
+; CHECK-NOT: {{(addq|subq) .*, %rsp}}
+; CHECK: callq _CxxThrowException
+; CHECK-NOT: {{(addq|subq) .*, %rsp}}
+; CHECK: .seh_handlerdata
More information about the llvm-commits
mailing list