-
Notifications
You must be signed in to change notification settings - Fork 833
[EH] Add instructions for new proposal #6181
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
Changes from all commits
cdb8eb9
424c799
168b984
9026623
8bb08e5
54b4e1c
51e4efd
6eaa08c
b3888f8
19a3a12
5fe611d
a1568de
89a5e46
21407d7
5420500
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,14 +75,22 @@ void operateOnScopeNameUsesAndSentTypes(Expression* expr, T func) { | |
| func(name, sw->value ? sw->value->type : Type::none); | ||
| } else if (auto* br = expr->dynCast<BrOn>()) { | ||
| func(name, br->getSentType()); | ||
| } else if (auto* tt = expr->dynCast<TryTable>()) { | ||
| for (Index i = 0; i < tt->catchTags.size(); i++) { | ||
| auto dest = tt->catchDests[i]; | ||
| if (dest == name) { | ||
| func(name, tt->sentTypes[i]); | ||
| } | ||
| } | ||
| } else { | ||
| assert(expr->is<Try>() || expr->is<Rethrow>()); // delegate or rethrow | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| // Similar to operateOnScopeNameUses, but also passes in the expression that is | ||
| // sent if the branch is taken. nullptr is given if there is no value. | ||
| // sent if the branch is taken. nullptr is given if there is no value or there | ||
| // is a value but it is not known statically. | ||
| template<typename T> | ||
| void operateOnScopeNameUsesAndSentValues(Expression* expr, T func) { | ||
| operateOnScopeNameUses(expr, [&](Name& name) { | ||
|
|
@@ -94,6 +102,10 @@ void operateOnScopeNameUsesAndSentValues(Expression* expr, T func) { | |
| func(name, sw->value); | ||
| } else if (auto* br = expr->dynCast<BrOn>()) { | ||
| func(name, br->ref); | ||
| } else if (auto* tt = expr->dynCast<TryTable>()) { | ||
| // The values are supplied by throwing instructions, so we are unable to | ||
| // know what they will be here. | ||
| func(name, nullptr); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before,
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| } else { | ||
| assert(expr->is<Try>() || expr->is<Rethrow>()); // delegate or rethrow | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -706,6 +706,11 @@ class EffectAnalyzer { | |
| parent.delegateTargets.insert(curr->delegateTarget); | ||
| } | ||
| } | ||
| void visitTryTable(TryTable* curr) { | ||
| for (auto name : curr->catchDests) { | ||
| parent.breakTargets.insert(name); | ||
| } | ||
| } | ||
| void visitThrow(Throw* curr) { | ||
| if (parent.tryDepth == 0) { | ||
| parent.throws_ = true; | ||
|
|
@@ -715,6 +720,11 @@ class EffectAnalyzer { | |
| if (parent.tryDepth == 0) { | ||
| parent.throws_ = true; | ||
| } | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to have truncated Rethrow's handling?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No that was intentional. So current
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, thanks! Makes sense. |
||
| void visitThrowRef(ThrowRef* curr) { | ||
| if (parent.tryDepth == 0) { | ||
| parent.throws_ = true; | ||
| } | ||
| // traps when the arg is null | ||
| parent.implicitTrap = true; | ||
| } | ||
|
|
||
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.
Why is this comparing to
name? The name should only be provided tofunc(which can modify it) IIUC.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.
Sorry I think I was confused. Fixed it.
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.
No I think the original code was correct...
namegiven here is a block label, and we should runfunconly when thenameis the label we are targeting. For example:In this case, when analyzing
TryTable, we shouldn't send bothi32and(i32 exnref)to a givenname, which will mean the givennamehas two different types. We send the type only when the givennamematches itssentType's label.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.
I see, thanks. So this is called multiple times for a single
TryTable, once per name, and we need to handle the right name each time.Maybe worth handling
TryTableoutside of theoperateOnScopeNameUsesto avoid the potentially quadratic work? But if we think that will be incredibly rare maybe just a comment is enough.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.
Even though theoretically there can be any number of
catches, in practice there is no reason to use more than two (one for one ofcatch/catch_refand the other for one ofcatch_all/catch_all_ref), unless you are trying to catch multiple language tags. And in LLVM we don't even generate twocatches: We only generate onecatchorcatch_allfor a singletrydue to the structure of Windows EH IR, and I think this will be also the case for the new spec. Even though we assume a non-LLVM multi-language handling case I doubt this will go more than 3.