Skip to content

Commit

Permalink
[wasm] Fix regression in compiling .bc -> .o files (#56063)
Browse files Browse the repository at this point in the history
* [wasm] Add back --emit-llvm that got removed mistakenly, in an earlier commit

.. found thanks to Jerome Laban.

* [wasm] Set EmccCompile's messages to MessageImportance.Low by default.

.. and to MessageImportance.Normal if `$(EmccVerbose)==true`.

* [wasm] Quote filenames passed to emcc compile command line

* Add more blazorwasm tests - for debug/release, aot/relinking

* Bump sdk for workload testing to 6.0.100-rc.1.21370.2

* [wasm] Fix regression in compiling bitcode -> .o

The `-emit-llvm` arg has been incorrectly added, and removed from the
args used for compiling .bc->.o .

This commit fixes it, and adds a crude test for it, so we don't regress
again.

* Fix build

(cherry picked from commit 1d8ad03)
  • Loading branch information
radical committed Jul 22, 2021
1 parent 78773d1 commit e56e13b
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 16 deletions.
2 changes: 1 addition & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@
<SQLitePCLRawbundle_greenVersion>2.0.4</SQLitePCLRawbundle_greenVersion>
<MoqVersion>4.12.0</MoqVersion>
<FsCheckVersion>2.14.3</FsCheckVersion>
<SdkVersionForWorkloadTesting>6.0.100-preview.7.21362.5</SdkVersionForWorkloadTesting>
<SdkVersionForWorkloadTesting>6.0.100-rc.1.21370.2</SdkVersionForWorkloadTesting>
<!-- Docs -->
<MicrosoftPrivateIntellisenseVersion>5.0.0-preview-20201009.2</MicrosoftPrivateIntellisenseVersion>
<!-- ILLink -->
Expand Down
14 changes: 11 additions & 3 deletions src/mono/wasm/build/WasmApp.Native.targets
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@
<EmccLinkOptimizationFlag Condition="'$(EmccLinkOptimizationFlag)' == ''" >$(EmccCompileOptimizationFlag)</EmccLinkOptimizationFlag>

<_EmccCompileRsp>$(_WasmIntermediateOutputPath)emcc-compile.rsp</_EmccCompileRsp>
<_EmccCompileOutputMessageImportance Condition="'$(EmccVerbose)' == 'true'">Normal</_EmccCompileOutputMessageImportance>
<_EmccCompileOutputMessageImportance Condition="'$(EmccVerbose)' != 'true'">Low</_EmccCompileOutputMessageImportance>
</PropertyGroup>

<ItemGroup>
Expand All @@ -181,6 +183,7 @@
<_EmccCFlags Include="-DLINK_ICALLS=1" Condition="'$(WasmLinkIcalls)' == 'true'" />
<_EmccCFlags Include="-DCORE_BINDINGS" />
<_EmccCFlags Include="-DGEN_PINVOKE=1" />
<_EmccCFlags Include="-emit-llvm" />

<_EmccCFlags Include="&quot;-I%(_EmccIncludePaths.Identity)&quot;" />
<_EmccCFlags Include="-g" Condition="'$(WasmNativeDebugSymbols)' == 'true'" />
Expand Down Expand Up @@ -240,7 +243,11 @@
<Exec Command="$(_EmBuilder) build MINIMAL" EnvironmentVariables="@(EmscriptenEnvVars)" StandardOutputImportance="Low" StandardErrorImportance="Low" />

<Message Text="Compiling native assets with emcc. This may take a while ..." Importance="High" />
<EmccCompile SourceFiles="@(_WasmSourceFileToCompile)" Arguments='"@$(_EmccDefaultFlagsRsp)" "@$(_EmccCompileRsp)"' EnvironmentVariables="@(EmscriptenEnvVars)" />
<EmccCompile
SourceFiles="@(_WasmSourceFileToCompile)"
Arguments='"@$(_EmccDefaultFlagsRsp)" "@$(_EmccCompileRsp)"'
EnvironmentVariables="@(EmscriptenEnvVars)"
OutputMessageImportance="$(_EmccCompileOutputMessageImportance)" />

<ItemGroup>
<WasmNativeAsset Include="%(_WasmSourceFileToCompile.ObjectFile)" />
Expand Down Expand Up @@ -269,8 +276,9 @@
<EmccCompile
Condition="@(_BitCodeFile->Count()) > 0"
SourceFiles="@(_BitCodeFile)"
Arguments="&quot;@$(_EmccDefaultFlagsRsp)&quot; &quot;@$(_EmccCompileRsp)&quot;"
EnvironmentVariables="@(EmscriptenEnvVars)" />
Arguments="&quot;@$(_EmccDefaultFlagsRsp)&quot; @(_EmccLDFlags, ' ')"
EnvironmentVariables="@(EmscriptenEnvVars)"
OutputMessageImportance="$(_EmccCompileOutputMessageImportance)" />

<ItemGroup>
<!-- order seems to matter -->
Expand Down
11 changes: 9 additions & 2 deletions src/tasks/WasmAppBuilder/EmccCompile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public class EmccCompile : Microsoft.Build.Utilities.Task
public bool DisableParallelCompile { get; set; }
public string Arguments { get; set; } = string.Empty;
public string? WorkingDirectory { get; set; }
public string OutputMessageImportance{ get; set; } = "Low";

[Output]
public ITaskItem[]? OutputFiles { get; private set; }
Expand All @@ -54,6 +55,12 @@ public override bool Execute()
return false;
}

