================
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/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
