Skip to content

[release/8.0-rc2] [browser] Fix SIMD+EH check #92422

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

Closed
github-actions wants to merge 10 commits into from
Closed

[release/8.0-rc2] [browser] Fix SIMD+EH check #92422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
49bf70a
Update dependencies from https://github.com/dotnet/source-build-exter…
dotnet-maestro[bot] Sep 20, 2023
1361b18
Update dependencies from dotnet/emsdk
radical Sep 20, 2023
02d408e
[wasm] InstallWorkloadFromArtifacts: correctly handle rtm versions
radical Sep 20, 2023
df1cf0c
Merge pull request #92325 from dotnet-maestro-bot/merge/release/8.0-r…
carlossanlop Sep 21, 2023
76252f3
[release/8.0] Update dependencies from dotnet/roslyn (#92303)
dotnet-maestro[bot] Sep 21, 2023
780aa4f
Move simd+eh check after emcripten provided build time values
maraf Sep 20, 2023
75c250f
Call cwraps.mono_wasm_abort from runtimeHelpers.abort when cwraps are…
maraf Sep 20, 2023
cad253e
Assign early mono_wasm_exit and abort even earlier
maraf Sep 20, 2023
4259361
Fire feature check before awaiting wasm download, but await it after.
maraf Sep 21, 2023
1ae155f
Whitespaces
maraf Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions eng/Version.Details.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,19 @@
<Sha>89be445dd4936157533ad96bafb95f701430653a</Sha>
<SourceBuild RepoName="cecil" ManagedOnly="true" />
</Dependency>
<Dependency Name="Microsoft.NET.Workload.Emscripten.Current.Manifest-8.0.100.Transport" Version="8.0.0-rc.2.23469.4">
<Dependency Name="Microsoft.NET.Workload.Emscripten.Current.Manifest-8.0.100.Transport" Version="8.0.0-rtm.23470.1">
Copy link
Member

@radical radical Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be changed from -rtm to -rc2 too.

<Uri>https://github.com/dotnet/emsdk</Uri>
<Sha>ea0e8e8214e9acc0cba7e78a836ed6656f788d11</Sha>
<Sha>446eeb331fcbf2f48c14a377601a8ab950ec942e</Sha>
<SourceBuild RepoName="emsdk" ManagedOnly="true" />
</Dependency>
<Dependency Name="Microsoft.SourceBuild.Intermediate.source-build-reference-packages" Version="8.0.0-alpha.1.23469.1">
<Uri>https://github.com/dotnet/source-build-reference-packages</Uri>
<Sha>d825c6693d4e26f63aaa93c3c1d057faa098e347</Sha>
<SourceBuild RepoName="source-build-reference-packages" ManagedOnly="true" />
</Dependency>
<Dependency Name="Microsoft.SourceBuild.Intermediate.source-build-externals" Version="8.0.0-alpha.1.23468.1">
<Dependency Name="Microsoft.SourceBuild.Intermediate.source-build-externals" Version="8.0.0-alpha.1.23469.1">
<Uri>https://github.com/dotnet/source-build-externals</Uri>
<Sha>e9d6489787a5ea5400a31dfa34aa6ad6b590de9b</Sha>
<Sha>c42a7ce3b6fa02957e7b4ef995c5c2a9a23d294c</Sha>
<SourceBuild RepoName="source-build-externals" ManagedOnly="true" />
</Dependency>
</ProductDependencies>
Expand Down Expand Up @@ -358,18 +358,18 @@
<Uri>https://github.com/dotnet/runtime-assets</Uri>
<Sha>99168dcff56809205e7ef8530d1256f3a07fab1f</Sha>
</Dependency>
<Dependency Name="Microsoft.Net.Compilers.Toolset" Version="4.8.0-3.23469.1">
<Dependency Name="Microsoft.Net.Compilers.Toolset" Version="4.8.0-3.23471.2">
Copy link
Member

@radical radical Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these being updated in this PR?

Copy link
Contributor

@carlossanlop carlossanlop Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect the target branch change caused this: the PR was initially targeting release/8.0.. Good catch.

If you don't mind, can you push the right fix to this PR so that we can get this merged sooner, @radical?

Copy link
Contributor

@carlossanlop carlossanlop Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually hold on @radical - I think because the original branch itself was targeting release/8.0 it brought many more commits that are not wanted here, not just the ones you're pointing out. @pavelsavara can you please take care of it yourself? Reset your local branch to point to rc2, then cherry-pick your changes, and force push to this PR.

Copy link
Member

@radical radical Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent catch!

<Uri>https://github.com/dotnet/roslyn</Uri>
<Sha>9233e36abc5e2ca263dbd4d1616f35623440a935</Sha>
<Sha>9f6f97debe7fe8121a2249f807343d12ef1e4ac6</Sha>
</Dependency>
<Dependency Name="Microsoft.CodeAnalysis" Version="4.8.0-3.23469.1">
<Dependency Name="Microsoft.CodeAnalysis" Version="4.8.0-3.23471.2">
<Uri>https://github.com/dotnet/roslyn</Uri>
<Sha>9233e36abc5e2ca263dbd4d1616f35623440a935</Sha>
<Sha>9f6f97debe7fe8121a2249f807343d12ef1e4ac6</Sha>
<SourceBuild RepoName="roslyn" ManagedOnly="true" />
</Dependency>
<Dependency Name="Microsoft.CodeAnalysis.CSharp" Version="4.8.0-3.23469.1">
<Dependency Name="Microsoft.CodeAnalysis.CSharp" Version="4.8.0-3.23471.2">
<Uri>https://github.com/dotnet/roslyn</Uri>
<Sha>9233e36abc5e2ca263dbd4d1616f35623440a935</Sha>
<Sha>9f6f97debe7fe8121a2249f807343d12ef1e4ac6</Sha>
</Dependency>
<Dependency Name="Microsoft.CodeAnalysis.Analyzers" Version="3.11.0-beta1.23468.1">
<Uri>https://github.com/dotnet/roslyn-analyzers</Uri>
Expand Down
8 changes: 4 additions & 4 deletions eng/Versions.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@
Any tools that contribute to the design-time experience should use the MicrosoftCodeAnalysisVersion_LatestVS property above to ensure
they do not break the local dev experience.
-->
<MicrosoftCodeAnalysisCSharpVersion>4.8.0-3.23469.1</MicrosoftCodeAnalysisCSharpVersion>
<MicrosoftCodeAnalysisVersion>4.8.0-3.23469.1</MicrosoftCodeAnalysisVersion>
<MicrosoftNetCompilersToolsetVersion>4.8.0-3.23469.1</MicrosoftNetCompilersToolsetVersion>
<MicrosoftCodeAnalysisCSharpVersion>4.8.0-3.23471.2</MicrosoftCodeAnalysisCSharpVersion>
<MicrosoftCodeAnalysisVersion>4.8.0-3.23471.2</MicrosoftCodeAnalysisVersion>
<MicrosoftNetCompilersToolsetVersion>4.8.0-3.23471.2</MicrosoftNetCompilersToolsetVersion>
</PropertyGroup>
<!--
For source generator support we need to target multiple versions of Roslyn in order to be able to run on older versions of Roslyn.
Expand Down Expand Up @@ -238,7 +238,7 @@
Note: when the name is updated, make sure to update dependency name in eng/pipelines/common/xplat-setup.yml
like - DarcDependenciesChanged.Microsoft_NET_Workload_Emscripten_Current_Manifest-8_0_100_Transport
-->
<MicrosoftNETWorkloadEmscriptenCurrentManifest80100TransportVersion>8.0.0-rc.2.23469.4</MicrosoftNETWorkloadEmscriptenCurrentManifest80100TransportVersion>
<MicrosoftNETWorkloadEmscriptenCurrentManifest80100TransportVersion>8.0.0-rtm.23470.1</MicrosoftNETWorkloadEmscriptenCurrentManifest80100TransportVersion>
<MicrosoftNETRuntimeEmscriptenVersion>$(MicrosoftNETWorkloadEmscriptenCurrentManifest80100TransportVersion)</MicrosoftNETRuntimeEmscriptenVersion>
<!-- workloads -->
<SwixPackageVersion>1.1.87-gba258badda</SwixPackageVersion>
Expand Down
6 changes: 6 additions & 0 deletions src/mono/wasm/runtime/globals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ export function setRuntimeGlobals(globalObjects: GlobalObjects) {
beforeOnRuntimeInitialized: createPromiseController<void>(),
afterOnRuntimeInitialized: createPromiseController<void>(),
afterPostRun: createPromiseController<void>(),
mono_wasm_exit: () => {
throw new Error("Mono shutdown");
},
abort: (reason: any) => {
throw reason;
}
});

