Make wasm-interpreter work like wasm-aot #1769
Conversation
|
@naricc are we done with this PR? |
| @@ -96,14 +96,7 @@ protected void GenerateProjectInterpreter(BuildPartition buildPartition, Artifac | |||
|
|
|||
| protected override string GetBinariesDirectoryPath(string buildArtifactsDirectoryPath, string configuration) | |||
| { | |||
| if (Aot) | |||
There was a problem hiding this comment.
I don't understand this change and I would like to be sure that we are not breaking anything. Have you tested both AOT and non-AOT toolchains?
There was a problem hiding this comment.
Basically, before wasm-intepreter mode was using a simplified custom app builder I built into BDN, because that was the best thing that could be done at the time.
Wasm-aot used all the app bundling stuff from the the runtime source directory, which put things in a different place than the one I coded into BDN.
With this change, both wasm-interpreter and wasm-aot will use the app builder from the runtime source dir, so the files will end up in the same place.
There was a problem hiding this comment.
I've tested both AOT and interpreter configurations.
| @@ -175,9 +175,6 @@ public class CommandLineOptions | |||
| [Option("wasmEngine", Required = false, HelpText = "Full path to a java script engine used to run the benchmarks, used by Wasm toolchain.")] | |||
| public FileInfo WasmJavascriptEngine { get; set; } | |||
|
|
|||
| [Option("wasmMainJS", Required = false, HelpText = "Path to the main.js file used by Wasm toolchain. Mandatory when using \"--runtimes wasm\"")] | |||
| public FileInfo WasmMainJs { get; set; } | |||
There was a problem hiding this comment.
Please keep in mind that once we delete this parameter and you update BDN in dotnet/performance everything that is still using this argument is going to get an error from the BDN config parser
There was a problem hiding this comment.
Yes, I am aware. We have made changes in dotnet/performance so nothing is using it.
|
@adamsitnik If you would be so kind as to merge it when the builds are green :) |
|
create PR dotnet/performance#1918 to have perf repo depends on this new version of bdn. |
This mostly unifies wasm-aot and wasm-interpreter, so they work largely the same way. It resolves some on-going issues with the existing wasm-interpreter toolchain.