Skip to content

[browser] Fix SIMD+EH check - #92348

Merged
lewing merged 5 commits into
dotnet:mainfrom
maraf:WasmSimdCheck
Sep 21, 2023
Merged

lewing merged 5 commits into
dotnet:mainfrom
maraf:WasmSimdCheck

Conversation

@maraf

@maraf maraf commented Sep 20, 2023

Copy link
Copy Markdown
Member
  • 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

ghost commented Sep 20, 2023

Copy link
Copy Markdown

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

Comment thread src/mono/wasm/runtime/startup.ts Outdated
Comment thread src/mono/wasm/runtime/startup.ts Outdated
@radical

radical commented Sep 20, 2023

Copy link
Copy Markdown
Member

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?

Comment thread src/mono/wasm/runtime/startup.ts Outdated
@maraf

maraf commented Sep 21, 2023 •

Copy link
Copy Markdown
Member Author

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

lewing commented Sep 21, 2023

Copy link
Copy Markdown
Member

/backport to release/8.0

@github-actions

Copy link
Copy Markdown
Contributor

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

@lewing

lewing commented Sep 21, 2023

Copy link
Copy Markdown
Member

/backport to release/8.0-rc2

@github-actions

Copy link
Copy Markdown
Contributor

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

@lewing

lewing commented Sep 21, 2023

Copy link
Copy Markdown
Member

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