Object.assign(globalObjects.module.config!, {}) as any;
Expand Down
38 changes: 22 additions & 16 deletions src/mono/wasm/runtime/startup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,7 @@ import { assertNoProxies } from "./gc-handles";
const MONO_PTHREAD_POOL_SIZE = 4;

export async function configureRuntimeStartup(): Promise<void> {
if (linkerWasmEnableSIMD) {
mono_assert(await loaderHelpers.simd(), "This browser/engine doesn't support WASM SIMD. Please use a modern version. See also https://aka.ms/dotnet-wasm-features");
}
if (linkerWasmEnableEH) {
mono_assert(await loaderHelpers.exceptions(), "This browser/engine doesn't support WASM exception handling. Please use a modern version. See also https://aka.ms/dotnet-wasm-features");
}

await init_polyfills_async();

await checkMemorySnapshotSize();
}

Expand Down Expand Up @@ -240,6 +232,15 @@ async function onRuntimeInitializedAsync(userOnRuntimeInitialized: () => void) {
// wait for previous stage
await runtimeHelpers.afterPreRun.promise;
mono_log_debug("onRuntimeInitialized");

runtimeHelpers.mono_wasm_exit = cwraps.mono_wasm_exit;
runtimeHelpers.abort = (reason: any) => {
if (!loaderHelpers.is_exited()) {
cwraps.mono_wasm_abort();
}
throw reason;
};

const mark = startMeasure();
// signal this stage, this will allow pending assets to allocate memory
runtimeHelpers.beforeOnRuntimeInitialized.promise_control.resolve();
Expand Down Expand Up @@ -361,13 +362,6 @@ function mono_wasm_pre_init_essential(isWorker: boolean): void {
}

init_c_exports();
runtimeHelpers.mono_wasm_exit = cwraps.mono_wasm_exit;
runtimeHelpers.abort = (reason: any) => {
if (!loaderHelpers.is_exited()) {
cwraps.mono_wasm_abort();
}
throw reason;
};
cwraps_internal(INTERNAL);
if (WasmEnableLegacyJsInterop && !linkerDisableLegacyJsInterop) {
cwraps_mono_api(MONO);
Expand All @@ -386,7 +380,6 @@ async function mono_wasm_pre_init_essential_async(): Promise<void> {
mono_log_debug("mono_wasm_pre_init_essential_async");
Module.addRunDependency("mono_wasm_pre_init_essential_async");


if (MonoWasmThreads) {
preAllocatePThreadWorkerPool(MONO_PTHREAD_POOL_SIZE, runtimeHelpers.config);
}
Expand Down Expand Up @@ -466,8 +459,12 @@ async function instantiate_wasm_module(
await runtimeHelpers.beforePreInit.promise;
Module.addRunDependency("instantiate_wasm_module");

const wasmFeaturePromise = ensureUsedWasmFeatures();

replace_linker_placeholders(imports);
const assetToLoad = await loaderHelpers.wasmDownloadPromise.promise;

await wasmFeaturePromise;
await instantiate_wasm_asset(assetToLoad, imports, successCallback);
assetToLoad.pendingDownloadInternal = null as any; // GC
assetToLoad.pendingDownload = null as any; // GC
Expand Down Expand Up @@ -499,6 +496,15 @@ async function instantiate_wasm_module(
Module.removeRunDependency("instantiate_wasm_module");
}

async function ensureUsedWasmFeatures() {
if (linkerWasmEnableSIMD) {
mono_assert(await loaderHelpers.simd(), "This browser/engine doesn't support WASM SIMD. Please use a modern version. See also https://aka.ms/dotnet-wasm-features");
}
if (linkerWasmEnableEH) {
mono_assert(await loaderHelpers.exceptions(), "This browser/engine doesn't support WASM exception handling. Please use a modern version. See also https://aka.ms/dotnet-wasm-features");
}
}

async function mono_wasm_before_memory_snapshot() {
const mark = startMeasure();
if (runtimeHelpers.loadedMemorySnapshotSize) {
Expand Down
2 changes: 1 addition & 1 deletion src/tasks/WorkloadBuildTasks/InstallWorkloadFromArtifacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public partial class InstallWorkloadFromArtifacts : Task
private string _tempDir = string.Empty;
private string _nugetCachePath = string.Empty;

[GeneratedRegex(@"^\d+\.\d+\.\d+(-[A-z]*\.*\d*)?")]
[GeneratedRegex(@"^\d+\.\d+\.\d+(-rtm|-[A-z]*\.*\d*)?")]
private static partial Regex bandVersionRegex();

public override bool Execute()
Expand Down