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

wasm2c SIMD final approach checklist #2224

Closed
9 tasks done
keithw opened this issue May 3, 2023 · 6 comments
Closed
9 tasks done

wasm2c SIMD final approach checklist #2224

keithw opened this issue May 3, 2023 · 6 comments

Comments

@keithw
Copy link
Member

keithw commented May 3, 2023

simd-everywhere has just merged fixes to pass the Wasm testsuite (simd-everywhere/simde#1010), so I think we're close to being able to close this out, cut a release, and check the box on webassembly.org. This will also be the first time in a while that wasm2c is passing 100% of the current Wasm core testsuite (which we should enjoy until tail-calls get merged).

Here's my thinking/proposal on next steps to get there:

  • Update our simde submodule pointer
  • Add the five missing tests
  • Check the SIMD box in the wasm2c README
  • Augment the IR to track whether SIMD is actually used in the module (later we can do the same for exceptions)
  • Include SIMD support in the generated code only if the module uses it + SIMD is enabled
  • Maybe controversial: When SIMD is being used, include the simde header in the generated code (like we do with the scalar operations) so the user doesn't need to have a compatible dev repo of SIMDe in their include path to compile the wasm2c output.
  • Eliminate WASM_RT_ENABLE_SIMD preprocessor macro and conditionally #include <simde/wasm/simd128> instead.
    • CWriter tracks whether SIMD is used in the module's external interface to decide whether the #include should be in the header file or only the C file (avoid pulling all of SIMDe simd128.h into the embedder's namespace unless necessary)
  • Cut a 1.0.33 release
  • Submit a PR to WebAssembly/website that wasm2c 1.0.33 supports SIMD (and extended-const)

@wrv @sbc100 @shravanrn wdyt?

@mr-c
Copy link
Contributor

mr-c commented May 4, 2023

Maybe controversial: When SIMD is being used, include the simde header in the generated code (like we do with the scalar operations) so the user doesn't need to have a compatible dev repo of SIMDe in their include path to compile the wasm2c output.

This is fine by me, and from the Debian side it would be nice to have a switch to easily use an installed copy of SIMDe instead of the bundled amalgated version.

@keithw
Copy link
Member Author

keithw commented May 5, 2023

This is fine by me, and from the Debian side it would be nice to have a switch to easily use an installed copy of SIMDe instead of the bundled amalgated version.

Sounds reasonable... maybe --external-simde or something?

@mr-c
Copy link
Contributor

mr-c commented May 5, 2023

Sounds reasonable... maybe --external-simde or something?

Oh, I was thinking at compile time of wasm2c output one would use -DEXTERNAL_SIMDE=/usr/include/ or similar

@keithw
Copy link
Member Author

keithw commented May 5, 2023

After attempting it, I'm realizing that pasting the simde header into the wasm2c generated code is going to be painful because:

  • if the module's public interface has a public-facing v128, or contains a v128 global, we have to include the header in the generated .h file (not just the .c file), and
  • the amalgamated header is 17,000 lines

Even if I try to split out the v128 definition into its own header, it still ends up pulling in about 8,000 lines because it wants simde-common.h which wants a bunch of stuff. I'm not seeing an easy way to get simde_v128 robustly without thousands of lines. And I don't really want wasm2c to be pasting all that into a generated .h file. :-(

Maaaybe the better path forward is really just to have an #include <simde/wasm/simd128.h> (in either the .h file or the .c file, depending on whether the public interface includes a v128 or there's a v128 global) and tell the user they'll need to have a compatible version of simde on hand to compile the wasm2c output. :-/

@shravanrn
Copy link
Collaborator

Seems reasonable. Based on the above, I suspect us taking ownership of amalgamating simd is going to lead to a bunch of work. Would be happy to include it as part of the output, but it would probably be easier to leave their file structure as is.

@keithw
Copy link
Member Author

keithw commented Jul 12, 2023

Merged in WebAssembly/website#334

@keithw keithw closed this as completed Jul 12, 2023
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