Refactor removing module elements (#2489)

aheejin · web-flow · commit 9d4acee666df · 2019-12-02T16:34:03.000-08:00
This creates utility functions for removing module elements: removing
one element by name, and removing multiple elements using a predicate
function. And makes other parts of code use it. I think this is a
light-handed approach than calling `Module::updateMaps` after removing
only a part of module elements.

This also fixes a bug in the inlining pass: it didn't call
`Module::updateMaps` after removing functions. After this patch callers
don't need to additionally call it anyway.
diff --git a/auto_update_tests.py b/auto_update_tests.py
@@ -197,7 +197,7 @@ def update_example_tests():
                  '-I' + os.path.join(shared.options.binaryen_root, 'src'), '-g', '-L' + libdir, '-pthread']
         print('build: ', ' '.join(extra))
         if src.endswith('.cpp'):
-            extra += ['-std=c++11']
+            extra += ['-std=c++14']
         print(os.getcwd())
         subprocess.check_call(extra)
         # Link against the binaryen C library DSO, using rpath
@@ -206,7 +206,7 @@ def update_example_tests():
         if os.environ.get('COMPILER_FLAGS'):
             for f in os.environ.get('COMPILER_FLAGS').split(' '):
                 cmd.append(f)
-        cmd = [os.environ.get('CXX') or 'g++', '-std=c++11'] + cmd
+        cmd = [os.environ.get('CXX') or 'g++', '-std=c++14'] + cmd
         try:
             print('link: ', ' '.join(cmd))
             subprocess.check_call(cmd)
diff --git a/check.py b/check.py
@@ -388,7 +388,7 @@ def run_gcc_tests():
             extra = [shared.NATIVECC, src, '-c', '-o', 'example.o',
                      '-I' + os.path.join(shared.options.binaryen_root, 'src'), '-g', '-L' + libpath, '-pthread']
             if src.endswith('.cpp'):
-                extra += ['-std=c++11']
+                extra += ['-std=c++14']
             if os.environ.get('COMPILER_FLAGS'):
                 for f in os.environ.get('COMPILER_FLAGS').split(' '):
                     extra.append(f)
@@ -402,7 +402,7 @@ def run_gcc_tests():
         if os.environ.get('COMPILER_FLAGS'):
             for f in os.environ.get('COMPILER_FLAGS').split(' '):
                 cmd.append(f)
-        cmd = [shared.NATIVEXX, '-std=c++11'] + cmd
+        cmd = [shared.NATIVEXX, '-std=c++14'] + cmd
         print('link: ', ' '.join(cmd))
         subprocess.check_call(cmd)
         print('run...', output_file)
