From 34e063fb382210b82a4e660edab063395fe64466 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 12 Jan 2023 14:48:39 -0800 Subject: [PATCH 1/4] work --- src/pass.h | 2 + src/passes/pass.cpp | 10 +++++ src/tools/optimization-options.h | 8 ++++ test/lit/help/wasm-opt.test | 2 + test/lit/help/wasm2js.test | 2 + test/lit/passes/O1_skip.wast | 70 ++++++++++++++++++++++++++++++++ 6 files changed, 94 insertions(+) create mode 100644 test/lit/passes/O1_skip.wast diff --git a/src/pass.h b/src/pass.h index 36d49f7b74a..c49a6d3ef27 100644 --- a/src/pass.h +++ b/src/pass.h @@ -226,6 +226,8 @@ struct PassOptions { // Arbitrary string arguments from the commandline, which we forward to // passes. std::unordered_map arguments; + // Passes to skip and not run. + std::unordered_set skippedPasses; // Effect info computed for functions. One pass can generate this and then // other passes later can benefit from it. It is up to the sequence of passes diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index 373322a51a8..7e5af92234e 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -930,6 +930,12 @@ struct AfterEffectModuleChecker { }; void PassRunner::runPass(Pass* pass) { + assert(!pass->isFunctionParallel()); + + if (options.skippedPasses.count(pass->name)) { + return; + } + std::unique_ptr checker; if (getPassDebug()) { checker = std::unique_ptr( @@ -949,6 +955,10 @@ void PassRunner::runPass(Pass* pass) { void PassRunner::runPassOnFunction(Pass* pass, Function* func) { assert(pass->isFunctionParallel()); + if (options.skippedPasses.count(pass->name)) { + return; + } + auto passDebug = getPassDebug(); // Add extra validation logic in pass-debug mode 2. The main logic in diff --git a/src/tools/optimization-options.h b/src/tools/optimization-options.h index eaa90f55ffa..419fd58d907 100644 --- a/src/tools/optimization-options.h +++ b/src/tools/optimization-options.h @@ -289,6 +289,14 @@ struct OptimizationOptions : public ToolOptions { Options::Arguments::Zero, [this](Options*, const std::string&) { passOptions.zeroFilledMemory = true; + }) + .add("--skip-pass", + "-sp", + "Skip a pass (do not run it)", + OptimizationOptionsCategory, + Options::Arguments::One, + [this](Options*, const std::string& pass) { + passOptions.skippedPasses.insert(pass); }); // add passes in registry diff --git a/test/lit/help/wasm-opt.test b/test/lit/help/wasm-opt.test index 042807e2f05..fab34b9150b 100644 --- a/test/lit/help/wasm-opt.test +++ b/test/lit/help/wasm-opt.test @@ -576,6 +576,8 @@ ;; CHECK-NEXT: --zero-filled-memory,-uim Assume that an imported memory ;; CHECK-NEXT: will be zero-initialized ;; CHECK-NEXT: +;; CHECK-NEXT: --skip-pass,-sp Skip a pass (do not run it) +;; CHECK-NEXT: ;; CHECK-NEXT: ;; CHECK-NEXT: Tool options: ;; CHECK-NEXT: ------------- diff --git a/test/lit/help/wasm2js.test b/test/lit/help/wasm2js.test index ab56ff4e8fc..e5196f8cd7d 100644 --- a/test/lit/help/wasm2js.test +++ b/test/lit/help/wasm2js.test @@ -535,6 +535,8 @@ ;; CHECK-NEXT: --zero-filled-memory,-uim Assume that an imported memory ;; CHECK-NEXT: will be zero-initialized ;; CHECK-NEXT: +;; CHECK-NEXT: --skip-pass,-sp Skip a pass (do not run it) +;; CHECK-NEXT: ;; CHECK-NEXT: ;; CHECK-NEXT: Tool options: ;; CHECK-NEXT: ------------- diff --git a/test/lit/passes/O1_skip.wast b/test/lit/passes/O1_skip.wast new file mode 100644 index 00000000000..c57d052b4c0 --- /dev/null +++ b/test/lit/passes/O1_skip.wast @@ -0,0 +1,70 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. +;; NOTE: This test was ported using port_passes_tests_to_lit.py and could be cleaned up. + +;; RUN: foreach %s %t wasm-opt -O2 --coalesce-locals --skip-pass=coalesce-locals -S -o - | filecheck %s + +;; We should skip coalese-locals even though it is run in -O2 and also we ask to +;; run it directly: the skip instruction overrides everything else. + +(module + ;; CHECK: (type $i32_i32_=>_none (func (param i32 i32))) + + ;; CHECK: (type $i32_=>_none (func (param i32))) + + ;; CHECK: (import "a" "b" (func $log (param i32 i32))) + (import "a" "b" (func $log (param i32 i32))) + + (func "foo" (param $p i32) + ;; The locals $x and $y can be coalesced into a single local, but as we do not + ;; run that pass, they will not be. Other minor optimizations will occur here, + ;; such as using a tee. + (local $x i32) + (local $y i32) + + (local.set $x + (i32.add + (local.get $p) + (i32.const 1) + ) + ) + (call $log + (local.get $x) + (local.get $x) + ) + + (local.set $y + (i32.add + (local.get $p) + (i32.const 1) + ) + ) + (call $log + (local.get $y) + (local.get $y) + ) + ) +) +;; CHECK: (export "foo" (func $0)) + +;; CHECK: (func $0 (; has Stack IR ;) (param $p i32) +;; CHECK-NEXT: (local $x i32) +;; CHECK-NEXT: (local $y i32) +;; CHECK-NEXT: (call $log +;; CHECK-NEXT: (local.tee $x +;; CHECK-NEXT: (i32.add +;; CHECK-NEXT: (local.get $p) +;; CHECK-NEXT: (i32.const 1) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (local.get $x) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (call $log +;; CHECK-NEXT: (local.tee $y +;; CHECK-NEXT: (i32.add +;; CHECK-NEXT: (local.get $p) +;; CHECK-NEXT: (i32.const 1) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (local.get $y) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) From 30c3f26af9a094ea5bcf972d7882e0f1a048423b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 24 Jan 2023 09:47:28 -0800 Subject: [PATCH 2/4] work --- src/pass.h | 5 ++++- src/passes/pass.cpp | 19 +++++++++++++++++-- src/tools/optimization-options.h | 2 +- test/lit/passes/skip-missing.wast | 8 ++++++++ 4 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 test/lit/passes/skip-missing.wast diff --git a/src/pass.h b/src/pass.h index c49a6d3ef27..973773c494b 100644 --- a/src/pass.h +++ b/src/pass.h @@ -227,7 +227,7 @@ struct PassOptions { // passes. std::unordered_map arguments; // Passes to skip and not run. - std::unordered_set skippedPasses; + std::unordered_set passesToSkip; // Effect info computed for functions. One pass can generate this and then // other passes later can benefit from it. It is up to the sequence of passes @@ -388,6 +388,9 @@ struct PassRunner { // Whether this pass runner has run. A pass runner should only be run once. bool ran = false; + // Passes in |options.passesToSkip| that we have seen and skipped. + std::unordered_set skippedPasses; + void runPass(Pass* pass); void runPassOnFunction(Pass* pass, Function* func); diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index 7e5af92234e..74e7f6dc307 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -692,6 +692,9 @@ void PassRunner::run() { assert(!ran); ran = true; + // As we run passes, we'll notice which we skip. + skippedPasses.clear(); + static const int passDebug = getPassDebug(); // Emit logging information when asked for. At passDebug level 1+ we log // the main passes, while in 2 we also log nested ones. Note that for @@ -812,6 +815,16 @@ void PassRunner::run() { } flush(); } + + // All the passes the users requested to skip should have been seen, and + // skipped. If not, the user may have had a typo in the name of a pass to + // skip, and we will warn. + for (auto pass : options.passesToSkip) { + if (!skippedPasses.count(pass)) { + std::cerr << "warning: --" << pass << " was requested to be skipped, " + << "but it was not found in the passes that were run.\n"; + } + } } void PassRunner::runOnFunction(Function* func) { @@ -932,7 +945,8 @@ struct AfterEffectModuleChecker { void PassRunner::runPass(Pass* pass) { assert(!pass->isFunctionParallel()); - if (options.skippedPasses.count(pass->name)) { + if (options.passesToSkip.count(pass->name)) { + skippedPasses.insert(pass->name); return; } @@ -955,7 +969,8 @@ void PassRunner::runPass(Pass* pass) { void PassRunner::runPassOnFunction(Pass* pass, Function* func) { assert(pass->isFunctionParallel()); - if (options.skippedPasses.count(pass->name)) { + if (options.passesToSkip.count(pass->name)) { + skippedPasses.insert(pass->name); return; } diff --git a/src/tools/optimization-options.h b/src/tools/optimization-options.h index 97f40ae2192..c87530536c9 100644 --- a/src/tools/optimization-options.h +++ b/src/tools/optimization-options.h @@ -284,7 +284,7 @@ struct OptimizationOptions : public ToolOptions { OptimizationOptionsCategory, Options::Arguments::One, [this](Options*, const std::string& pass) { - passOptions.skippedPasses.insert(pass); + passOptions.passesToSkip.insert(pass); }); // add passes in registry diff --git a/test/lit/passes/skip-missing.wast b/test/lit/passes/skip-missing.wast new file mode 100644 index 00000000000..6cfc5867de0 --- /dev/null +++ b/test/lit/passes/skip-missing.wast @@ -0,0 +1,8 @@ +;; We should warn on a pass called "waka" not having been run and skipped. + +;; RUN: not wasm-opt %s --skip-pass=waka 2>&1 | filecheck %s + +;; CHECK: waka + +(module +) From b319ce72f3de6c363dc92c533c10611b7804e278 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 24 Jan 2023 09:49:44 -0800 Subject: [PATCH 3/4] test --- test/lit/passes/skip-missing.wast | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/lit/passes/skip-missing.wast b/test/lit/passes/skip-missing.wast index 6cfc5867de0..02778dab092 100644 --- a/test/lit/passes/skip-missing.wast +++ b/test/lit/passes/skip-missing.wast @@ -1,8 +1,8 @@ ;; We should warn on a pass called "waka" not having been run and skipped. -;; RUN: not wasm-opt %s --skip-pass=waka 2>&1 | filecheck %s +;; RUN: wasm-opt %s -O1 --skip-pass=waka 2>&1 | filecheck %s -;; CHECK: waka +;; CHECK: warning: --waka was requested to be skipped, but it was not found in the passes that were run. (module ) From d05ebfcd128c962e4fd8b322df36a6686d78ad0d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 24 Jan 2023 11:19:09 -0800 Subject: [PATCH 4/4] typo --- src/passes/pass.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index 74e7f6dc307..593023c7cc4 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -816,7 +816,7 @@ void PassRunner::run() { flush(); } - // All the passes the users requested to skip should have been seen, and + // All the passes the user requested to skip should have been seen, and // skipped. If not, the user may have had a typo in the name of a pass to // skip, and we will warn. for (auto pass : options.passesToSkip) {