if (!Enum.TryParse(OutputMessageImportance, ignoreCase: true, out MessageImportance messageImportance))
{
Log.LogError($"Invalid value for OutputMessageImportance={OutputMessageImportance}. Valid values: {string.Join(", ", Enum.GetNames(typeof(MessageImportance)))}");
return false;
}

IDictionary<string, string> envVarsDict = GetEnvironmentVariablesDict();
ConcurrentBag<ITaskItem> outputItems = new();
try
Expand Down Expand Up @@ -112,7 +119,7 @@ bool ProcessSourceFile(ITaskItem srcItem)

try
{
string command = $"emcc {Arguments} -c -o {objFile} {srcFile}";
string command = $"emcc {Arguments} -c -o \"{objFile}\" \"{srcFile}\"";

// Log the command in a compact format which can be copy pasted
StringBuilder envStr = new StringBuilder(string.Empty);
Expand All @@ -125,7 +132,7 @@ bool ProcessSourceFile(ITaskItem srcItem)
envVarsDict,
workingDir: Environment.CurrentDirectory,
logStdErrAsMessage: true,
debugMessageImportance: MessageImportance.High,
debugMessageImportance: messageImportance,
label: Path.GetFileName(srcFile));

if (exitCode != 0)
Expand Down
18 changes: 13 additions & 5 deletions src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,16 @@ public BlazorWasmTests(ITestOutputHelper output, SharedBuildPerTestClassFixture
{
}

[ConditionalFact(typeof(BuildTestBase), nameof(IsUsingWorkloads))]
public void PublishTemplateProject()
// TODO: invariant case?

[ConditionalTheory(typeof(BuildTestBase), nameof(IsUsingWorkloads))]
[InlineData("Debug", false)]
[InlineData("Debug", true)] // just aot
[InlineData("Release", false)] // should re-link
[InlineData("Release", true)]
public void PublishTemplateProject(string config, bool aot)
{
string id = "blazorwasm";
string id = $"blazorwasm_{config}_aot_{aot}";
InitPaths(id);
if (Directory.Exists(_projectDir))
Directory.Delete(_projectDir, recursive: true);
Expand All @@ -37,14 +43,16 @@ public void PublishTemplateProject()
.ExecuteWithCapturedOutput("new blazorwasm")
.EnsureSuccessful();

string publishLogPath = Path.Combine(logPath, $"{id}.publish.binlog");
string publishLogPath = Path.Combine(logPath, $"{id}.binlog");
new DotNetCommand(s_buildEnv)
.WithWorkingDirectory(_projectDir)
.ExecuteWithCapturedOutput("publish", $"-bl:{publishLogPath}", "-p:RunAOTCompilation=true")
.ExecuteWithCapturedOutput("publish", $"-bl:{publishLogPath}", aot ? "-p:RunAOTCompilation=true" : "", $"-p:Configuration={config}")
.EnsureSuccessful();

//TODO: validate the build somehow?
// compare dotnet.wasm?
// relinking - dotnet.wasm should be smaller
//
// playwright?
}
}
Expand Down
3 changes: 0 additions & 3 deletions src/tests/BuildWasmApps/Wasm.Build.Tests/BuildTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -332,10 +332,7 @@ protected static BuildArgs ExpandBuildArgs(BuildArgs buildArgs, string extraProp
}

