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.

Sign up for GitHub

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

Jump to bottom

Support -msse4 alias to -msse4.2 #12746

Closed
RReverser opened this issue Nov 9, 2020 · 5 comments · Fixed by #16750
Closed

Support -msse4 alias to -msse4.2 #12746

RReverser opened this issue Nov 9, 2020 · 5 comments · Fixed by #16750
Labels
good first bug SIMD

Comments

@RReverser
Copy link
Collaborator

Just ran into this while porting a real project. On native x86 target Clang supports -msse4 as an alias for -msse4.2, however Emscripten seems to ignore it and pass as-is to the underlying Clang (making it no-op). cc @juj @tlively @sbc100
@juj
Copy link
Collaborator

juj commented Nov 10, 2020

Supporting -msse4 sounds good to me.

RReverser added a commit to RReverser/libwebp2 that referenced this issue Nov 10, 2020
@RReverser
Couple of fixes to allow SIMD on Emscripten
9f03807 
 - Switch from `-msse` to `-msse4.2`. In theory, those should be identical, but Emscripten currently has a bug where only latter form is supported: emscripten-core/emscripten#12746. Since we're using only SSE 4.2, an explicit form seems to be preferable anyway.
 - Add `-msimd128` to flags to actually enable WebAssembly SIMD when performing SIMD detection. It's currently required in addition to `-msse*` / `-mfpu=neon` flags which only perform translation of corresponding intrinsics to Wasm SIMD ones. See a discussion at emscripten-core/emscripten#12714 for automating this and making easier in the future.
 - Remove compilation branch that prevented definitions of `WP2_USE_SSE` and `WP2_USE_NEON` on Emscripten even when SIMD support was detected at compile-time.
 - Add an implementation of `WP2GetCPUInfo` for Emscripten which uses static `WP2_USE_*` flags to determine if a corresponding SIMD instruction is supported. This is because Wasm doesn't have proper feature detection (yet) and requires making separate build for SIMD version anyway.
RReverser added a commit to RReverser/libwebp2 that referenced this issue Nov 10, 2020
@RReverser
Couple of fixes to allow SIMD on Emscripten
2b263fe 
 - Switch from `-msse` to `-msse4.2`. In theory, those should be identical, but Emscripten currently has a bug where only latter form is supported: emscripten-core/emscripten#12746. Since we're using only SSE 4.2, an explicit form seems to be preferable anyway.
 - Add `-msimd128` to flags to actually enable WebAssembly SIMD when performing SIMD detection. It's currently required in addition to `-msse*` / `-mfpu=neon` flags which only perform translation of corresponding intrinsics to Wasm SIMD ones. See a discussion at emscripten-core/emscripten#12714 for automating this and making easier in the future.
 - Remove compilation branch that prevented definitions of `WP2_USE_SSE` and `WP2_USE_NEON` on Emscripten even when SIMD support was detected at compile-time.
 - Add an implementation of `WP2GetCPUInfo` for Emscripten which uses static `WP2_USE_*` flags to determine if a corresponding SIMD instruction is supported. This is because Wasm doesn't have proper feature detection (yet) and requires making separate build for SIMD version anyway.
RReverser added a commit to RReverser/libwebp2 that referenced this issue Nov 10, 2020
@RReverser
Couple of fixes to allow SIMD on Emscripten
f8911dc 
 - Switch from `-msse` to `-msse4.2`. In theory, those should be identical, but Emscripten currently has a bug where only latter form is supported: emscripten-core/emscripten#12746. Since we're using only SSE 4.2, an explicit form seems to be preferable anyway.
 - Add `-msimd128` to flags to actually enable WebAssembly SIMD when performing SIMD detection. It's currently required in addition to `-msse*` / `-mfpu=neon` flags which only perform translation of corresponding intrinsics to Wasm SIMD ones. See a discussion at emscripten-core/emscripten#12714 for automating this and making easier in the future.
 - Remove compilation branch that prevented definitions of `WP2_USE_SSE` and `WP2_USE_NEON` on Emscripten even when SIMD support was detected at compile-time.
 - Add an implementation of `WP2GetCPUInfo` for Emscripten which uses static `WP2_USE_*` flags to determine if a corresponding SIMD instruction is supported. This is because Wasm doesn't have proper feature detection (yet) and requires making separate build for SIMD version anyway.
@giatorta
Copy link

Hi, I would like to take this issue as my "first bug", if it's ok I'd have two questions:

  • what does it mean "Emscripten ignore it and pass as-is to the underlying Clang (making it no-op)" ?
    wouldn't passing it to Clang make Clang process it correctly as an alias?

  • I guess emcc is the program that is involved in calling Clang and that should be modified to fix the issue?

thank you for the advice!

@RReverser
Copy link
Collaborator Author

@giatorta Best way is to look at places in Emscripten codebase where "-msse4.2" occurs and see how to add "-msse4" there so that it would behave in the same way.

@giatorta
Copy link

@RReverser thanks for the hint, I think I now understand what I have to fix. Currently working on it in my fork of the repo...

giatorta added a commit to giatorta/emscripten that referenced this issue Nov 19, 2020
@giatorta
Fix missing support for -msse4 alias
678f980 
See emscripten-core#12746
@giatorta giatorta mentioned this issue Nov 19, 2020
Closed
giatorta added a commit to giatorta/emscripten that referenced this issue Nov 24, 2020
@giatorta
Fix missing support for -msse4 alias
11fad9d 
See emscripten-core#12746
giatorta added a commit to giatorta/emscripten that referenced this issue Nov 24, 2020
@giatorta
Fix missing support for -msse4 alias
e877c05 
See emscripten-core#12746
@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Apr 17, 2022
tlively added a commit that referenced this issue Apr 18, 2022
@tlively
Support -msse4 as an alias for -msse4.2
8865a8b 
Also update some of the test decorators to support parameterized tests so that
test_sse4 can be parameterized to use either -msse4 or -msse4.2.

Fixes #12746.
@tlively tlively mentioned this issue Apr 18, 2022
Merged
tlively added a commit that referenced this issue Apr 18, 2022
@tlively
Support -msse4 as an alias for -msse4.2 (#16750)
4146033 
Also update some of the test decorators to support parameterized tests so that
test_sse4 can be parameterized to use either -msse4 or -msse4.2.

Fixes #12746.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first bug SIMD
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support -msse4 as an alias for -msse4.2 emscripten-core/emscripten
4 participants
@juj @RReverser @giatorta @tlively