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

Support try-catch in Asyncify / lower wasm exceptions to MVP #5343

Open
kripken opened this issue Dec 13, 2022 · 7 comments
Open

Support try-catch in Asyncify / lower wasm exceptions to MVP #5343

kripken opened this issue Dec 13, 2022 · 7 comments

Comments

@kripken
Copy link
Member

kripken commented Dec 13, 2022

See discussion in #5143 - lowering wasm exceptions to wasm MVP might be the best approach for this.

@caiiiycuk
Copy link
Contributor

Sorry, still misunderstand this. What is old js exceptions? asmjs exceptions?

wasm+asyncify didn't support any exceptions even simplest one? like

try{
//...
} catch {
//...
}

I just trying to understand, is dosbox-x exceptions handling not working because of bug, or because try/catch is not implemented (this task).

@kripken
Copy link
Member Author

kripken commented Dec 13, 2022

There are two modes, see https://emscripten.org/docs/porting/exceptions.html

Basically the old mode is -fexceptions. It works with Asyncify (but there might be bugs). The new mode is -fwasm-exceptions. That is much faster and smaller, but it doesn't work with Asyncify yet.

@caiiiycuk
Copy link
Contributor

Hi @tlively. I have motivation to implement this. I hope I don't bother you with some questions. From bird's-eye the lower wasm exceptions to MVP seems doable.

My first idea is to replace the try blocks with just a block with br statement. When a br occurs, I store the exception data somewhere in memory and then process the blocks associated with the catches until exception is consumed.

Something like this:

**FROM**
try 
  throw ...
catch
  ...

**TO**
block
  // store exception data
  br 1

// catch realted to thrown exception
block
  // load exception data

I excuse that it very raw, but I don't want to implement something not working :)

Anyway,

  1. when we up to function level, after each function call we need to check if we processing some exception right now. This means that I need to put some logic after each call/call_indirect instruction. Do you think is it will bloat the binary very fast?
  2. try/delegate instruction, I don't have an idea how to handle it, it breaks the linear execution flow, and makes this simple idea not working. Maybe there is option to not generate them, but again it will bloat binary.
  3. Can we just get implementation from wasm2c? Even more what happen if I do wasm2c -> c2wasm loop, can it work as workaorund?

@tlively
Copy link
Member

tlively commented Jan 9, 2023

This sounds like a good direction. I do think that having to check for exceptions after every call would bloat the binary, but it may not be avoidable. Maybe you could do an analysis to find calls that cannot possibly throw exceptions to skip instrumenting them.

For try-delegate, I expect you will have to put in more branch instructions to transfer control to the correct handler block.

Doing a wasm2c -> c2wasm loop would be a good idea, too, although implementing exceptions in wasm2c would require solving a lot of the same problems.

caiiiycuk added a commit to caiiiycuk/binaryen-fwasm-exceptions that referenced this issue Feb 2, 2023
caiiiycuk added a commit to caiiiycuk/binaryen-fwasm-exceptions that referenced this issue Feb 2, 2023
caiiiycuk added a commit to caiiiycuk/binaryen-fwasm-exceptions that referenced this issue Feb 2, 2023
caiiiycuk added a commit to caiiiycuk/binaryen-fwasm-exceptions that referenced this issue Feb 2, 2023
caiiiycuk added a commit to caiiiycuk/binaryen-fwasm-exceptions that referenced this issue Feb 2, 2023
caiiiycuk added a commit to caiiiycuk/binaryen-fwasm-exceptions that referenced this issue Feb 2, 2023
caiiiycuk added a commit to caiiiycuk/binaryen-fwasm-exceptions that referenced this issue Feb 2, 2023
caiiiycuk added a commit to caiiiycuk/binaryen-fwasm-exceptions that referenced this issue Mar 11, 2023
caiiiycuk added a commit to caiiiycuk/binaryen-fwasm-exceptions that referenced this issue Jun 22, 2023
caiiiycuk added a commit to caiiiycuk/binaryen-fwasm-exceptions that referenced this issue Nov 2, 2023
@badgermole
Copy link

There are two modes, see https://emscripten.org/docs/porting/exceptions.html

Basically the old mode is -fexceptions. It works with Asyncify (but there might be bugs). The new mode is -fwasm-exceptions. That is much faster and smaller, but it doesn't work with Asyncify yet.

Is there any update on Asyncify and -fwasm-exceptions working together?

@caiiiycuk
Copy link
Contributor

There are two modes, see https://emscripten.org/docs/porting/exceptions.html
Basically the old mode is -fexceptions. It works with Asyncify (but there might be bugs). The new mode is -fwasm-exceptions. That is much faster and smaller, but it doesn't work with Asyncify yet.

Is there any update on Asyncify and -fwasm-exceptions working together?

Well, if an exception never occurs in the asyncify code path (meaning no exceptions in the call stack of emscripten_sleep), then you can use the code from the my PR #5475 . It works well; I've used it with DOSBox-X. Additionally, I have prebuilt binaries of wasm-opt available. You can find them here.

@badgermole
Copy link

Thanks very much @caiiiycuk, I wish I could.

caiiiycuk added a commit to caiiiycuk/binaryen-fwasm-exceptions that referenced this issue Jun 10, 2024
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

4 participants