diff --git a/src/passes/DuplicateFunctionElimination.cpp b/src/passes/DuplicateFunctionElimination.cpp
@@ -89,14 +89,8 @@ struct DuplicateFunctionElimination : public Pass {
       // perform replacements
       if (replacements.size() > 0) {
         // remove the duplicates
-        auto& v = module->functions;
-        v.erase(std::remove_if(v.begin(),
-                               v.end(),
-                               [&](const std::unique_ptr<Function>& curr) {
-                                 return duplicates.count(curr->name) > 0;
-                               }),
-                v.end());
-        module->updateMaps();
+        module->removeFunctions(
+          [&](Function* func) { return duplicates.count(func->name) > 0; });
         OptUtils::replaceFunctions(runner, *module, replacements);
       } else {
         break;
diff --git a/src/passes/Inlining.cpp b/src/passes/Inlining.cpp
@@ -385,23 +385,12 @@ struct Inlining : public Pass {
       OptUtils::optimizeAfterInlining(inlinedInto, module, runner);
     }
     // remove functions that we no longer need after inlining
-    auto& funcs = module->functions;
-    funcs.erase(std::remove_if(funcs.begin(),
-                               funcs.end(),
-                               [&](const std::unique_ptr<Function>& curr) {
-                                 auto name = curr->name;
-                                 auto& info = infos[name];
-                                 bool canRemove =
-                                   inlinedUses.count(name) &&
-                                   inlinedUses[name] == info.calls &&
-                                   !info.usedGlobally;
-#ifdef INLINING_DEBUG
-                                 if (canRemove)
-                                   std::cout << "removing " << name << '\n';
-#endif
-                                 return canRemove;
-                               }),
-                funcs.end());
+    module->removeFunctions([&](Function* func) {
+      auto name = func->name;
+      auto& info = infos[name];
+      return inlinedUses.count(name) && inlinedUses[name] == info.calls &&
+             !info.usedGlobally;
+    });
     // return whether we did any work
     return inlinedUses.size() > 0;
   }
diff --git a/src/passes/RemoveUnusedModuleElements.cpp b/src/passes/RemoveUnusedModuleElements.cpp
@@ -211,40 +211,18 @@ struct RemoveUnusedModuleElements : public Pass {
     // Compute reachability starting from the root set.
     ReachabilityAnalyzer analyzer(module, roots);
     // Remove unreachable elements.
-    {
-      auto& v = module->functions;
-      v.erase(std::remove_if(v.begin(),
-                             v.end(),
-                             [&](const std::unique_ptr<Function>& curr) {
-                               return analyzer.reachable.count(ModuleElement(
-                                        ModuleElementKind::Function,
-                                        curr->name)) == 0;
-                             }),
-              v.end());
-    }
-    {
-      auto& v = module->globals;
-      v.erase(std::remove_if(v.begin(),
-                             v.end(),
-                             [&](const std::unique_ptr<Global>& curr) {
-                               return analyzer.reachable.count(
-                                        ModuleElement(ModuleElementKind::Global,
-                                                      curr->name)) == 0;
-                             }),
-              v.end());
-    }
-    {
-      auto& v = module->events;
-      v.erase(std::remove_if(v.begin(),
-                             v.end(),
-                             [&](const std::unique_ptr<Event>& curr) {
-                               return analyzer.reachable.count(
-                                        ModuleElement(ModuleElementKind::Event,
-                                                      curr->name)) == 0;
-                             }),
-              v.end());
-    }
-    module->updateMaps();
+    module->removeFunctions([&](Function* curr) {
+      return analyzer.reachable.count(
+               ModuleElement(ModuleElementKind::Function, curr->name)) == 0;
+    });
+    module->removeGlobals([&](Global* curr) {
+      return analyzer.reachable.count(
+               ModuleElement(ModuleElementKind::Global, curr->name)) == 0;
+    });
+    module->removeEvents([&](Event* curr) {
+      return analyzer.reachable.count(
+               ModuleElement(ModuleElementKind::Event, curr->name)) == 0;
+    });
     // Handle the memory and table
     if (!exportsMemory && !analyzer.usesMemory) {
       if (!importsMemory) {
@@ -302,14 +280,8 @@ struct RemoveUnusedModuleElements : public Pass {
       call->fullType = canonicalize(call->fullType);
     }
     // remove no-longer used types
-    module->functionTypes.erase(
-      std::remove_if(module->functionTypes.begin(),
-                     module->functionTypes.end(),
-                     [&needed](std::unique_ptr<FunctionType>& type) {
-                       return needed.count(type.get()) == 0;
-                     }),
-      module->functionTypes.end());
-    module->updateMaps();
+    module->removeFunctionTypes(
+      [&](FunctionType* type) { return needed.count(type) == 0; });
   }
 };
 
diff --git a/src/wasm.h b/src/wasm.h
@@ -1396,6 +1396,12 @@ class Module {
   void removeGlobal(Name name);
   void removeEvent(Name name);
 
+  void removeFunctionTypes(std::function<bool(FunctionType*)> pred);
+  void removeExports(std::function<bool(Export*)> pred);
+  void removeFunctions(std::function<bool(Function*)> pred);
+  void removeGlobals(std::function<bool(Global*)> pred);
+  void removeEvents(std::function<bool(Event*)> pred);
+
   void updateMaps();
 
   void clearDebugInfo();
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
@@ -1155,57 +1155,64 @@ Event* Module::addEvent(Event* curr) {
 
 void Module::addStart(const Name& s) { start = s; }
 
-void Module::removeFunctionType(Name name) {
-  for (size_t i = 0; i < functionTypes.size(); i++) {
-    if (functionTypes[i]->name == name) {
-      functionTypes.erase(functionTypes.begin() + i);
+template<typename Vector, typename Map>
+void removeModuleElement(Vector& v, Map& m, Name name) {
+  m.erase(name);
+  for (size_t i = 0; i < v.size(); i++) {
+    if (v[i]->name == name) {
+      v.erase(v.begin() + i);
       break;
     }
   }
-  functionTypesMap.erase(name);
 }
 
+void Module::removeFunctionType(Name name) {
+  removeModuleElement(functionTypes, functionTypesMap, name);
+}
 void Module::removeExport(Name name) {
-  for (size_t i = 0; i < exports.size(); i++) {
-    if (exports[i]->name == name) {
-      exports.erase(exports.begin() + i);
-      break;
-    }
-  }
-  exportsMap.erase(name);
+  removeModuleElement(exports, exportsMap, name);
 }
-
 void Module::removeFunction(Name name) {
-  for (size_t i = 0; i < functions.size(); i++) {
-    if (functions[i]->name == name) {
-      functions.erase(functions.begin() + i);
-      break;
-    }
-  }
-  functionsMap.erase(name);
+  removeModuleElement(functions, functionsMap, name);
 }
-
 void Module::removeGlobal(Name name) {
-  for (size_t i = 0; i < globals.size(); i++) {
-    if (globals[i]->name == name) {
-      globals.erase(globals.begin() + i);
-      break;
-    }
-  }
-  globalsMap.erase(name);
+  removeModuleElement(globals, globalsMap, name);
 }
-
 void Module::removeEvent(Name name) {
-  for (size_t i = 0; i < events.size(); i++) {
-    if (events[i]->name == name) {
-      events.erase(events.begin() + i);
-      break;
+  removeModuleElement(events, eventsMap, name);
+}
+
+template<typename Vector, typename Map, typename Elem>
+void removeModuleElements(Vector& v,
+                          Map& m,
+                          std::function<bool(Elem* elem)> pred) {
+  for (auto it = m.begin(); it != m.end();) {
+    if (pred(it->second)) {
+      it = m.erase(it);
+    } else {
+      it++;
     }
   }
-  eventsMap.erase(name);
+  v.erase(
+    std::remove_if(v.begin(), v.end(), [&](auto& e) { return pred(e.get()); }),
+    v.end());
 }
 
-// TODO: remove* for other elements
+void Module::removeFunctionTypes(std::function<bool(FunctionType*)> pred) {
+  removeModuleElements(functionTypes, functionTypesMap, pred);
+}
+void Module::removeExports(std::function<bool(Export*)> pred) {
+  removeModuleElements(exports, exportsMap, pred);
+}
+void Module::removeFunctions(std::function<bool(Function*)> pred) {
+  removeModuleElements(functions, functionsMap, pred);
+}
+void Module::removeGlobals(std::function<bool(Global*)> pred) {
+  removeModuleElements(globals, globalsMap, pred);
+}
+void Module::removeEvents(std::function<bool(Event*)> pred) {
+  removeModuleElements(events, eventsMap, pred);
+}
 
 void Module::updateMaps() {
   functionsMap.clear();
