Skip to content

[release/7.0][wasm] SIMD related build fixes #75042

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 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion eng/testing/scenarios/BuildWasmAppsJobsList.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ Wasm.Build.Tests.RebuildTests
Wasm.Build.Tests.SatelliteAssembliesTests
Wasm.Build.Tests.WasmBuildAppTest
Wasm.Build.Tests.WasmNativeDefaultsTests
Wasm.Build.Tests.WorkloadTests
Wasm.Build.Tests.WasmRunOutOfAppBundleTests
Wasm.Build.Tests.WasmSIMDTests
Wasm.Build.Tests.WasmTemplateTests
Wasm.Build.Tests.WorkloadTests

Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

<PropertyGroup Condition="'$(TargetsNet7)' == 'true' and '$(RuntimeIdentifier)' == 'browser-wasm' AND '$(UsingBrowserRuntimeWorkload)' == ''">
<!-- $(WasmBuildNative)==true is needed to enable workloads, when using native references, without AOT -->
<UsingBrowserRuntimeWorkload Condition="'$(RunAOTCompilation)' == 'true' or '$(WasmBuildNative)' == 'true' or '$(WasmGenerateAppBundle)' == 'true' or '$(UsingMicrosoftNETSdkBlazorWebAssembly)' != 'true'" >true</UsingBrowserRuntimeWorkload>
<UsingBrowserRuntimeWorkload Condition="'$(RunAOTCompilation)' == 'true' or '$(WasmEnableSIMD)' == 'true' or '$(WasmBuildNative)' == 'true' or '$(WasmGenerateAppBundle)' == 'true' or '$(UsingMicrosoftNETSdkBlazorWebAssembly)' != 'true'" >true</UsingBrowserRuntimeWorkload>
<UsingBrowserRuntimeWorkload Condition="'$(UsingBrowserRuntimeWorkload)' == ''" >$(WasmNativeWorkload7)</UsingBrowserRuntimeWorkload>
</PropertyGroup>

Expand Down
7 changes: 5 additions & 2 deletions src/mono/wasm/build/WasmApp.Native.targets
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@
<!-- build AOT, only if explicitly requested -->
<WasmBuildNative Condition="'$(RunAOTCompilation)' == 'true' and '$(RunAOTCompilationAfterBuild)' == 'true'">true</WasmBuildNative>

<WasmBuildNative Condition="'$(WasmBuildNative)' == '' and '$(WasmEnableSIMD)' == 'true'">true</WasmBuildNative>

<WasmBuildNative Condition="'$(WasmBuildNative)' == '' and @(NativeFileReference->Count()) > 0" >true</WasmBuildNative>

<WasmBuildNative Condition="'$(WasmBuildNative)' == ''">false</WasmBuildNative>
Expand All @@ -118,6 +120,7 @@
<!-- AOT==true overrides WasmBuildNative -->
<WasmBuildNative Condition="'$(RunAOTCompilation)' == 'true'">true</WasmBuildNative>
<WasmBuildNative Condition="'$(WasmBuildNative)' == '' and @(NativeFileReference->Count()) > 0" >true</WasmBuildNative>
<WasmBuildNative Condition="'$(WasmBuildNative)' == '' and '$(WasmEnableSIMD)' == 'true'">true</WasmBuildNative>

<!-- not aot, not trimmed app, no reason to relink -->
<WasmBuildNative Condition="'$(WasmBuildNative)' == '' and '$(PublishTrimmed)' != 'true'">false</WasmBuildNative>
Expand Down Expand Up @@ -195,7 +198,7 @@
<_EmccCommonFlags Include="-v" Condition="'$(EmccVerbose)' != 'false'" />
<_EmccCommonFlags Include="-s DISABLE_EXCEPTION_CATCHING=0" Condition="'$(WasmExceptionHandling)' == 'false'" />
<_EmccCommonFlags Include="-fwasm-exceptions" Condition="'$(WasmExceptionHandling)' == 'true'" />
<_EmccCommonFlags Include="-msimd128" Condition="'$(WasmSIMD)' == 'true'" />
<_EmccCommonFlags Include="-msimd128" Condition="'$(WasmEnableSIMD)' == 'true'" />

