Skip to content

[browser] Fix SIMD+EH check #92348

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

Merged
merged 5 commits into from
Sep 21, 2023
Merged

[browser] Fix SIMD+EH check #92348

merged 5 commits into from
Sep 21, 2023

Conversation

maraf
Copy link
Member

@maraf maraf commented Sep 20, 2023

  • Before this change the linkerWasmEnableSIMD and linkerWasmEnableEH used in configureRuntimeStartup to check support for these features always had the default (true) value. Throwing assert error even when SIMD and EH was disabled for build.
  • Call cwraps.mono_wasm_abort from runtimeHelpers.abort only after cwraps are ready (onRuntimeInitializedAsync).

@maraf maraf added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm labels Sep 20, 2023
@maraf maraf added this to the 9.0.0 milestone Sep 20, 2023
@maraf maraf requested a review from lewing as a code owner September 20, 2023 15:29
@maraf maraf self-assigned this Sep 20, 2023
@ghost
Copy link

ghost commented Sep 20, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Before this change the linkerWasmEnableSIMD and linkerWasmEnableEH used in configureRuntimeStartup to check support for these features always had the default (true) value. Throwing assert error even when SIMD and EH was disabled for build.
  • Call cwraps.mono_wasm_abort from runtimeHelpers.abort only after cwraps are ready (onRuntimeInitializedAsync).
Author: maraf
Assignees: maraf
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript, os-browser

Milestone: 9.0.0

@radical
Copy link
Member

radical commented Sep 20, 2023

Before this change the linkerWasmEnableSIMD and linkerWasmEnableEH used in configureRuntimeStartup to check support for these features always had the default (true) value. Throwing assert error even when SIMD and EH was disabled for build.

So, this always failed on browsers that didn't support SIMD/EH, even when SIMD was disabled in the build? Can we test this with chrome/v8?

@maraf
Copy link
Member Author

maraf commented Sep 21, 2023

So, this always failed on browsers that didn't support SIMD/EH, even when SIMD was disabled in the build?

Exactly.

Can we test this with chrome/v8?

If we install Chrome 85 or Firefox 88, we can test it. I can look into it in a separate PR.

@lewing lewing merged commit 49930c1 into dotnet:main Sep 21, 2023
@lewing
Copy link
Member

lewing commented Sep 21, 2023

/backport to release/8.0

@github-actions
Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6265063819

@lewing
Copy link
Member

lewing commented Sep 21, 2023

/backport to release/8.0-rc2

@github-actions
Copy link
Contributor

Started backporting to release/8.0-rc2: https://github.com/dotnet/runtime/actions/runs/6267696867

@lewing
Copy link
Member

lewing commented Sep 21, 2023

closed in favor of #92439

@maraf maraf deleted the WasmSimdCheck branch September 22, 2023 07:23
@ghost ghost locked as resolved and limited conversation to collaborators Oct 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants