Use Binaryen StackIR at the end of the pipeline (#22218)

kripken · web-flow · commit 3905550b49c2 · 2024-07-11T11:30:50.000-07:00
Before this PR, if we did -O3 then we ended up doing this: * wasm-opt -O3 * wasm-metadce * wasm-opt --minify-imports-and-exports The first invocation uses StackIR (which is enabled by default in -O3), but then the later invocations "lose" those optimizations, since they read the wasm binary and generate structured BinaryenIR once more. We would need to do StackIR opts again, but even more, we'd need to do work to undo the effects of stackiness, which can lead to more locals and such, see #22196 After this PR, we do this: * wasm-opt -O3 --no-stack-ir * wasm-metadce * wasm-opt --minify-imports-and-exports --optimize-stack-ir The first command is now told to not use StackIR. The second remains unchanged. The last then does StackIR optimizations. This is the same amount of work (we just shuffle around the StackIR opts), so compile times are unaffected, but we gain some code improvements. To see them, see the commit before last which applies the size changes on top of a recent rebase. Many tests improve by small but noticeable amounts, though two wasm2js tests somehow lose by a few bytes, which I think is noise. There is a slight code complexity downside here, but given this avoids corner cases that may be quite annoying (#22196) it seems worthwhile. Also improve our handling of --emit-symbol-map to use llvm-objcopy, which avoids wasm-opt roundtrip issues (this is necessary in this commit as otherwise the improvements of this PR are undone by that roundtrip, which was caught by a test).
diff --git a/test/code_size/embind_val_wasm.json b/test/code_size/embind_val_wasm.json
@@ -3,8 +3,8 @@
   "a.html.gz": 380,
   "a.js": 7072,
   "a.js.gz": 2997,
-  "a.wasm": 11625,
-  "a.wasm.gz": 5814,
-  "total": 19249,
-  "total_gz": 9191
+  "a.wasm": 11588,
+  "a.wasm.gz": 5778,
+  "total": 19212,
+  "total_gz": 9155
 }
diff --git a/test/code_size/hello_webgl2_wasm.json b/test/code_size/hello_webgl2_wasm.json
@@ -3,8 +3,8 @@
   "a.html.gz": 328,
   "a.js": 4521,
   "a.js.gz": 2312,
-  "a.wasm": 10427,
-  "a.wasm.gz": 6719,
-  "total": 15402,
-  "total_gz": 9359
+  "a.wasm": 10399,
+  "a.wasm.gz": 6695,
+  "total": 15374,
+  "total_gz": 9335
 }
diff --git a/test/code_size/hello_webgl2_wasm2js.json b/test/code_size/hello_webgl2_wasm2js.json
@@ -1,8 +1,8 @@
 {
   "a.html": 346,
   "a.html.gz": 262,
-  "a.js": 22191,
-  "a.js.gz": 11586,
-  "total": 22537,
-  "total_gz": 11848
+  "a.js": 22193,
+  "a.js.gz": 11584,
+  "total": 22539,
+  "total_gz": 11846
 }
diff --git a/test/code_size/hello_webgl_wasm.json b/test/code_size/hello_webgl_wasm.json
@@ -3,8 +3,8 @@
   "a.html.gz": 328,
   "a.js": 4059,
   "a.js.gz": 2153,
-  "a.wasm": 10427,
-  "a.wasm.gz": 6719,
-  "total": 14940,
-  "total_gz": 9200
+  "a.wasm": 10399,
+  "a.wasm.gz": 6695,
+  "total": 14912,
+  "total_gz": 9176
 }
diff --git a/test/code_size/hello_webgl_wasm2js.json b/test/code_size/hello_webgl_wasm2js.json
@@ -1,8 +1,8 @@
 {
   "a.html": 346,
   "a.html.gz": 262,
-  "a.js": 21717,
-  "a.js.gz": 11425,
-  "total": 22063,
-  "total_gz": 11687
+  "a.js": 21719,
+  "a.js.gz": 11420,
+  "total": 22065,
+  "total_gz": 11682
 }
diff --git a/test/code_size/random_printf_wasm.json b/test/code_size/random_printf_wasm.json
@@ -1,6 +1,6 @@
 {
-  "a.html": 12746,
-  "a.html.gz": 6918,
-  "total": 12746,
-  "total_gz": 6918
+  "a.html": 12694,
+  "a.html.gz": 6891,
+  "total": 12694,
+  "total_gz": 6891
 }
