Fix opt/shrink levels when running the optimizer multiple times (#5333)

Previously -O3 -O1 would run -O1 twice since the last flag set the global opt level
to 1, and then all invocations of the optimizer pipeline read that. This makes each
pipeline define its own opt level.

This has been a long-standing annoyance, which wasn't much noticed except that
with wasm GC there is more of a need to run the optimization pipeline more than
once. And sometimes it is nice to run different levels.

2 files changed, 62 insertions, 15 deletions

diff --git a/src/tools/optimization-options.h b/src/tools/optimization-options.h
index ff9bb973d..eaa90f55f 100644
--- a/src/tools/optimization-options.h
+++ b/src/tools/optimization-options.h
@@ -30,7 +30,36 @@ struct OptimizationOptions : public ToolOptions {
   static constexpr const int OS_OPTIMIZE_LEVEL = 2;
   static constexpr const int OS_SHRINK_LEVEL = 1;
 
-  std::vector<std::string> passes;
+  // Information to run a pass, as requested by a commandline flag.
+  struct PassInfo {
+    // The name of the pass to run.
+    std::string name;
+
+    // The optimize and shrink levels to run the pass with, if specified. If not
+    // specified then the defaults are used.
+    std::optional<int> optimizeLevel;
+    std::optional<int> shrinkLevel;
+
+    PassInfo(std::string name) : name(name) {}
+    PassInfo(const char* name) : name(name) {}
+    PassInfo(std::string name, int optimizeLevel, int shrinkLevel)
+      : name(name), optimizeLevel(optimizeLevel), shrinkLevel(shrinkLevel) {}
+  };
+
+  std::vector<PassInfo> passes;
+
+  // Add a request to run all the default opt passes. They are run with the
+  // current opt and shrink levels specified, which are read from passOptions.
+  //
+  // Each caller to here sets the opt and shrink levels before, which provides
+  // the right values for us to read. That is, -Os etc. sets the default opt
+  // level, so that the last of -O3 -Os will override the previous default, but
+  // also we note the current opt level for when we run the pass, so that the
+  // sequence -O3 -Os will run -O3 and then -Os, and not -Os twice.
+  void addDefaultOptPasses() {
+    passes.push_back(PassInfo{
+      DEFAULT_OPT_PASSES, passOptions.optimizeLevel, passOptions.shrinkLevel});
+  }
 
   constexpr static const char* OptimizationOptionsCategory =
     "Optimization options";
@@ -51,7 +80,7 @@ struct OptimizationOptions : public ToolOptions {
             PassOptions::DEFAULT_OPTIMIZE_LEVEL == OS_OPTIMIZE_LEVEL &&
               PassOptions::DEFAULT_SHRINK_LEVEL == OS_SHRINK_LEVEL,
             "Help text states that -O is equivalent to -Os but now it isn't.");
-          passes.push_back(DEFAULT_OPT_PASSES);
+          addDefaultOptPasses();
         })
       .add("",
            "-O0",
@@ -71,7 +100,7 @@ struct OptimizationOptions : public ToolOptions {
            [this](Options*, const std::string&) {
              passOptions.optimizeLevel = 1;
              passOptions.shrinkLevel = 0;
-             passes.push_back(DEFAULT_OPT_PASSES);
+             addDefaultOptPasses();
            })
       .add(
         "",
@@ -82,7 +111,7 @@ struct OptimizationOptions : public ToolOptions {
         [this](Options*, const std::string&) {
           passOptions.optimizeLevel = 2;
           passOptions.shrinkLevel = 0;
-          passes.push_back(DEFAULT_OPT_PASSES);
+          addDefaultOptPasses();
         })
       .add("",
            "-O3",
@@ -93,7 +122,7 @@ struct OptimizationOptions : public ToolOptions {
            [this](Options*, const std::string&) {
              passOptions.optimizeLevel = 3;
              passOptions.shrinkLevel = 0;
-             passes.push_back(DEFAULT_OPT_PASSES);
+             addDefaultOptPasses();
            })
       .add("",
            "-O4",
@@ -105,7 +134,7 @@ struct OptimizationOptions : public ToolOptions {
            [this](Options*, const std::string&) {
              passOptions.optimizeLevel = 4;
              passOptions.shrinkLevel = 0;
-             passes.push_back(DEFAULT_OPT_PASSES);
+             addDefaultOptPasses();
            })
       .add("",
            "-Os",
@@ -115,7 +144,7 @@ struct OptimizationOptions : public ToolOptions {
            [this](Options*, const std::string&) {
              passOptions.optimizeLevel = OS_OPTIMIZE_LEVEL;
              passOptions.shrinkLevel = OS_SHRINK_LEVEL;
-             passes.push_back(DEFAULT_OPT_PASSES);
+             addDefaultOptPasses();
            })
       .add("",
            "-Oz",
@@ -125,7 +154,7 @@ struct OptimizationOptions : public ToolOptions {
            [this](Options*, const std::string&) {
              passOptions.optimizeLevel = 2;
              passOptions.shrinkLevel = 2;
-             passes.push_back(DEFAULT_OPT_PASSES);
+             addDefaultOptPasses();
            })
       .add("--optimize-level",
            "-ol",
@@ -293,7 +322,7 @@ struct OptimizationOptions : public ToolOptions {
 
   bool runningDefaultOptimizationPasses() {
     for (auto& pass : passes) {
-      if (pass == DEFAULT_OPT_PASSES) {
+      if (pass.name == DEFAULT_OPT_PASSES) {
         return true;
       }
     }
@@ -308,11 +337,25 @@ struct OptimizationOptions : public ToolOptions {
       passRunner.setDebug(true);
     }
     for (auto& pass : passes) {
-      if (pass == DEFAULT_OPT_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;
+      }
+      if (pass.shrinkLevel) {
+        passRunner.options.shrinkLevel = *pass.shrinkLevel;
+      }
+
+      if (pass.name == DEFAULT_OPT_PASSES) {
         passRunner.addDefaultOptimizationPasses();
       } else {
-        passRunner.add(pass);
+        passRunner.add(pass.name);
       }
+
+      // Revert back to the default levels, if we changed them.
+      passRunner.options.optimizeLevel = oldOptimizeLevel;
+      passRunner.options.shrinkLevel = oldShrinkLevel;
     }
     passRunner.run();
   }
diff --git a/src/tools/wasm-opt.cpp b/src/tools/wasm-opt.cpp
index 538496ecf..effd085bc 100644
--- a/src/tools/wasm-opt.cpp
+++ b/src/tools/wasm-opt.cpp
@@ -59,9 +59,10 @@ static std::string runCommand(std::string command) {
 #endif
 }
 
-static bool willRemoveDebugInfo(const std::vector<std::string>& passes) {
+static bool
+willRemoveDebugInfo(const std::vector<OptimizationOptions::PassInfo>& passes) {
   for (auto& pass : passes) {
-    if (PassRunner::passRemovesDebugInfo(pass)) {
+    if (PassRunner::passRemovesDebugInfo(pass.name)) {
       return true;
     }
   }
@@ -250,8 +251,11 @@ int main(int argc, const char* argv[]) {
     // If the user asked to print the module, print it even if invalid,
     // as otherwise there is no way to print the broken module (the pass
     // to print would not be reached).
-    if (std::find(options.passes.begin(), options.passes.end(), "print") !=
-        options.passes.end()) {
+    if (std::any_of(options.passes.begin(),
+                    options.passes.end(),
+                    [](const OptimizationOptions::PassInfo& info) {
+                      return info.name == "print";
+                    })) {
       std::cout << wasm << '\n';
     }
     Fatal() << message;