if (useCache)
{
_buildContext.CacheBuild(buildArgs, new BuildProduct(_projectDir, logFilePath, true));
Console.WriteLine($"caching build for {buildArgs}");
}

return (_projectDir, result.buildOutput);
}
Expand Down
34 changes: 33 additions & 1 deletion src/tests/BuildWasmApps/Wasm.Build.Tests/NativeBuildTests.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

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

#nullable enable

Expand Down Expand Up @@ -43,5 +43,37 @@ private void NativeBuild(string projectNamePrefix, string projectContents, Build
test: output => {},
host: host, id: id);
}

[Theory]
[BuildAndRun(host: RunHost.None, aot: true)]
public void IntermediateBitcodeToObjectFilesAreNotLLVMIR(BuildArgs buildArgs, string id)
{
string printFileTypeTarget = @"
<Target Name=""PrintIntermediateFileType"" AfterTargets=""WasmBuildApp"">
<Exec Command=""wasm-dis $(_WasmIntermediateOutputPath)System.Private.CoreLib.dll.o -o $(_WasmIntermediateOutputPath)wasm-dis-out.txt""
ConsoleToMSBuild=""true""
EnvironmentVariables=""@(EmscriptenEnvVars)""
IgnoreExitCode=""true"">
<Output TaskParameter=""ExitCode"" PropertyName=""ExitCode"" />
</Exec>
<Message Text=""wasm-dis exit code: $(ExitCode)"" Importance=""High"" />
</Target>
";
string projectName = $"bc_to_o_{buildArgs.Config}";

buildArgs = buildArgs with { ProjectName = projectName };
buildArgs = ExpandBuildArgs(buildArgs, insertAtEnd: printFileTypeTarget);

(_, string output) = BuildProject(buildArgs,
initProject: () => File.WriteAllText(Path.Combine(_projectDir!, "Program.cs"), s_mainReturns42),
dotnetWasmFromRuntimePack: false,
id: id);

if (!output.Contains("wasm-dis exit code: 0"))
throw new XunitException($"Expected to successfully run wasm-dis on System.Private.CoreLib.dll.o ."
+ " It might fail if it was incorrectly compiled to a bitcode file, instead of wasm.");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ private void RemoveDirectory(string path)
{
try
{
Directory.Delete(path, recursive: true);
Directory.Delete(path, recursive: true);
}
catch (Exception ex)
{
Expand Down
2 changes: 2 additions & 0 deletions src/tests/BuildWasmApps/Wasm.Build.Tests/ToolCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ private async Task<CommandResult> ExecuteAsyncInternal(string executable, string
return;
output.Add($"[{_label}] {e.Data}");
Console.WriteLine($"[{_label}] {e.Data}");
ErrorDataReceived?.Invoke(s, e);
};

Expand All @@ -97,6 +98,7 @@ private async Task<CommandResult> ExecuteAsyncInternal(string executable, string
return;
output.Add($"[{_label}] {e.Data}");
Console.WriteLine($"[{_label}] {e.Data}");
OutputDataReceived?.Invoke(s, e);
};

Expand Down

0 comments on commit e56e13b

Please sign in to comment.