diff --git a/test/other/metadce/test_metadce_files_wasmfs.size b/test/other/metadce/test_metadce_files_wasmfs.size
@@ -1 +1 @@
-51018
+50957
diff --git a/test/other/metadce/test_metadce_hello_O3.size b/test/other/metadce/test_metadce_hello_O3.size
@@ -1 +1 @@
-1741
+1737
diff --git a/test/other/metadce/test_metadce_hello_Os.size b/test/other/metadce/test_metadce_hello_Os.size
@@ -1 +1 @@
-1732
+1728
diff --git a/test/other/metadce/test_metadce_hello_Oz.size b/test/other/metadce/test_metadce_hello_Oz.size
@@ -1 +1 @@
-1290
+1286
diff --git a/test/other/metadce/test_metadce_hello_dylink.size b/test/other/metadce/test_metadce_hello_dylink.size
@@ -1 +1 @@
-9355
+9347
diff --git a/test/other/metadce/test_metadce_hello_wasmfs.size b/test/other/metadce/test_metadce_hello_wasmfs.size
@@ -1 +1 @@
-1741
+1737
diff --git a/test/other/metadce/test_metadce_mem_O3.size b/test/other/metadce/test_metadce_mem_O3.size
@@ -1 +1 @@
-5265
+5261
diff --git a/test/other/metadce/test_metadce_mem_O3_grow.size b/test/other/metadce/test_metadce_mem_O3_grow.size
@@ -1 +1 @@
-5266
+5262
diff --git a/test/other/metadce/test_metadce_mem_O3_grow_standalone.size b/test/other/metadce/test_metadce_mem_O3_grow_standalone.size
@@ -1 +1 @@
-5589
+5585
diff --git a/test/other/metadce/test_metadce_mem_O3_standalone.size b/test/other/metadce/test_metadce_mem_O3_standalone.size
@@ -1 +1 @@
-5514
+5510
diff --git a/test/other/metadce/test_metadce_mem_O3_standalone_lib.size b/test/other/metadce/test_metadce_mem_O3_standalone_lib.size
@@ -1 +1 @@
-5277
+5273
diff --git a/test/other/metadce/test_metadce_mem_O3_standalone_narg.size b/test/other/metadce/test_metadce_mem_O3_standalone_narg.size
@@ -1 +1 @@
-5307
+5303
diff --git a/test/other/metadce/test_metadce_mem_O3_standalone_narg_flto.size b/test/other/metadce/test_metadce_mem_O3_standalone_narg_flto.size
@@ -1 +1 @@
-4124
+4120
diff --git a/test/other/metadce/test_metadce_minimal_pthreads.size b/test/other/metadce/test_metadce_minimal_pthreads.size
@@ -1 +1 @@
-19481
+19421
diff --git a/test/test_other.py b/test/test_other.py
@@ -8038,6 +8038,26 @@ def test_eval_ctors_debug_output(self):
     self.assertNotContained('ctor_evaller: not successful', err)
     self.assertContained('could not eval: call import: env.external_thing', err)
 
