-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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] Add test checking that disabiling SIMD actually disables it #89302
Comments
@kg do we need |
I think |
Without -msimd128 no code that uses wasm intrinsics will compile, even if we don't run it. So we would need to ifdef everything relevant out I think. Part of the problem is that -msimd128 isn't just an 'enable support' flag, unfortunately it turns a bunch of other stuff on by default and that's probably the source of the sections or opcodes that are breaking things for people. I don't think the switch specifically needs to be there; I tried a bunch of things to try and get SIMD to work and that was the only simple solution I found that worked. |
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsThe SIMD cannot be disabled anymore. These are the 2 parts we need to fix:
optionally:
following step:
|
This is the first part of changes to make wasm/SIMD optional again, dotnet#89302
For mono libs and src/mono/CMakeLists.txt I will try to extract the interp simd code to a separate library, similar to what we do for exception handling, and link it conditionally. |
* [wasm] Do not set -msimd128 in the default rsp This is the first part of changes to make wasm/SIMD optional again, #89302 * Feedback * Feedback * Update the tests, we force relink with SIMD disabled now
remaining parts can be done in net9 |
Hello, My project settings:
At program launch i'm getting bunch of errors like this:
not sure what i can do more to get rid of SIMD support in .Net8 |
@KubaGluszkiewicz Just a note, use lower case |
|
I have it on my list. Using chrome 85 on CI should be enough |
Running wasm-opt without --enable- is an easy way to see what is in use |
We have customers hitting this on Windows 10 machines. What is the timescale for the fix? |
@paulguz-datapa Disabling SIMD should be working, they this issue open only to make an automated test. Are you setting both |
@maraf Yes, those flags work, thanks. |
Closing as completed if this is incorrect please reopen |
@maraf unfortunately your fix does not fix our problem.
Unfortunately, this does not solve the actual problem. Our test environment runs on "Microsoft Windows Server 2019 Standard - 10.0.17763" is a Virtual Machine with the processor "Intel64 Family 6 Model 85 Stepping 7 GenuineIntel" (Unfortunately without SIMD support). Unfortunately, I am not allowed to share the project code with you, so I have created a sample application that at least also outputs a SIMD error message. Unfortunately I can't get the same error as in the project above. It works on my personal machine ;) Nevertheless, I suspect that SIMD is still included in the published Version somewhere. |
@luccawilli If I understand it correctly, you are able to reproduce the error in your test environment, right? Can we please verify that https://maraf.github.io/dotnet-wasm-nosimd/ works on the machine without SIMD? |
@maraf correct, your example works without an error. But it does not use any Pages or Components. |
It's plain WebAssembly without blazor. And your sample with Blazor doesn't work https://github.com/luccawilli/dotnet-wasm-nosimd, right? |
@maraf Yes correct, mine does not work. And yes i installed wasm-tools. |
Thanks! I'm getting the same "AllowRange" error on my machine. I'll investigate it |
I have created demonstration projects showing how to build Blazor WASM apps with both SIMD enabled and disabled. And then loading the appropriate build at runtime based on the environment. Blazor Web App SIMD Detect Example Blazor WebAssembly Standalone SIMD Detect Example |
@luccawilli The "AllowRange" error comes from the Jiterpreter that is testing what capabilities it can use. We'll prepare a fix for it not to test SIMD on runtime when it was disabled during compilation. Anyway, with disable SIMD during build ( <BlazorWebAssemblyRuntimeOptions>--no-jiterpreter-simd-enabled --no-jiterpreter-wasm-eh-enabled</BlazorWebAssemblyRuntimeOptions> |
I found that disabling SIMD without also disabling BlazorWebAssemblyJiterpreter gave an error but still allowed the app to run. @maraf You "cc@kg" my above comment on that thread. maraf quoting my above comment |
You actually don't need to disable the whole jiterpreter, just the SIMD and EH |
@maraf Thank you for the investigation. As i see the error in my test app has vanished :) Unfortunately, the change does not yet solve any problems in our main project. I will try to improve my test app, so that i can show you my main problem. |
So I have improved my example. It now shows the same error that we get in our main project. I also added the publish result and the folder publish profile. The error shown in my test system: No errors on my system (with SIMD support) (except for the missing icon) @maraf Can you also investigate this one? Thanks alot :) |
Why are you setting WasmBuildNative=false? This setting disables relinking .NET runtime and thus makes all the other settings void. We are improving the experience in 9 to produce an error when incompatible settings are used |
Oh damn, okay alright. I didn't know that. But I think I can ignore that, since it only needs to be published in my Docker-Image. Thank you once again |
The SIMD cannot be disabled anymore. These are the 2 parts we need to fix:
-msimd128
toemcc-default.rsp
, otherwise we compile with SIMD even whenWasmEnableSIMD
isfalse
. Instead add it conditionally insrc/mono/wasm/wasm.proj
,src/mono/wasm/runtime/CMakeLists.txt
andsrc/mono/wasm/build/WasmApp.Native.targets
-msimd128
or provide multiple native libraries versionsoptionally:
wasm-opt
. I think this doesn't add any SIMD instructions, so we probably don't need it. We should check it though.following step:
.wasm
files without SIMD instructions. This can be done easily once we have thewa-info
tool in the runtime repo.The text was updated successfully, but these errors were encountered: