[SjLjEHPrepare] Configure call sites correctly #117656
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
After 9fe78db, the pass inserts `store volatile i32 -1, ptr %call_site` before all invoke instruction except the one in the entry block, which has the effect of bypassing landing pads on exceptions. When configuring the call site for a potentially throwing instruction check that it is not `InvokeInst` -- they are handled by earlier code.
|
@llvm/pr-subscribers-backend-arm @llvm/pr-subscribers-backend-x86 Author: Sergei Barannikov (s-barannikov) ChangesAfter 9fe78db, the pass inserts When configuring the call site for a potentially throwing instruction check that it is not Full diff: https://github.com/llvm/llvm-project/pull/117656.diff 4 Files Affected:
diff --git a/llvm/test/CodeGen/ARM/sjljehprepare-lower-empty-struct.ll b/llvm/test/CodeGen/ARM/sjljehprepare-lower-empty-struct.ll
index d691a7891e97ca..5b6dd39df0db2d 100644
--- a/llvm/test/CodeGen/ARM/sjljehprepare-lower-empty-struct.ll
+++ b/llvm/test/CodeGen/ARM/sjljehprepare-lower-empty-struct.ll
@@ -16,11 +16,15 @@
define ptr @foo(i8 %a, {} %c) personality ptr @baz {
entry:
; CHECK: bl __Unwind_SjLj_Register
+; CHECK-NEXT: mov r0, #1
+; CHECK-NEXT: str r0, [sp, #{{[0-9]+}}]
; CHECK-NEXT: {{[A-Z][a-zA-Z0-9]*}}:
; CHECK-NEXT: bl _bar
; CHECK: bl __Unwind_SjLj_Resume
; CHECK-LINUX: bl _Unwind_SjLj_Register
+; CHECK-LINUX-NEXT: mov r0, #1
+; CHECK-LINUX-NEXT: str r0, [sp, #{{[0-9]+}}]
; CHECK-LINUX-NEXT: .{{[A-Z][a-zA-Z0-9]*}}:
; CHECK-LINUX-NEXT: bl bar
; CHECK-LINUX: bl _Unwind_SjLj_Resume
diff --git a/llvm/test/CodeGen/Generic/sjlj-eh-prepare.ll b/llvm/test/CodeGen/Generic/sjlj-eh-prepare.ll
new file mode 100644
index 00000000000000..6efed09ea520b6
--- /dev/null
+++ b/llvm/test/CodeGen/Generic/sjlj-eh-prepare.ll
@@ -0,0 +1,40 @@
+; RUN: opt -p sjlj-eh-prepare %s -S -o - | FileCheck %s
+
+; Check that callsites are set up correctly:
+; 1. Throwing call in the entry block does not set call_site
+; (function context hasn't been configured yet).
+; 2. Throwing call not in the entry block sets call_site to -1
+; (reset to the initial state).
+; 3. Invoke instructions set call_site to the correct call site number.
+; 4. Resume instruction sets call_site to -1 (reset to the initial state).
+
+define void @test_call_sites() personality ptr @__gxx_personality_sj0 {
+entry:
+ ; CHECK-NOT: store volatile
+ ; CHECK: call void @may_throw()
+ call void @may_throw()
+
+ ; CHECK: store volatile i32 1
+ ; CHECK-NEXT: call void @llvm.eh.sjlj.callsite(i32 1)
+ ; CHECK-NEXT: invoke void @may_throw()
+ invoke void @may_throw() to label %invoke.cont unwind label %lpad
+
+invoke.cont:
+ ; CHECK: store volatile i32 2
+ ; CHECK-NEXT: call void @llvm.eh.sjlj.callsite(i32 2)
+ ; CHECK-NEXT: invoke void @may_throw()
+ invoke void @may_throw() to label %try.cont unwind label %lpad
+
+lpad:
+ ; CHECK: store volatile i32 -1
+ ; CHECK-NEXT: resume
+ %lp = landingpad { ptr, i32 } cleanup
+ resume { ptr, i32 } %lp
+
+try.cont:
+ call void @may_throw()
+ ret void
+}
+
+declare void @may_throw()
+declare i32 @__gxx_personality_sj0(...)
diff --git a/llvm/test/CodeGen/X86/indirect-branch-tracking-eh2.ll b/llvm/test/CodeGen/X86/indirect-branch-tracking-eh2.ll
index 01234317ce834b..1c48e8c5814464 100644
--- a/llvm/test/CodeGen/X86/indirect-branch-tracking-eh2.ll
+++ b/llvm/test/CodeGen/X86/indirect-branch-tracking-eh2.ll
@@ -24,9 +24,9 @@ define dso_local i32 @main() #0 personality ptr @__gxx_personality_sj0 {
; NUM-NEXT: movq %rbp, -104(%rbp)
; NUM-NEXT: movq %rsp, -88(%rbp)
; NUM-NEXT: movq $.LBB0_9, -96(%rbp)
-; NUM-NEXT: movl $1, -144(%rbp)
; NUM-NEXT: leaq -152(%rbp), %rdi
; NUM-NEXT: callq _Unwind_SjLj_Register@PLT
+; NUM-NEXT: movl $1, -144(%rbp)
; NUM-NEXT: .Ltmp0:
; NUM-NEXT: callq _Z3foov
; NUM-NEXT: .Ltmp1:
@@ -110,9 +110,9 @@ define dso_local i32 @main() #0 personality ptr @__gxx_personality_sj0 {
; SJLJ-NEXT: movq %rbp, -104(%rbp)
; SJLJ-NEXT: movq %rsp, -88(%rbp)
; SJLJ-NEXT: movq $.LBB0_9, -96(%rbp)
-; SJLJ-NEXT: movl $1, -144(%rbp)
; SJLJ-NEXT: leaq -152(%rbp), %rdi
; SJLJ-NEXT: callq _Unwind_SjLj_Register@PLT
+; SJLJ-NEXT: movl $1, -144(%rbp)
; SJLJ-NEXT: .Ltmp0:
; SJLJ-NEXT: callq _Z3foov
; SJLJ-NEXT: .Ltmp1:
diff --git a/llvm/test/CodeGen/X86/sjlj-eh.ll b/llvm/test/CodeGen/X86/sjlj-eh.ll
index 84f0088fa71eec..d2dcb35a4908ef 100644
--- a/llvm/test/CodeGen/X86/sjlj-eh.ll
+++ b/llvm/test/CodeGen/X86/sjlj-eh.ll
@@ -48,14 +48,13 @@ try.cont:
; CHECK: movl %esp, -24(%ebp)
; UFC.__jbuf[1] = $EIP
; CHECK: movl $[[RESUME:LBB[0-9]+_[0-9]+]], -28(%ebp)
-; UFC.__callsite = 1
-; CHECK: movl $1, -60(%ebp)
; _Unwind_SjLj_Register(&UFC);
; CHECK: leal -64(%ebp), %eax
; CHECK: pushl %eax
; CHECK: calll __Unwind_SjLj_Register
; CHECK: addl $4, %esp
; function_that_throws();
+; CHECK: movl $1, -60(%ebp)
; CHECK: calll __Z20function_that_throwsv
; _Unwind_SjLj_Unregister(&UFC);
; CHECK: leal -64(%ebp), %eax
@@ -99,12 +98,11 @@ try.cont:
; UFC.__jbuf[1] = $RIP
; CHECK-X64: leaq .[[RESUME:LBB[0-9]+_[0-9]+]](%rip), %rax
; CHECK-X64: movq %rax, -256(%rbp)
-; UFC.__callsite = 1
-; CHECK-X64: movl $1, -304(%rbp)
; _Unwind_SjLj_Register(&UFC);
; CHECK-X64: leaq -312(%rbp), %rcx
; CHECK-X64: callq _Unwind_SjLj_Register
; function_that_throws();
+; CHECK-X64: movl $1, -304(%rbp)
; CHECK-X64: callq _Z20function_that_throwsv
; _Unwind_SjLj_Unregister(&UFC);
; CHECK-X64: leaq -312(%rbp), %rcx
|
s-barannikov
force-pushed
the
sjlj-callsite-fix
branch
from
3281b8a to
a89eed9
Compare
s-barannikov
commented
Nov 26, 2024
| // Register the function context and make sure it's known to not throw. | ||
| CallInst *Register = Builder.CreateCall(RegisterFn, FuncCtx, ""); | ||
| Register->setDoesNotThrow(); | ||
|
|
There was a problem hiding this comment.
This NFC change wasn't strictly necessary, but with it the generated code becomes more logical/structured (setting up call sites right before associated calls).
s-barannikov
commented
Nov 26, 2024
| ; CHECK: store volatile i32 2 | ||
| ; CHECK-NEXT: call void @llvm.eh.sjlj.callsite(i32 2) | ||
| ; CHECK-NEXT: invoke void @may_throw() | ||
| invoke void @may_throw() to label %try.cont unwind label %lpad |
There was a problem hiding this comment.
IR before this change:
invoke.cont: ; preds = %entry
%call_site2 = getelementptr { ptr, i32, [4 x i32], ptr, ptr, [5 x ptr] }, ptr %fn_context, i32 0, i32 1
store volatile i32 2, ptr %call_site2, align 4
call void @llvm.eh.sjlj.callsite(i32 2)
%call_site3 = getelementptr { ptr, i32, [4 x i32], ptr, ptr, [5 x ptr] }, ptr %fn_context, i32 0, i32 1
store volatile i32 -1, ptr %call_site3, align 4
invoke void @may_throw()
to label %try.cont unwind label %lpad
nikic
approved these changes
Nov 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After 9fe78db, the pass inserts
store volatile i32 -1, ptr %call_sitebefore all invoke instruction except the one in the entry block, which has the effect of bypassing landing pads on exceptions.When configuring the call site for a potentially throwing instruction check that it is not
InvokeInst-- they are handled by earlier code.