Wasm jiterpreter cleanup and bug fixes pt. 3 #78782
Conversation
kg
requested review from
lewing,
pavelsavara,
vargaz,
lambdageek and
BrzVlad
as code owners
|
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsBlocks #78428
|
|
Tagging subscribers to this area: @BrzVlad Issue DetailsBlocks #78428
|
| if (!isF64) | ||
| builder.appendU8(intrinsicFpBinop); | ||
| builder.i32_const(<any>opcode); | ||
| builder.callImport("relop_fp"); |
There was a problem hiding this comment.
Do I understand correctly this is call from generated wasm into main wasm file ? is this expensive ?
There was a problem hiding this comment.
Yes, it's calling into dotnet.wasm - in this case, mono_jiterp_relop_fp. It is somewhat expensive, but the alternative is painful. In the long run if the performance of these comparisons was sensitive we'd generate all the ordering comparison logic inline.
| // in the correct place and compute the stack offset, then it passes that in to this | ||
| // function in order to actually enter the interpreter and process the return value | ||
| EMSCRIPTEN_KEEPALIVE void | ||
| mono_jiterp_interp_entry (JiterpEntryData *_data, stackval *sp_args, void *res) |
There was a problem hiding this comment.
Another option here would have been to make interp_exec_method static again as it was before the original jiterpreter commit and then have a global mono_interp_exec_method that calls it.
There was a problem hiding this comment.
Do you want me to change it? I originally had a wrapper like you describe, but the performance impact was measurable so that's how we ended up here. I might be able to get it to inline though, I don't think I spent much time trying when I was first prototyping.
ghost
locked as resolved and limited conversation to collaborators
Blocks #78428