<_EmccIncludePaths Include="$(_WasmIntermediateOutputPath.TrimEnd('\/'))" />
<_EmccIncludePaths Include="$(_WasmRuntimePackIncludeDir)mono-2.0" />
Expand Down Expand Up @@ -524,7 +527,7 @@
<MonoAOTCompilerDefaultAotArguments Include="static" />
<MonoAOTCompilerDefaultAotArguments Include="direct-icalls" />
<MonoAOTCompilerDefaultAotArguments Include="deterministic" />
<MonoAOTCompilerDefaultAotArguments Include="mattr=simd" Condition="'$(WasmSIMD)' == 'true'" />
<MonoAOTCompilerDefaultAotArguments Include="mattr=simd" Condition="'$(WasmEnableSIMD)' == 'true'" />
<MonoAOTCompilerDefaultProcessArguments Include="--wasm-exceptions" Condition="'$(WasmExceptionHandling)' == 'true'" />
<MonoAOTCompilerDefaultProcessArguments Include="--wasm-gc-safepoints" Condition="'$(WasmEnableThreads)' == 'true' or '$(WasmEnablePerfTracing)' == 'true'" />
<AotProfilePath Include="$(WasmAotProfilePath)"/>
Expand Down
4 changes: 2 additions & 2 deletions src/mono/wasm/build/WasmApp.targets
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
Defaults to false.
- $(WasmAotProfilePath) - Path to an AOT profile file.
- $(WasmExceptionHandling) - Enable support for the WASM Exception Handling feature.
- $(WasmSIMD) - Enable support for the WASM SIMD feature.
- $(WasmEnableSIMD) - Enable support for the WASM SIMD feature.
Copy link
Member

Choose a reason for hiding this comment

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

I wanted to keep it similar to other feature props, like the WasmExceptionHandling above. Shouldn't we then change that too?


Public items:
- @(WasmExtraFilesToDeploy) - Files to copy to $(WasmAppDir).
Expand All @@ -87,7 +87,7 @@
<PropertyGroup>
<WasmDedup Condition="'$(WasmDedup)' == ''">false</WasmDedup>
<WasmExceptionHandling Condition="'$(WasmExceptionHandling)' == ''">false</WasmExceptionHandling>
<WasmSIMD Condition="'$(WasmSIMD)' == ''">false</WasmSIMD>
<WasmEnableSIMD Condition="'$(WasmEnableSIMD)' == ''">false</WasmEnableSIMD>

