[S] Change in dart/sdk[main]: [dart2wasm] Include wasm-opt in the shipped dart-sdk.
Jackson Gardner (Gerrit)
Set Ready For Review
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
Jackson Gardner (Gerrit)
Attention is currently required from: Aske Simon Christensen, Joshua Litt.
Jackson Gardner would like Aske Simon Christensen and Joshua Litt to review this change.
[dart2wasm] Include wasm-opt in the shipped dart-sdk.
Change-Id: Ib224f0b92fa28019ad3cf67d7ba2bef5c31b92ef
---
M pkg/dart2wasm/tool/compile_benchmark
M sdk/BUILD.gn
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/pkg/dart2wasm/tool/compile_benchmark b/pkg/dart2wasm/tool/compile_benchmark
index c3f213b..a88d093 100755
--- a/pkg/dart2wasm/tool/compile_benchmark
+++ b/pkg/dart2wasm/tool/compile_benchmark
@@ -46,7 +46,7 @@
fi
"$DART2WASM" "$1" "$TEMPFILE" $COMPILE_FLAGS
-"$BINARYEN" -all -g --closed-world -tnh -O3 --type-ssa --gufa -O3 --type-merging "$TEMPFILE" -o "$2"
+"$BINARYEN" -all -g --closed-world -tnh -O3 --type-ssa --gufa --type-merging "$TEMPFILE" -o "$2"
# Locate JS runtime and copy it to the same directory as "$2". We also rename it
# using the basename of "$2".
diff --git a/sdk/BUILD.gn b/sdk/BUILD.gn
index 74dca42..159a977 100644
--- a/sdk/BUILD.gn
+++ b/sdk/BUILD.gn
@@ -518,6 +518,16 @@
[ "$root_out_dir/$dart_sdk_output/bin/snapshots/{{source_file_part}}" ]
}
+copy("copy_wasm_opt") {
+ visibility = [ ":create_full_sdk" ]
+ deps = [
+ ":copy_libraries",
+ "../third_party/binaryen:wasm-opt",
+ ]
+ sources = [ "$root_out_dir/wasm-opt${executable_suffix}" ]
+ outputs = [ "$root_out_dir/$dart_sdk_output/bin/utils/{{source_file_part}}" ]
+}
+
# Copies DDC's SDK full and outline .dill files to lib/_internal.
copy("copy_dev_compiler_dills") {
visibility = [ ":copy_dev_compiler_sdk" ]
@@ -755,6 +765,7 @@
public_deps += [
":copy_dart2wasm_platform",
":copy_dart2wasm_snapshot",
+ ":copy_wasm_opt",
]
}
}
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
CBuild (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner, Joshua Litt.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/69699fd18f7652a4ffe4bfcd5cc3b343654a86cf
CBuild (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner, Joshua Litt.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/fed4af629d4a7862816676153ba4859e98900ef8
CBuild (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner, Joshua Litt.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/851607c6f95103ad77ad17397468d3c025a75f82
CBuild (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner, Joshua Litt.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/678e9841dd7519880c2ae5aa62c3fe085be1dee6
Aske Simon Christensen (Gerrit)
Attention is currently required from: Jackson Gardner, Joshua Litt.
1 comment:
File pkg/dart2wasm/tool/compile_benchmark:
This is needed. It runs optimizations again after the type flow analysis.
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
Jackson Gardner (Gerrit)
Attention is currently required from: Aske Simon Christensen, Joshua Litt.
1 comment:
File pkg/dart2wasm/tool/compile_benchmark:
This is needed. It runs optimizations again after the type flow analysis.
Ah, okay, I didn't think having the flag twice would actually run it twice. I'll revert this change.
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
CBuild (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner, Joshua Litt.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/107e6084b459819c1beac8b2e11218783da8567c
CBuild (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner, Joshua Litt.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/9dbb3d54bc6b578ee3815131851ec3357a4f6628
CBuild (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner, Joshua Litt.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/f1647efd976f3d35bc308f7dc05db9b1b8b433a3
CBuild (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner, Joshua Litt.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/5bfbb8d17863547b7f4a9ae0690538864389fafb
CBuild (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner, Joshua Litt.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/e6de989ef040d7393eb8baa15ea9963bb6136c87
Joshua Litt (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner.
1 comment:
Patchset:
This CL looks reasonable to me, but I really don't know enough about `build` to be able to provide reasonable feedback 😞.
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
Nicholas Shahan (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner, Martin Kustermann.
Jackson Gardner has uploaded this change for review.
[dart2wasm] Include wasm-opt in the shipped dart-sdk.
Change-Id: Ib224f0b92fa28019ad3cf67d7ba2bef5c31b92ef
---
M build/config/compiler/BUILD.gn
M build/toolchain/win/tool_wrapper.py
M sdk/BUILD.gn
M third_party/binaryen/BUILD.gn
4 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn
index feb13b1..be95a23 100644
--- a/build/config/compiler/BUILD.gn
+++ b/build/config/compiler/BUILD.gn
@@ -729,6 +729,15 @@
}
}
+config("enable_exceptions") {
+ if (is_win) {
+ cflags_cc = [ "/EHsc" ]
+ defines = [ "_HAS_EXCEPTIONS=1" ]
+ } else if (is_clang) {
+ cflags_cc = [ "-fexceptions" ]
+ }
+}
+
# Optimization -----------------------------------------------------------------
#
# Note that BUILDCONFIG.gn sets up a variable "default_optimization_config"
diff --git a/build/toolchain/win/tool_wrapper.py b/build/toolchain/win/tool_wrapper.py
index 652a879..3f52ee0 100644
--- a/build/toolchain/win/tool_wrapper.py
+++ b/build/toolchain/win/tool_wrapper.py
@@ -8,6 +8,7 @@
"""
import os
+import pathlib
import re
import shutil
import subprocess
@@ -149,7 +150,28 @@
not line.startswith('Generating code') and
not line.startswith('Finished generating code')):
print(line)
- return link.wait()
+ link_result = link.wait()
+
+ if link_result != 0:
+ return link_result
+
+ # The toolchain configuration in gn always expects a .lib file to be
+ # included in the output of the link step. However, this only happens
+ # when the output has exports, and that is not always the case. In
+ # order to satisfy the expected outputs, we create a dummy .lib file
+ # in cases where the link step didn't actually create one.
+ for arg in args:
+ m = _LINK_EXE_OUT_ARG.match(arg)
+ if m:
+ output_filename = m.group('out')
+ (basename, extension) = os.path.splitext(output_filename)
+ if extension == '.exe':
+ lib_path = pathlib.Path(basename + ".lib")
+ if not os.path.exists(lib_path):
+ lib_path.touch()
+ break
+
+ return link_result
def ExecMidlWrapper(self, arch, outdir, tlb, h, dlldata, iid, proxy, idl,
*flags):
diff --git a/sdk/BUILD.gn b/sdk/BUILD.gn
index 74dca42..159a977 100644
--- a/sdk/BUILD.gn
+++ b/sdk/BUILD.gn
@@ -518,6 +518,16 @@
[ "$root_out_dir/$dart_sdk_output/bin/snapshots/{{source_file_part}}" ]
}
+copy("copy_wasm_opt") {
+ visibility = [ ":create_full_sdk" ]
+ deps = [
+ ":copy_libraries",
+ "../third_party/binaryen:wasm-opt",
+ ]
+ sources = [ "$root_out_dir/wasm-opt${executable_suffix}" ]
+ outputs = [ "$root_out_dir/$dart_sdk_output/bin/utils/{{source_file_part}}" ]
+}
+
# Copies DDC's SDK full and outline .dill files to lib/_internal.
copy("copy_dev_compiler_dills") {
visibility = [ ":copy_dev_compiler_sdk" ]
@@ -755,6 +765,7 @@
public_deps += [
":copy_dart2wasm_platform",
":copy_dart2wasm_snapshot",
+ ":copy_wasm_opt",
]
}
}
diff --git a/third_party/binaryen/BUILD.gn b/third_party/binaryen/BUILD.gn
index 139d9f8..f4ab1d3 100644
--- a/third_party/binaryen/BUILD.gn
+++ b/third_party/binaryen/BUILD.gn
@@ -59,6 +59,8 @@
# Ensure generated config.h file is include path.
include_dirs += [ "$target_gen_dir" ]
+
+ configs += [ "//build/config/compiler:enable_exceptions" ]
}
template("wasm_tool") {
@@ -72,6 +74,8 @@
include_dirs = [ "src/src" ]
deps = [ ":binaryen_sources" ]
forward_variables_from(invoker, "*")
+
+ configs += [ "//build/config/compiler:enable_exceptions" ]
}
}
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
Nicholas Shahan (Gerrit)
Attention is currently required from: Aske Simon Christensen, Jackson Gardner, Martin Kustermann.
1 comment:
File sdk/BUILD.gn:
Patch Set #9, Line 768:
":copy_wasm_opt",Is this adding a new transitive dependency on ../third_party/binaryen:binaryen_sources?
@kuste...@google.com I just want to highlight this for you in case you think this could cause any difficulties.
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
Aske Simon Christensen (Gerrit)
Attention is currently required from: Jackson Gardner, Martin Kustermann.
Patch set 9:Code-Review +1
Martin Kustermann (Gerrit)
Attention is currently required from: Jackson Gardner, Nicholas Shahan.
2 comments:
Commit Message:
Patch Set #9, Line 7:
[dart2wasm] Include wasm-opt in the shipped dart-sdk.AFAIK we still do not offer a `dart compile wasm` command. As such adding wasm related things to our core Dart SDK will make it bigger without any benefit to users of Dart SDK - this is a bad smell.
As the CL is written now, it seems to add 128 MB:
```
128M out/ReleaseX64/dart-sdk/bin/utils/wasm-opt
```At least we should strip the binary. You can see how this is done for the `dart-sdk/bin/dart` binary and do something similar here.
Given that stripped `wasm-opt` binary is 5.5 MB
```
5.5M out/ReleaseX64/exe.stripped/wasm-opt
```
it may be ok to add it to our Dart SDK even if it's not used.
File sdk/BUILD.gn:
Patch Set #9, Line 768:
":copy_wasm_opt",
Is this adding a new transitive dependency on ../third_party/binaryen:binaryen_sources? […]
No issue regarding correct re-building of binaryen if files change (the topic we discussed offline before), but thank you for CC'ing me, Nicolas!
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
Jackson Gardner (Gerrit)
Attention is currently required from: Martin Kustermann, Nicholas Shahan.
1 comment:
Commit Message:
Patch Set #9, Line 7:
[dart2wasm] Include wasm-opt in the shipped dart-sdk.
AFAIK we still do not offer a `dart compile wasm` command. […]
For a bit of context, `dart compile wasm` is not too far off from being a reality, and that will use the `wasm-opt` executable, so including this in the dart sdk is an inevitability. For now, the idea is to invoke `wasm-opt` ad-hoc in the flutter tool when doing `flutter build web --wasm`, which is still experimental but we'd like to start using with some high-level benchmarks soon. It's best if we consume `wasm-opt` from the dart-sdk because (a) that is eventually what will be used anyway and (b) support for WasmGC instructions is still in flux right now with binaryen, so we often need to be pinned to a very specific version that matches what `dart2wasm` emits.
I will definitely take a look at stripping the binary though, good catch.
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
Jackson Gardner (Gerrit)
Attention is currently required from: Martin Kustermann, Nicholas Shahan.
Patch set 10:Commit-Queue +2
Jackson Gardner (Gerrit)
Attention is currently required from: Martin Kustermann, Nicholas Shahan.
Patch set 10:-Commit-Queue
1 comment:
Commit Message:
Patch Set #9, Line 7:
[dart2wasm] Include wasm-opt in the shipped dart-sdk.
For a bit of context, `dart compile wasm` is not too far off from being a reality, and that will use […]
I went ahead and
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
Jackson Gardner (Gerrit)
Attention is currently required from: Martin Kustermann, Nicholas Shahan.
1 comment:
Commit Message:
Patch Set #9, Line 7:
[dart2wasm] Include wasm-opt in the shipped dart-sdk.
I went ahead and
Whoops, I didn't meant to submit this immediately. I'm doing a dry run on CQ right now to make sure everything is fine, but the newest patch copies over the stripped `wasm-opt` binary instead of the one with symbols. Let me know if that's sufficient, I'd like to get your sign-off before actually merging.
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
CBuild (Gerrit)
Attention is currently required from: Martin Kustermann, Nicholas Shahan.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/6302315b02c6c3fde94ed8b5cf8105eccf8ea7c3
Martin Kustermann (Gerrit)
Attention is currently required from: Jackson Gardner, Nicholas Shahan.
Patch set 10:Code-Review +1
2 comments:
File third_party/binaryen/BUILD.gn:
Patch Set #10, Line 63:
configs += [ "//build/config/compiler:enable_exceptions" ]Out of curiosity: Why is this needed now (it seemed to have built `wasm-opt` without as well)?
File tools/gn.py:
Patch Set #10, Line 303:
gn_args['wasm_opt_stripped_binary'] = 'exe.stripped/wasm-opt'I was wondering whether `flutter/tools/gn` will need the same logic (as it is responsible for setting up Flutter Engine build, which also builds Dart SDK). Though it seems the engine build will strip by default and separates out the unstripped versions to `exe.unstripped/*`. So I guess it doesn't require any changes to engine.
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
Jackson Gardner (Gerrit)
Attention is currently required from: Martin Kustermann, Nicholas Shahan.
2 comments:
File third_party/binaryen/BUILD.gn:
Patch Set #10, Line 63:
configs += [ "//build/config/compiler:enable_exceptions" ]Out of curiosity: Why is this needed now (it seemed to have built `wasm-opt` without as well)?
It's for the Windows side. Exceptions are disabled by default with the Windows toolchain, so you have to add a few flags to turn them on (binaryen uses c++ exceptions). It works without the config on Linux and macOS since exceptions are on by default.
File tools/gn.py:
Patch Set #10, Line 303:
gn_args['wasm_opt_stripped_binary'] = 'exe.stripped/wasm-opt'
I was wondering whether `flutter/tools/gn` will need the same logic (as it is responsible for settin […]
I'll double check to make sure this doesn't break the engine's dart-sdk build.
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
Jackson Gardner (Gerrit)
Attention is currently required from: Martin Kustermann, Nicholas Shahan.
Patch set 10:Commit-Queue +2
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
Commit Queue (Gerrit)
Commit Queue submitted this change.
[dart2wasm] Include wasm-opt in the shipped dart-sdk.
Change-Id: Ib224f0b92fa28019ad3cf67d7ba2bef5c31b92ef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280840
Reviewed-by: Martin Kustermann <kuste...@google.com>
Commit-Queue: Jackson Gardner <jackson...@google.com>
Reviewed-by: Aske Simon Christensen <ask...@google.com>
---
M build/config/compiler/BUILD.gn
M build/toolchain/win/tool_wrapper.py
M sdk/BUILD.gn
M third_party/binaryen/BUILD.gn
M tools/gn.py
5 files changed, 62 insertions(+), 1 deletion(-)
index 74dca42..cf77b0c 100644
--- a/sdk/BUILD.gn
+++ b/sdk/BUILD.gn
@@ -27,6 +27,7 @@
dart_precompiled_runtime_stripped_binary = "dart_precompiled_runtime_product"
gen_snapshot_stripped_binary = "gen_snapshot_product"
analyze_snapshot_binary = "analyze_snapshot"
+ wasm_opt_stripped_binary = "wasm-opt"
}
# The directory layout of the SDK is as follows:
@@ -518,6 +519,16 @@
[ "$root_out_dir/$dart_sdk_output/bin/snapshots/{{source_file_part}}" ]
}
+copy("copy_wasm_opt") {
+ visibility = [ ":create_full_sdk" ]
+ deps = [
+ ":copy_libraries",
+ "../third_party/binaryen:wasm-opt",
+ ]
+ sources = [ "$root_out_dir/${wasm_opt_stripped_binary}${executable_suffix}" ]
+ outputs = [ "$root_out_dir/$dart_sdk_output/bin/utils/{{source_file_part}}" ]
+}
+
# Copies DDC's SDK full and outline .dill files to lib/_internal.
copy("copy_dev_compiler_dills") {
visibility = [ ":copy_dev_compiler_sdk" ]
@@ -755,6 +766,7 @@
public_deps += [
":copy_dart2wasm_platform",
":copy_dart2wasm_snapshot",
+ ":copy_wasm_opt",
]
}
}
diff --git a/third_party/binaryen/BUILD.gn b/third_party/binaryen/BUILD.gn
index 139d9f8..f4ab1d3 100644
--- a/third_party/binaryen/BUILD.gn
+++ b/third_party/binaryen/BUILD.gn
@@ -59,6 +59,8 @@
# Ensure generated config.h file is include path.
include_dirs += [ "$target_gen_dir" ]
+
+ configs += [ "//build/config/compiler:enable_exceptions" ]
}
template("wasm_tool") {
@@ -72,6 +74,8 @@
include_dirs = [ "src/src" ]
deps = [ ":binaryen_sources" ]
forward_variables_from(invoker, "*")
+
+ configs += [ "//build/config/compiler:enable_exceptions" ]
}
}
diff --git a/tools/gn.py b/tools/gn.py
index be46581..c8bac2f 100755
--- a/tools/gn.py
+++ b/tools/gn.py
@@ -300,6 +300,7 @@
gn_args['gen_snapshot_stripped_binary'] = (
'exe.stripped/gen_snapshot_product')
gn_args['analyze_snapshot_binary'] = ('exe.stripped/analyze_snapshot')
+ gn_args['wasm_opt_stripped_binary'] = 'exe.stripped/wasm-opt'
# Setup the user-defined sysroot.
if UseSysroot(args, gn_args):
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
William Hesse (Gerrit)
William Hesse has created a revert of this change.
To view, visit change 280840. To unsubscribe, or for help writing mail filters, visit settings.
CBuild (Gerrit)
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/6271d26baf498ab2b053c52527bc65f7f43e802b