+  @uses_canonical_tmp
+  @with_env_modify({'EMCC_DEBUG': '1'})
+  @parameterized({
+    # StackIR optimizations should happen once at the end, and not multiple
+    # times in the middle. In -O2 we simply do them in the main wasm-opt
+    # invocation, while in -O3 we run wasm-opt later, and so we should disable
+    # StackIR first and enable it at the end.
+    'O2': (['-O2'], False),
+    'O3': (['-O3'], True),
+  })
+  def test_binaryen_stack_ir(self, opts, disable_and_enable):
+    err = self.run_process([EMXX, test_file('hello_world.c')] + opts, stderr=PIPE).stderr
+    DISABLE = '--no-stack-ir'
+    ENABLE = '--optimize-stack-ir'
+    self.assertContainedIf(DISABLE, err, disable_and_enable)
+    self.assertContainedIf(ENABLE, err, disable_and_enable)
+    # they should also appear at most once each
+    self.assertLess(err.count(DISABLE), 2)
+    self.assertLess(err.count(ENABLE), 2)
+
   def test_override_js_execution_environment(self):
     create_file('main.c', r'''
       #include <emscripten.h>
diff --git a/tools/building.py b/tools/building.py
@@ -719,7 +719,10 @@ def minify_wasm_js(js_file, wasm_file, expensive_optimizations, debug_info):
     # if we are optimizing for size, shrink the combined wasm+JS
     # TODO: support this when a symbol map is used
     if expensive_optimizations:
-      js_file = metadce(js_file, wasm_file, debug_info=debug_info)
+      js_file = metadce(js_file,
+                        wasm_file,
+                        debug_info=debug_info,
+                        last=not settings.MINIFY_WASM_IMPORTS_AND_EXPORTS)
       # now that we removed unneeded communication between js and wasm, we can clean up
       # the js some more.
       passes = ['AJSDCE']
@@ -743,8 +746,16 @@ def is_internal_global(name):
   return name in internal_start_stop_symbols or any(name.startswith(p) for p in internal_prefixes)
 
 
+# get the flags to pass into the very last binaryen tool invocation, that runs
+# the final set of optimizations
+def get_last_binaryen_opts():
+  return [f'--optimize-level={settings.OPT_LEVEL}',
+          f'--shrink-level={settings.SHRINK_LEVEL}',
+          '--optimize-stack-ir']
+
+
 # run binaryen's wasm-metadce to dce both js and wasm
-def metadce(js_file, wasm_file, debug_info):
+def metadce(js_file, wasm_file, debug_info, last):
   logger.debug('running meta-DCE')
   temp_files = shared.get_temp_files()
   # first, get the JS part of the graph
@@ -810,10 +821,13 @@ def metadce(js_file, wasm_file, debug_info):
   temp = temp_files.get('.json', prefix='emcc_dce_graph_').name
   utils.write_file(temp, json.dumps(graph, indent=2))
   # run wasm-metadce
+  args = ['--graph-file=' + temp]
+  if last:
+    args += get_last_binaryen_opts()
   out = run_binaryen_command('wasm-metadce',
                              wasm_file,
                              wasm_file,
-                             ['--graph-file=' + temp],
+                             args,
                              debug=debug_info,
                              stdout=PIPE)
   # find the unused things in js
@@ -878,23 +892,19 @@ def asyncify_lazy_load_code(wasm_target, debug):
 def minify_wasm_imports_and_exports(js_file, wasm_file, minify_exports, debug_info):
   logger.debug('minifying wasm imports and exports')
   # run the pass
+  args = []
   if minify_exports:
     # standalone wasm mode means we need to emit a wasi import module.
     # otherwise, minify even the imported module names.
     if settings.MINIFY_WASM_IMPORTED_MODULES:
-      pass_name = '--minify-imports-and-exports-and-modules'
+      args.append('--minify-imports-and-exports-and-modules')
     else:
-      pass_name = '--minify-imports-and-exports'
+      args.append('--minify-imports-and-exports')
   else:
-    pass_name = '--minify-imports'
-  out = run_wasm_opt(wasm_file, wasm_file,
-                     [pass_name],
-                     debug=debug_info,
-                     stdout=PIPE)
-  # TODO this is the last tool we run, after normal opts and metadce. it
-  # might make sense to run Stack IR optimizations here or even -O (as
-  # metadce which runs before us might open up new general optimization
-  # opportunities). however, the benefit is less than 0.5%.
+    args.append('--minify-imports')
+  # this is always the last tool we run (if we run it)
+  args += get_last_binaryen_opts()
+  out = run_wasm_opt(wasm_file, wasm_file, args, debug=debug_info, stdout=PIPE)
 
   # get the mapping
   SEP = ' => '
@@ -1057,15 +1067,18 @@ def handle_final_wasm_symbols(wasm_file, symbols_file, debug_info):
   args = []
   if symbols_file:
     args += ['--print-function-map']
-  if not debug_info:
-    # to remove debug info, we just write to that same file, and without -g
-    args += ['-o', wasm_file]
   else:
     # suppress the wasm-opt warning regarding "no output file specified"
     args += ['--quiet']
   output = run_wasm_opt(wasm_file, args=args, stdout=PIPE)
   if symbols_file:
     utils.write_file(symbols_file, output)
+  if not debug_info:
+    # strip the names section using llvm-objcopy. this is slightly slower than
+    # using wasm-opt (we could run wasm-opt without -g here and just tell it to
+    # write the file back out), but running wasm-opt would undo StackIR
+    # optimizations, if we did those.
+    strip(wasm_file, wasm_file, sections=['name'])
 
 
 def is_ar(filename):
diff --git a/tools/link.py b/tools/link.py
@@ -424,6 +424,17 @@ def check_human_readable_list(items):
   if settings.WASM_EXNREF:
     passes += ['--emit-exnref']
 
+  # If we are going to run metadce then that means we will be running binaryen
+  # tools after the main invocation, whose flags are determined here
+  # (specifically we will run metadce and possibly also wasm-opt for import/
+  # export minification). And when we run such a tool it will "undo" any
+  # StackIR optimizations (since the conversion to BinaryenIR undoes them as it
+  # restructures the code). We could re-run those opts, but it is most efficient
+  # to just not do them now if we'll invoke other tools later, and we'll do them
+  # only in the very last invocation.
+  if will_metadce():
+    passes += ['--no-stack-ir']
+
   return passes
 
 

Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,8 @@`
`1`	`1`	`{`
`2`	`2`	`"a.html": 346,`
`3`	`3`	`"a.html.gz": 262,`
`4`		`- "a.js": 22191,`
`5`		`- "a.js.gz": 11586,`
`6`		`- "total": 22537,`
`7`		`- "total_gz": 11848`
	`4`	`+ "a.js": 22193,`
	`5`	`+ "a.js.gz": 11584,`
	`6`	`+ "total": 22539,`
	`7`	`+ "total_gz": 11846`
`8`	`8`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`		`- "a.html": 12746,`
`3`		`- "a.html.gz": 6918,`
`4`		`- "total": 12746,`
`5`		`- "total_gz": 6918`
	`2`	`+ "a.html": 12694,`
	`3`	`+ "a.html.gz": 6891,`
	`4`	`+ "total": 12694,`
	`5`	`+ "total_gz": 6891`
`6`	`6`	`}`