diff options
| -rw-r--r-- | CHANGELOG.md | 8 | ||||
| -rw-r--r-- | src/pass.h | 3 | ||||
| -rw-r--r-- | src/passes/pass.cpp | 19 | ||||
| -rw-r--r-- | src/tools/optimization-options.h | 58 | ||||
| -rw-r--r-- | test/lit/passes/O3_Oz.wast | 45 | ||||
| -rw-r--r-- | test/lit/passes/skip-missing.wast | 8 |
6 files changed, 92 insertions, 49 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index c416f16bf..beccb3e23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,14 @@ full changeset diff at the end of each section. Current Trunk ------------- +- Fix a bug where e.g. -O3 -Oz ran the -O3 with the opt levels of -Oz, which + could inhibit inlining, for example. While this is a bugfix, it affects how + commandline options are interpreted, so if you depended on the old behavior + this may be a breaking change. That is, the old behavior made -O3 -Oz run the + first -O3 with -Oz's opt levels, and the new behavior is to run -O3 with the + proper (-O3) opt levels. This is a followup to #5333 from a previous release. + (#5787) + v113 ---- diff --git a/src/pass.h b/src/pass.h index 973773c49..090aa8d33 100644 --- a/src/pass.h +++ b/src/pass.h @@ -388,9 +388,6 @@ private: // 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<std::string> skippedPasses; - void runPass(Pass* pass); void runPassOnFunction(Pass* pass, Function* func); diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index 0411dcc75..adaa42e04 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -698,9 +698,6 @@ 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 @@ -821,20 +818,6 @@ void PassRunner::run() { } flush(); } - - if (!isNested) { - // 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. (We don't do this in a nested runner because - // those are used for various internal tasks inside passes, which would lead - // to many spurious warnings.) - 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) { @@ -956,7 +939,6 @@ void PassRunner::runPass(Pass* pass) { assert(!pass->isFunctionParallel()); if (options.passesToSkip.count(pass->name)) { - skippedPasses.insert(pass->name); return; } @@ -980,7 +962,6 @@ void PassRunner::runPassOnFunction(Pass* pass, Function* func) { assert(pass->isFunctionParallel()); 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 c87530536..045542ac2 100644 --- a/src/tools/optimization-options.h +++ b/src/tools/optimization-options.h @@ -328,32 +328,52 @@ struct OptimizationOptions : public ToolOptions { bool runningPasses() { return passes.size() > 0; } void runPasses(Module& wasm) { - PassRunner passRunner(&wasm, passOptions); - if (debug) { - passRunner.setDebug(true); - } - for (auto& pass : passes) { - // We apply the pass's intended opt and shrink levels, if any. - auto oldOptimizeLevel = passRunner.options.optimizeLevel; - auto oldShrinkLevel = passRunner.options.shrinkLevel; - if (pass.optimizeLevel) { - passRunner.options.optimizeLevel = *pass.optimizeLevel; + std::unique_ptr<PassRunner> passRunner; + + // Flush anything in the current pass runner, and then reset it to a fresh + // state so it is ready for new things. + auto flushAndReset = [&]() { + if (passRunner) { + passRunner->run(); } - if (pass.shrinkLevel) { - passRunner.options.shrinkLevel = *pass.shrinkLevel; + passRunner = std::make_unique<PassRunner>(&wasm, passOptions); + if (debug) { + passRunner->setDebug(true); } + }; + flushAndReset(); + + for (auto& pass : passes) { if (pass.name == DEFAULT_OPT_PASSES) { - passRunner.addDefaultOptimizationPasses(); + // This is something like -O3 or -Oz. We must run this now, in order to + // set the proper opt and shrink levels. To do that, first reset the + // runner so that anything already queued is run (since we can only run + // after those things). + flushAndReset(); + + // -O3/-Oz etc. always set their own optimize/shrinkLevels. + assert(pass.optimizeLevel); + assert(pass.shrinkLevel); + passRunner->options.optimizeLevel = *pass.optimizeLevel; + passRunner->options.shrinkLevel = *pass.shrinkLevel; + + // Run our optimizations now, and reset the runner so that the default + // pass options are used later (and not the temporary optimize/ + // shrinkLevels we just set). + passRunner->addDefaultOptimizationPasses(); + flushAndReset(); } else { - passRunner.add(pass.name); - } + // This is a normal pass. Add it to the queue for execution. + passRunner->add(pass.name); - // Revert back to the default levels, if we changed them. - passRunner.options.optimizeLevel = oldOptimizeLevel; - passRunner.options.shrinkLevel = oldShrinkLevel; + // Normal passes do not set their own optimize/shrinkLevels. + assert(!pass.optimizeLevel); + assert(!pass.shrinkLevel); + } } - passRunner.run(); + + flushAndReset(); } }; diff --git a/test/lit/passes/O3_Oz.wast b/test/lit/passes/O3_Oz.wast new file mode 100644 index 000000000..a60064778 --- /dev/null +++ b/test/lit/passes/O3_Oz.wast @@ -0,0 +1,45 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: foreach %s %t wasm-opt -O3 -Oz -S -o - | filecheck %s + +(module + (func $inline.me (param $x i32) (result i32) + (i32.add + (local.get $x) + (i32.const 2) + ) + ) + + (func "export" (param $x i32) (result i32) + ;; $inline.me is called twice, so we do not always inline it like called- + ;; once functions are. -Oz is too cautious to inline such things that may + ;; end up increasing total code size, but we are running -O3 -Oz here and so + ;; the first -O3 will inline there. That is, this test verifies that the + ;; later -Oz does not affect the earlier -O3 (which it could, if -Oz set + ;; global state that -O3 then reads to see the optimization and shrink + ;; levels). + (i32.add + (call $inline.me + (local.get $x) + ) + (call $inline.me + (local.get $x) + ) + ) + ) +) +;; CHECK: (type $i32_=>_i32 (func (param i32) (result i32))) + +;; CHECK: (export "export" (func $1)) + +;; CHECK: (func $1 (; has Stack IR ;) (param $0 i32) (result i32) +;; CHECK-NEXT: (i32.add +;; CHECK-NEXT: (local.tee $0 +;; CHECK-NEXT: (i32.add +;; CHECK-NEXT: (local.get $0) +;; CHECK-NEXT: (i32.const 2) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (local.get $0) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) diff --git a/test/lit/passes/skip-missing.wast b/test/lit/passes/skip-missing.wast deleted file mode 100644 index 02778dab0..000000000 --- a/test/lit/passes/skip-missing.wast +++ /dev/null @@ -1,8 +0,0 @@ -;; We should warn on a pass called "waka" not having been run and skipped. - -;; RUN: wasm-opt %s -O1 --skip-pass=waka 2>&1 | filecheck %s - -;; CHECK: warning: --waka was requested to be skipped, but it was not found in the passes that were run. - -(module -) |