[PATCH] Initial SEH IR generation implementation

Reid Kleckner rnk at google.com
Fri Jan 16 14:14:47 PST 2015


================
Comment at: lib/CodeGen/CGException.cpp:1702
@@ +1701,3 @@
+llvm::Constant *
+CodeGenFunction::GenerateSEHFilterFunction() {
+  // Get the mangled function name.
----------------
rjmccall wrote:
> Can you at least document what the point of this is?
Sure, check out the comment.

================
Comment at: lib/CodeGen/CGException.cpp:1702
@@ +1701,3 @@
+llvm::Constant *
+CodeGenFunction::GenerateSEHFilterFunction() {
+  // Get the mangled function name.
----------------
rnk wrote:
> rjmccall wrote:
> > Can you at least document what the point of this is?
> Sure, check out the comment.
Sure, see the new comment.

================
Comment at: lib/CodeGen/CGException.cpp:1772
@@ +1771,3 @@
+
+  if (SEHFinallyStmt *Finally = S.getFinallyHandler()) {
+    // SEH cleanups should be simple.
----------------
rjmccall wrote:
> Doesn't the finally handler need to be active within the catch handler?  You probably need to push it first.
A `__try` can only have one handler, `__except` or `__finally`, and not both. I'll straighten out the control flow to clarify that.

================
Comment at: lib/CodeGen/CGException.cpp:1821
@@ +1820,3 @@
+    }
+    Builder.CreateCall(FilterIntrin, R);
+
----------------
rjmccall wrote:
> Shouldn't you be branching on the filter result or something?  It feels wrong for the control flow here to be implicit.
The idea in this patch is that we enter the landing pad, and we sort of non-determinstically already know what the selector is. The `@llvm.eh.seh.filter` call acts as a barrier against code motion of side effects. It's kind of like a suspend / resume point in a coroutine. What happens in practice is that the code between the typeid check and the `@llvm.eh.seh.filter` call gets outlined.

Now that I'm thinking about this harder, I'll admit this is kind of broken for nested cleanups, as the cleanups will run before the filter, which is not what actually happens in practice (filters run during phase 1). =/

What do you think about going back to outlining filters in the frontend? I've had many weeks of coffee and whiteboard discussions trying to figure out the least disruptive way to fit filter functions into LLVM's EH scheme, and we haven't come up with anything good. We can still do backend outlining for cleanups and C++ EH. LLVM's EH model just doesn't have a place for code that gets executed during phase 1 EH unwinding.

================
Comment at: lib/CodeGen/CGException.cpp:1830
@@ +1829,3 @@
+  }
+
+  if (SEHFinallyStmt *Finally = S.getFinallyHandler()) {
----------------
rjmccall wrote:
> Yeah, you're popping it after you pop the catch.  That can't possibly work if you pushed the catch first.
> 
> This also suggests that you aren't testing that adequately. :)
As mentioned, this is impossible, but it's a reasonable comment. :)

================
Comment at: lib/Sema/SemaChecking.cpp:467
@@ +466,3 @@
+  case Builtin::BI_exception_code: {
+    Scope *S = getCurScope();
+    while (S && !S->isSEHExceptScope())
----------------
rjmccall wrote:
> This checking can run in template instantiations, but getCurScope() isn't meaningful there.
Right. :( This is always a syntactic nesting property, though. Instantiation cannot change it. Should I just check it during the parse (if so, where?), or just suppress this Sema check during instantiation, since it will always give the same result?

http://reviews.llvm.org/D5607

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list