Skip to content
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

Add 'end' at the end of 'catch' as well #73

Closed
aheejin opened this issue Dec 2, 2018 · 6 comments
Closed

Add 'end' at the end of 'catch' as well #73

aheejin opened this issue Dec 2, 2018 · 6 comments

Comments

@aheejin
Copy link
Member

aheejin commented Dec 2, 2018

#52 discussed how should folded format for try and catch would look like, and we settled on

(try
   ...
  (catch
     ...
  )
)

But in real instructions, currently this sequence is

try
  ...
catch
  ...
end

I'm wondering, can we add end at the end of catch as well, to match the text format, like this?

try
  ...
  catch
  ...
  end
end

This would increase the code size slightly, so I'm not pushing hard for this, but just would like to hear people's opinions. I'm not proposing to promote catch to a full block that can take a signature. It will not take a signature, but it will be counted as a block boundary when we compute relative depth for br, br_if (and other branch instructions that will be added in future), and rethrow.

Backstory:
It's not currently in the spec, and I'm planning to add this to the spec text soon, but anyway the plan is to add a 'depth' argument to the rethrow instruction so that it can rethrow not only to the innermost enclosing catch but also to any outer catches. This was first suggested in #29 (comment). Without this code generation for exception becomes very difficult, because we can't structure control flow for EH as in with branches with depth arguments.

So the advantage of adding end at the end of catch is, we can make rethrow's new 'depth' argument consistent. I'd like to compute rethrow's depth argument in the same way as we compute branch's depth argument, but in the current spec, it can't be done. When computing depths,

For branches,
block / loop / try: stack depth +1
end_block / end_loop / end_try : stack depth -1

For rethrows,
block / loop / try: stack depth +1
end_block / end_loop / catch: stack depth -1 (not end_try!)

try                    +1
  try                  +1
  catch                -1 only for rethrows
    br_if N
    rethrow N
  end                  -1 only for branches
catch                  -1 only for rethrows
end                    -1 only for branches

To avoid this problem, the current LLVM toolchain implementation maintains separate EH stack for rethrow depth calculation, in the way that EH stack depth is only incremented with try and decremented with catch and does not count other blocks or loops. We can do it this way, but if the code size increase for adding end at the end of catch is negligible, it would make the way of computing depth argument for branches and rethrows the same, which is more consistent.

@mstarzinger and I discussed how to compute rethrow's depth argument over email chain a little, and I'm also not sure if we settled on a conclusion.

@aheejin aheejin changed the title Add end at the end of `catch as well Add 'end' at the end of 'catch' as well Dec 2, 2018
@titzer
Copy link
Contributor

titzer commented Dec 3, 2018

Personally I think of the catch as syntactically part of a try...catch...end construct, so it would be weird to have another end. I also don't think the catch should not be considered to be nested in the try in terms of control depth; they are on the same level, similar to an if.

@aheejin
Copy link
Member Author

aheejin commented Dec 3, 2018

@titzer

I also don't think the catch should not be considered to be nested in the try in terms of control depth; they are on the same level, similar to an if.

As I wrote in the example above, for rethrows, catch is the point where level changes. Here's another example:

try
  try
    rethrow 0      // rethrows to (1)
  catch            <--- (1)
    rethrow 0      // rethrows to (2)
  end
catch              <--- (2)
end

Even if two rethrows are in the same try ~ end level and uses the same depth 0, two rethrows end up rethrowing to different catches.

I'm ok with not adding an additional end to catch. But in that case, as I said, we have to use a separate EH-pad only counting scheme to calculate rethrow's depth argument.

@titzer
Copy link
Contributor

titzer commented Dec 3, 2018

Ok, I see. I think the example actually suggests that the try...catch range is actually more deeply nested then catch...end, rather than vice versa.

@aheejin
Copy link
Member Author

aheejin commented Dec 3, 2018

Oh right... then adding end at the end of catch may be actually weirder... I'll close this issue and open another issue for rethrow's depth argument then. Thank you.

@aheejin aheejin closed this as completed Dec 3, 2018
@rossberg
Copy link
Member

rossberg commented Dec 3, 2018

I don't understand. I thought that with the current design, rethrow doesn't have any immediate anymore, and that you can simply use a regular branch?

@aheejin aheejin reopened this Dec 3, 2018
@aheejin
Copy link
Member Author

aheejin commented Dec 3, 2018

Can we continue discussion in #74, because the title of this issue is misleading?

@aheejin aheejin closed this as completed Dec 3, 2018
ioannad pushed a commit to ioannad/exception-handling that referenced this issue Jun 6, 2020
The data count section has a count that must match the number of data segments. If the data count section isn't present, then `memory.init` and `data.drop` cannot be used.

Fixes issue WebAssembly#73.
ioannad pushed a commit to ioannad/exception-handling that referenced this issue Jun 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants