Simpify PassRunner.add() and automatically parallelize parallel functions (#2242)

Main change here is in pass.h, everything else is changes to work with the new API.

The add("name") remains as before, while the weird variadic add(..) which constructed the pass now just gets a std::unique_ptr of a pass. This also makes the memory management internally fully automatic. And it makes it trivial to parallelize WalkerPass::run on parallel passes.

As a benefit, this allows removing a lot of code since in many cases there is no need to create a new pass runner, and running a pass can be just a single line.

Author: kripken

src/asm2wasm.h

@@ -1068,10 +1068,10 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
           passRunner.setValidateGlobally(false);
         }
         // run autodrop first, before optimizations
-        passRunner.add<AutoDrop>();
+        passRunner.add(make_unique<AutoDrop>());
         if (preprocessor.debugInfo) {
           // fix up debug info to better survive optimization
-          passRunner.add<AdjustDebugInfo>();
+          passRunner.add(make_unique<AdjustDebugInfo>());
         }
         // optimize relooper label variable usage at the wasm level, where it is
         // easy
@@ -1680,7 +1680,7 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
   }
   // finalizeCalls also does autoDrop, which is crucial for the non-optimizing
   // case, so that the output of the first pass is valid
-  passRunner.add<FinalizeCalls>(this);
+  passRunner.add(make_unique<FinalizeCalls>(this));
   passRunner.add(ABI::getLegalizationPass(legalizeJavaScriptFFI
                                             ? ABI::LegalizationLevel::Full
                                             : ABI::LegalizationLevel::Minimal));
@@ -1697,11 +1697,11 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
     if (preprocessor.debugInfo) {
       // we would have run this before if optimizing, do it now otherwise. must
       // precede ApplyDebugInfo
-      passRunner.add<AdjustDebugInfo>();
+      passRunner.add(make_unique<AdjustDebugInfo>());
     }
   }
   if (preprocessor.debugInfo) {
-    passRunner.add<ApplyDebugInfo>();
+    passRunner.add(make_unique<ApplyDebugInfo>());
     // FIXME maybe just remove the nops that were debuginfo nodes, if not
     // optimizing?
     passRunner.add("vacuum");

src/binaryen-c.cpp

@@ -3390,10 +3390,8 @@ void BinaryenModuleAutoDrop(BinaryenModuleRef module) {
   }
 
   Module* wasm = (Module*)module;
-  PassRunner passRunner(wasm);
-  passRunner.options = globalPassOptions;
-  passRunner.add<AutoDrop>();
-  passRunner.run();
+  PassRunner runner(wasm, globalPassOptions);
+  AutoDrop().run(&runner, wasm);
 }
 
 static BinaryenBufferSizes writeModule(BinaryenModuleRef module,

src/ir/flat.h

@@ -114,9 +114,7 @@ inline void verifyFlatness(Module* module) {
   };
 
   PassRunner runner(module);
-  runner.setIsNested(true);
-  runner.add<VerifyFlatness>();
-  runner.run();
+  VerifyFlatness().run(&runner, module);
 }
 
 } // namespace Flat

src/ir/module-utils.h

