-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[wasm] Use libc directly for math in the jiterpreter, implement more opcodes #82963
Conversation
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsPreviously the jiterpreter used a bunch of hand-written wrappers to call libc math functions. We can export the libc math functions directly and use them that way instead! This makes it easier and less error-prone to implement the remaining math opcodes, so I've done that. I also took this opportunity to use the f32 versions of the libc math functions when appropriate instead of doing promotion/demotion as before. The trace import section is starting to get pretty bloated, so I'll have to address that in a future PR. But this shrinks and simplifies the actual generated code for floating point math operations, which should be helpful for large traces that do math.
|
cc @BrzVlad @lambdageek if you have thoughts on a better way to structure this or make sure it matches the interpreter automatically I'd love to hear them, it's easy for errors to creep in since it's still so manual. On the other hand these opcodes probably don't change much |
93f743f
to
fe91a1a
Compare
Wouldn't exporting all these functions going to cause some problem ? |
I'm not sure, the interp already uses them. Do you think it would stop them from being inlined into C callers? |
…ppers Implement more math opcodes and use the f32 functions as appropriate
fe91a1a
to
7ba4967
Compare
@kg, we might need to keep a couple of these helpers. RyuJIT has some cmake based checks in the PAL layer to check for IEEE 754 spec required behavior that the underlying implementations sometimes miss out on: https://github.com/dotnet/runtime/blob/main/src/coreclr/pal/src/cruntime/math.cpp I believe mono (and therefore likely WASM) may have (or at least had) some of the managed tests disabled since it wasn't doing the same handling. |
-- It's possible these are all fine for WASM, in which case feel free to ignore. Just wanted to call out the potential consideration. |
I would be happy to do the work now to make sure we match all of that in a new PR, I've had to make corrections in the past to conform to spec. I'll give that file a look. The wasm native opcodes for things like min/max have specified behavior that (on my last check) matched spec but I'm not super familiar with 754 so it's possible some of them are wrong. Do we have a conformance suite in tree I could run? |
EDIT: I should also clarify that first and foremost jiterp needs to match the interpreter, otherwise we will get weird bugs. But if the interpreter doesn't match spec that's probably something we should fix too. I had to fix it when adding opcodes for Min and Max, there are dedicated helpers in interp.c for those. |
We should generally have managed coverage via the We correspondingly have native tests covering various cases here: https://github.com/dotnet/runtime/tree/main/src/coreclr/pal/tests/palsuite/c_runtime For results that are expected to produce -- If you do the "Institutional Sign In" on https://ieeexplore.ieee.org/Xplore/home.jsp using your -- Many of the same requirements are reiterated in |
Thanks! The extensions suite passes with jiterpreter set to aggressive mode, and I can see compile failures for a few math opcodes that aren't implemented right now like coshf, so it seems like things are working OK. I will keep this all in mind if we get any bug reports related to math, and add a to-do item to go over the spec and try to verify how conformant emscripten's libc is. |
Thanks a ton! |
Previously the jiterpreter used a bunch of hand-written wrappers to call libc math functions. We can export the libc math functions directly and use them that way instead! This makes it easier and less error-prone to implement the remaining math opcodes, so I've done that. I also took this opportunity to use the f32 versions of the libc math functions when appropriate instead of doing promotion/demotion as before.
This shrinks and simplifies the actual generated code for floating point math operations, which should be helpful for large traces that do math.