-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[KeyInstr][Clang] Catch variable init atom #134641
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
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Orlando Cazalet-Hyams (OCHyams) ChangesThis patch is part of a stack that teaches Clang to generate Key Instructions The Key Instructions project is introduced, including a "quick summary" section The feature is only functional in LLVM if LLVM is built with CMake flag The Clang-side work is demoed here: Full diff: https://github.com/llvm/llvm-project/pull/134641.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 77e995b4c933a..b3f1e208fa318 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -5051,6 +5051,7 @@ void ItaniumCXXABI::emitBeginCatch(CodeGenFunction &CGF,
// Emit the local.
CodeGenFunction::AutoVarEmission var = CGF.EmitAutoVarAlloca(*CatchParam);
+ ApplyAtomGroup Grp(CGF.getDebugInfo());
InitCatchParam(CGF, *CatchParam, var.getObjectAddress(CGF), S->getBeginLoc());
CGF.EmitAutoVarCleanups(var);
}
diff --git a/clang/test/KeyInstructions/try-catch.cpp b/clang/test/KeyInstructions/try-catch.cpp
new file mode 100644
index 0000000000000..3d1080aca2f07
--- /dev/null
+++ b/clang/test/KeyInstructions/try-catch.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang -gkey-instructions %s -gmlt -S -emit-llvm -o - -fexceptions \
+// RUN: | FileCheck %s
+
+void except() {
+ // FIXME(OCH): Should `store i32 32, ptr %exception` be key?
+ throw 32;
+}
+
+void attempt() {
+ try { except(); }
+// CHECK: catch:
+// CHECK: %4 = call ptr @__cxa_begin_catch(ptr %exn)
+// CHECK: %5 = load i32{{.*}}, !dbg [[G1R2:!.*]]
+// CHECK: store i32 %5, ptr %e{{.*}}, !dbg [[G1R1:!.*]]
+// CHECK: call void @__cxa_end_catch()
+ catch (int e) { }
+}
+
+// CHECK: [[G1R2]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 2)
+// CHECK: [[G1R1]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)
|
c25359b
to
58f7f4d
Compare
638b319
to
54eca71
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, with a question inline.
@@ -5055,6 +5055,7 @@ void ItaniumCXXABI::emitBeginCatch(CodeGenFunction &CGF, | |||
|
|||
// Emit the local. | |||
CodeGenFunction::AutoVarEmission var = CGF.EmitAutoVarAlloca(*CatchParam); | |||
ApplyAtomGroup Grp(CGF.getDebugInfo()); | |||
InitCatchParam(CGF, *CatchParam, var.getObjectAddress(CGF), S->getBeginLoc()); | |||
CGF.EmitAutoVarCleanups(var); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Risk that a destructor for the catch-parameter could be given this instruction group?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes if a few things were different and wrong. 1. Calls aren't annotated. 2. If they were, it would be a bug if whatever call handling code didn't have its own ApplyAtomGroup
for the dtor.
I could add a scope around ApplyAtomGroup and InitCatchParam, and/or add more tests, if you think it would be useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm -- yes please for the braces, no worries about the tests. This is just about reducing the amount of potentially-unexpected behaviour
58f7f4d
to
5d45280
Compare
This patch is part of a stack that teaches Clang to generate Key Instructions metadata for C and C++. The Key Instructions project is introduced, including a "quick summary" section at the top which adds context for this PR, here: https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668 The feature is only functional in LLVM if LLVM is built with CMake flag LLVM_EXPERIMENTAL_KEY_INSTRUCTIONs. Eventually that flag will be removed. The Clang-side work is demoed here: #130943
46e356a
to
5d308dd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(LGTM with the braces)
This patch is part of a stack that teaches Clang to generate Key Instructions
metadata for C and C++.
The Key Instructions project is introduced, including a "quick summary" section
at the top which adds context for this PR, here:
https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668
The feature is only functional in LLVM if LLVM is built with CMake flag
LLVM_EXPERIMENTAL_KEY_INSTRUCTIONs. Eventually that flag will be removed.
The Clang-side work is demoed here:
#130943