<!--<WasmStripAOTAssemblies Condition="'$(AOTMode)' == 'LLVMOnlyInterp'">false</WasmStripAOTAssemblies>-->
<!--<WasmStripAOTAssemblies Condition="'$(WasmStripAOTAssemblies)' == ''">$(RunAOTCompilation)</WasmStripAOTAssemblies>-->
Expand Down
5 changes: 4 additions & 1 deletion src/tests/BuildWasmApps/Wasm.Build.Tests/BuildTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ protected string RunAndTestWasmApp(BuildArgs buildArgs,
Dictionary<string, string>? envVars = null,
string targetFramework = DefaultTargetFramework,
string? extraXHarnessMonoArgs = null,
string? extraXHarnessArgs = null,
string jsRelativePath = "test-main.js")
{
buildDir ??= _projectDir;
Expand All @@ -159,13 +160,15 @@ protected string RunAndTestWasmApp(BuildArgs buildArgs,
throw new InvalidOperationException("Running tests with V8 on windows isn't supported");

// Use wasm-console.log to get the xharness output for non-browser cases
(string testCommand, string extraXHarnessArgs, bool useWasmConsoleOutput) = host switch
(string testCommand, string xharnessArgs, bool useWasmConsoleOutput) = host switch
{
RunHost.V8 => ("wasm test", $"--js-file={jsRelativePath} --engine=V8 -v trace", true),
RunHost.NodeJS => ("wasm test", $"--js-file={jsRelativePath} --engine=NodeJS -v trace", true),
_ => ("wasm test-browser", $"-v trace -b {host} --web-server-use-cop", false)
};

extraXHarnessArgs += " " + xharnessArgs;

string testLogPath = Path.Combine(_logPath, host.ToString());
string output = RunWithXHarness(
testCommand,
Expand Down
130 changes: 110 additions & 20 deletions src/tests/BuildWasmApps/Wasm.Build.Tests/WasmSIMDTests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO;
using Xunit;
using Xunit.Abstractions;

Expand All @@ -16,25 +17,114 @@ public WasmSIMDTests(ITestOutputHelper output, SharedBuildPerTestClassFixture bu
}

[Theory]
[MemberData(nameof(MainMethodTestData), parameters: new object[] { /*aot*/ true, RunHost.All })]
public void BuildWithSIMD(BuildArgs buildArgs, RunHost host, string id)
=> TestMain("main_simd_aot",
@"
using System;
using System.Runtime.Intrinsics;

public class TestClass {
public static int Main()
{
var v1 = Vector128.Create(0x12345678);
var v2 = Vector128.Create(0x23456789);
var v3 = v1*v2;
Console.WriteLine(v3);
Console.WriteLine(""Hello, World!"");

return 42;
}
}",
buildArgs, host, id, extraProperties: "<WasmSIMD>true</WasmSIMD>");
[MemberData(nameof(MainMethodTestData), parameters: new object[] { /*aot*/ false, RunHost.All })]
public void BuildWithSIMD_NoAOT_ShouldRelink(BuildArgs buildArgs, RunHost host, string id)
{
string projectName = $"sim_with_workload_no_aot";
buildArgs = buildArgs with { ProjectName = projectName };
buildArgs = ExpandBuildArgs(buildArgs, "<WasmEnableSIMD>true</WasmEnableSIMD>");

(_, string output) = BuildProject(buildArgs,
id: id,
new BuildProjectOptions(
InitProject: () => File.WriteAllText(Path.Combine(_projectDir!, "Program.cs"), s_simdProgramText),
Publish: false,
DotnetWasmFromRuntimePack: false));

if (!_buildContext.TryGetBuildFor(buildArgs, out _))
{
// Check if this is not a cached build
Assert.Contains("Compiling native assets with excc", output);
}

RunAndTestWasmApp(buildArgs,
extraXHarnessArgs: host == RunHost.NodeJS ? "--engine-arg=--experimental-wasm-simd" : "",
expectedExitCode: 42,
test: output =>
{
Assert.Contains("<-2094756296, -2094756296, -2094756296, -2094756296>", output);
Assert.Contains("Hello, World!", output);
}, host: host, id: id);
}

[Theory]
// https://github.com/dotnet/runtime/issues/75044 - disabled for V8, and NodeJS
//[MemberData(nameof(MainMethodTestData), parameters: new object[] { /*aot*/ true, RunHost.All })]
[MemberData(nameof(MainMethodTestData), parameters: new object[] { /*aot*/ true, RunHost.Chrome })]
[MemberData(nameof(MainMethodTestData), parameters: new object[] { /*aot*/ false, RunHost.All })]
public void PublishWithSIMD_AOT(BuildArgs buildArgs, RunHost host, string id)
{
string projectName = $"sim_with_workload_aot";
buildArgs = buildArgs with { ProjectName = projectName };
buildArgs = ExpandBuildArgs(buildArgs, "<WasmEnableSIMD>true</WasmEnableSIMD>");

BuildProject(buildArgs,
id: id,
new BuildProjectOptions(
InitProject: () => File.WriteAllText(Path.Combine(_projectDir!, "Program.cs"), s_simdProgramText),
DotnetWasmFromRuntimePack: false));

RunAndTestWasmApp(buildArgs,
extraXHarnessArgs: host == RunHost.NodeJS ? "--engine-arg=--experimental-wasm-simd" : "",
expectedExitCode: 42,
test: output =>
{
Assert.Contains("<-2094756296, -2094756296, -2094756296, -2094756296>", output);
Assert.Contains("Hello, World!", output);
}, host: host, id: id);
}

[Theory, TestCategory("no-workload")]
[InlineData("Debug", /*aot*/true, /*publish*/true)]
[InlineData("Debug", /*aot*/false, /*publish*/false)]
[InlineData("Debug", /*aot*/false, /*publish*/true)]
[InlineData("Release", /*aot*/true, /*publish*/true)]
[InlineData("Release", /*aot*/false, /*publish*/false)]
[InlineData("Release", /*aot*/false, /*publish*/true)]
public void BuildWithSIMDNeedsWorkload(string config, bool aot, bool publish)
{
string id = Path.GetRandomFileName();
string projectName = $"simd_no_workload_{config}_aot_{aot}";
BuildArgs buildArgs = new
(
ProjectName: projectName,
Config: config,
AOT: aot,
ProjectFileContents: "placeholder",
ExtraBuildArgs: string.Empty
);

string extraProperties = """
<RuntimeIdentifier>browser-wasm</RuntimeIdentifier>
<WasmEnableSIMD>true</WasmEnableSIMD>
""";
buildArgs = ExpandBuildArgs(buildArgs, extraProperties);

(_, string output) = BuildProject(buildArgs,
id: id,
new BuildProjectOptions(
InitProject: () => File.WriteAllText(Path.Combine(_projectDir!, "Program.cs"), s_simdProgramText),
Publish: publish,
ExpectSuccess: false,
UseCache: false));
Assert.Contains("following workloads must be installed: wasm-tools", output);
}

private static string s_simdProgramText = @"
using System;
using System.Runtime.Intrinsics;

public class TestClass {
public static int Main()
{
var v1 = Vector128.Create(0x12345678);
var v2 = Vector128.Create(0x23456789);
var v3 = v1*v2;
Console.WriteLine(v3);
Console.WriteLine(""Hello, World!"");

return 42;
}
}";
}
}