@@ -319,7 +319,6 @@ template<typename T> struct ParallelFunctionMap {
 
     // Run on the implemented functions.
     struct Mapper : public WalkerPass<PostWalker<Mapper>> {
-
       bool isFunctionParallel() override { return true; }
 
       Mapper(Info* info) : info(info) {}
@@ -338,9 +337,7 @@ template<typename T> struct ParallelFunctionMap {
     Info info = {&map, work};
 
     PassRunner runner(&wasm);
-    runner.setIsNested(true);
-    runner.add<Mapper>(&info);
-    runner.run();
+    Mapper(&info).run(&runner, &wasm);
   }
 };
 

src/pass.h

@@ -39,7 +39,7 @@ struct PassRegistry {
   typedef std::function<Pass*()> Creator;
 
   void registerPass(const char* name, const char* description, Creator create);
-  Pass* createPass(std::string name);
+  std::unique_ptr<Pass> createPass(std::string name);
   std::vector<std::string> getRegisteredNames();
   std::string getPassDescription(std::string name);
 
@@ -142,7 +142,7 @@ struct PassOptions {
 struct PassRunner {
   Module* wasm;
   MixedArena* allocator;
-  std::vector<Pass*> passes;
+  std::vector<std::unique_ptr<Pass>> passes;
   PassOptions options;
 
   PassRunner(Module* wasm) : wasm(wasm), allocator(&wasm->allocator) {}
@@ -163,16 +163,18 @@ struct PassRunner {
     options.validateGlobally = validate;
   }
 
+  // Add a pass using its name.
   void add(std::string passName) {
     auto pass = PassRegistry::get()->createPass(passName);
     if (!pass) {
       Fatal() << "Could not find pass: " << passName << "\n";
     }
-    doAdd(pass);
+    doAdd(std::move(pass));
   }
 
-  template<class P, class... Args> void add(Args&&... args) {
-    doAdd(new P(std::forward<Args>(args)...));
+  // Add a pass given an instance.
+  template<class P> void add(std::unique_ptr<P> pass) {
+    doAdd(std::move(pass));
   }
 
   // Adds the default set of optimization passes; this is
@@ -205,8 +207,6 @@ struct PassRunner {
   // Get the last pass that was already executed of a certain type.
   template<class P> P* getLast();
 
-  ~PassRunner();
-
   // When running a pass runner within another pass runner, this
   // flag should be set. This influences how pass debugging works,
   // and may influence other things in the future too.
@@ -227,7 +227,7 @@ struct PassRunner {
   bool isNested = false;
 
 private:
-  void doAdd(Pass* pass);
+  void doAdd(std::unique_ptr<Pass> pass);
 
   void runPass(Pass* pass);
   void runPassOnFunction(Pass* pass, Function* func);
@@ -315,6 +315,17 @@ class WalkerPass : public Pass, public WalkerType {
 
 public:
   void run(PassRunner* runner, Module* module) override {
+    // Parallel pass running is implemented in the PassRunner.
+    if (isFunctionParallel()) {
+      PassRunner runner(module);
+      runner.setIsNested(true);
+      std::unique_ptr<Pass> copy;
+      copy.reset(create());
+      runner.add(std::move(copy));
+      runner.run();
+      return;
+    }
+    // Single-thread running just calls the walkModule traversal.
     setPassRunner(runner);
     WalkerType::setModule(module);
     WalkerType::walkModule(module);

src/passes/Asyncify.cpp

@@ -1025,7 +1025,7 @@ struct Asyncify : public Pass {
         runner.add("reorder-locals");
         runner.add("merge-blocks");
       }
-      runner.add<AsyncifyFlow>(&analyzer);
+      runner.add(make_unique<AsyncifyFlow>(&analyzer));
       runner.setIsNested(true);
       runner.setValidateGlobally(false);
       runner.run();
@@ -1040,7 +1040,7 @@ struct Asyncify : public Pass {
       if (optimize) {
         runner.addDefaultFunctionOptimizationPasses();
       }
-      runner.add<AsyncifyLocals>(&analyzer);
+      runner.add(make_unique<AsyncifyLocals>(&analyzer));
       if (optimize) {
         runner.addDefaultFunctionOptimizationPasses();
       }

src/passes/DeadArgumentElimination.cpp

@@ -236,12 +236,7 @@ struct DAE : public Pass {
       }
     }
     // Scan all the functions.
-    {
-      PassRunner runner(module);
-      runner.setIsNested(true);
-      runner.add<DAEScanner>(&infoMap);
-      runner.run();
-    }
+    DAEScanner(&infoMap).run(runner, module);
     // Combine all the info.
     std::unordered_map<Name, std::vector<Call*>> allCalls;
     for (auto& pair : infoMap) {

src/passes/Directize.cpp

@@ -109,12 +109,7 @@ struct Directize : public Pass {
       return;
     }
     // The table exists and is constant, so this is possible.
-    {
-      PassRunner runner(module);
-      runner.setIsNested(true);
-      runner.add<FunctionDirectizer>(&flatTable);
-      runner.run();
-    }
+    FunctionDirectizer(&flatTable).run(runner, module);
   }
 };
 

src/passes/DuplicateFunctionElimination.cpp

@@ -70,10 +70,7 @@ struct DuplicateFunctionElimination : public Pass {
       limit--;
       // Hash all the functions
       auto hashes = FunctionHasher::createMap(module);
-      PassRunner hasherRunner(module);
-      hasherRunner.setIsNested(true);
-      hasherRunner.add<FunctionHasher>(&hashes);
-      hasherRunner.run();
+      FunctionHasher(&hashes).run(runner, module);
       // Find hash-equal groups
       std::map<uint32_t, std::vector<Function*>> hashGroups;
       ModuleUtils::iterDefinedFunctions(*module, [&](Function* func) {
@@ -121,10 +118,7 @@ struct DuplicateFunctionElimination : public Pass {
                 v.end());
         module->updateMaps();
         // replace direct calls
-        PassRunner replacerRunner(module);
-        replacerRunner.setIsNested(true);
-        replacerRunner.add<FunctionReplacer>(&replacements);
-        replacerRunner.run();
+        FunctionReplacer(&replacements).run(runner, module);
         // replace in table
         for (auto& segment : module->table.segments) {
           for (auto& name : segment.data) {

src/passes/FuncCastEmulation.cpp

@@ -185,10 +185,7 @@ struct FuncCastEmulation : public Pass {
       }
     }
     // update call_indirects
-    PassRunner subRunner(module, runner->options);
-    subRunner.setIsNested(true);
-    subRunner.add<ParallelFuncCastEmulation>(ABIType);
-    subRunner.run();
+    ParallelFuncCastEmulation(ABIType).run(runner, module);
   }
 
 private: