[PATCH] Initial SEH IR generation implementation

John McCall rjmccall at gmail.com
Tue Jan 20 18:13:39 PST 2015


================
Comment at: lib/CodeGen/CGException.cpp:1772
@@ +1771,3 @@
+
+  if (SEHFinallyStmt *Finally = S.getFinallyHandler()) {
+    // SEH cleanups should be simple.
----------------
rnk wrote:
> 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.
Okay, makes sense.

================
Comment at: lib/CodeGen/CGException.cpp:1821
@@ +1820,3 @@
+    }
+    Builder.CreateCall(FilterIntrin, R);
+
----------------
rnk wrote:
> 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.
I think outlining filters is an acceptable implementation model given the oddity of the control flow.  Can we get away with just disallowing references to local variables in them for now, or is that an important use case?

================
Comment at: lib/Sema/SemaChecking.cpp:467
@@ +466,3 @@
+  case Builtin::BI_exception_code: {
+    Scope *S = getCurScope();
+    while (S && !S->isSEHExceptScope())
----------------
rnk wrote:
> 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?
In theory, you can construct builtin calls in template instantiations with e.g. non-type template arguments.  It's probably safe to just not diagnose that well as long as you don't crash during IRGen if you miss a case.

You don't really have a choice about where you detect this; there *are* often separate Sema entrypoints for parsing vs. instantiation, but we don't want to burden every function call parse with an extra check just for this.  I think Sema tracks that's currently instantiating a function body, but I don't remember where.

http://reviews.llvm.org/D5607

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






More information about the cfe-commits mailing list