Warn when running a pass not compatible with DWARF

src/pass.h

@@ -183,18 +183,18 @@ struct PassRunner {
 
   // 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(std::move(pass));
+    doAdd(PassRegistry::get()->createPass(passName));
   }
 
   // Add a pass given an instance.
   template<class P> void add(std::unique_ptr<P> pass) {
     doAdd(std::move(pass));
   }
 
+  // Adds the pass if it supports DWARF (or, if DWARF is not present, it always
+  // adds it).
+  void addIfDWARFAllowed(std::string passName);
+
   // Adds the default set of optimization passes; this is
   // what -O does.
   void addDefaultOptimizationPasses();
@@ -312,6 +312,10 @@ class Pass {
   // out any Stack IR - it would need to be regenerated and optimized.
   virtual bool modifiesBinaryenIR() { return true; }
 
+  // Some passes modify the wasm in a way that we cannot update DWARF properly
+  // for. This is used to issue a proper warning about that.
+  virtual bool invalidatesDWARF() { return false; }
+
   std::string name;
 
 protected:

src/passes/CoalesceLocals.cpp

@@ -46,6 +46,10 @@ struct CoalesceLocals
   : public WalkerPass<LivenessWalker<CoalesceLocals, Visitor<CoalesceLocals>>> {
   bool isFunctionParallel() override { return true; }
 
+  // This pass merges locals, mapping the originals to new ones.
+  // FIXME DWARF updating does not handle local changes yet.
+  bool invalidatesDWARF() override { return true; }
+
   Pass* create() override { return new CoalesceLocals; }
 
   // main entry point

src/passes/DeadArgumentElimination.cpp

@@ -254,6 +254,10 @@ struct DAEScanner
 };
 
 struct DAE : public Pass {
+  // This pass changes locals and parameters.
+  // FIXME DWARF updating does not handle local changes yet.
+  bool invalidatesDWARF() override { return true; }
+
   bool optimize = false;
 
   void run(PassRunner* runner, Module* module) override {

src/passes/DuplicateFunctionElimination.cpp

@@ -31,6 +31,9 @@
 namespace wasm {
 
 struct DuplicateFunctionElimination : public Pass {
+  // FIXME Merge DWARF info
+  bool invalidatesDWARF() override { return true; }
+
   void run(PassRunner* runner, Module* module) override {
     // Multiple iterations may be necessary: A and B may be identical only after
     // we see the functions C1 and C2 that they call are in fact identical.

src/passes/Flatten.cpp

@@ -51,6 +51,11 @@ struct Flatten
       ExpressionStackWalker<Flatten, UnifiedExpressionVisitor<Flatten>>> {
   bool isFunctionParallel() override { return true; }
 
+  // Flattening splits the original locals into a great many other ones, losing
+  // track of the originals that DWARF refers to.
+  // FIXME DWARF updating does not handle local changes yet.
+  bool invalidatesDWARF() override { return true; }
+
   Pass* create() override { return new Flatten; }
 
   // For each expression, a bunch of expressions that should execute right

src/passes/Inlining.cpp

@@ -287,6 +287,10 @@ doInlining(Module* module, Function* into, const InliningAction& action) {
 }
 
 struct Inlining : public Pass {
+  // This pass changes locals and parameters.
+  // FIXME DWARF updating does not handle local changes yet.
+  bool invalidatesDWARF() override { return true; }
+
   // whether to optimize where we inline
   bool optimize = false;
 

src/passes/LocalCSE.cpp

@@ -52,6 +52,10 @@ namespace wasm {
 struct LocalCSE : public WalkerPass<LinearExecutionWalker<LocalCSE>> {
   bool isFunctionParallel() override { return true; }
 
+  // CSE adds and reuses locals.
+  // FIXME DWARF updating does not handle local changes yet.
+  bool invalidatesDWARF() override { return true; }
+
   Pass* create() override { return new LocalCSE(); }
 
   struct Usable {

src/passes/MergeLocals.cpp

@@ -58,6 +58,10 @@ struct MergeLocals
       PostWalker<MergeLocals, UnifiedExpressionVisitor<MergeLocals>>> {
   bool isFunctionParallel() override { return true; }
 
+  // This pass merges locals, mapping the originals to new ones.
+  // FIXME DWARF updating does not handle local changes yet.
+  bool invalidatesDWARF() override { return true; }
+
   Pass* create() override { return new MergeLocals(); }
 
   void doWalkFunction(Function* func) {

src/passes/SSAify.cpp

@@ -69,6 +69,10 @@ static LocalSet IMPOSSIBLE_SET;
 struct SSAify : public Pass {
   bool isFunctionParallel() override { return true; }
 
+  // SSAify maps each original local to a number of new ones.
+  // FIXME DWARF updating does not handle local changes yet.
+  bool invalidatesDWARF() override { return true; }
+
   Pass* create() override { return new SSAify(allowMerges); }
 
   SSAify(bool allowMerges) : allowMerges(allowMerges) {}

src/passes/pass.cpp

@@ -49,7 +49,7 @@ void PassRegistry::registerPass(const char* name,
 
 std::unique_ptr<Pass> PassRegistry::createPass(std::string name) {
   if (passInfos.find(name) == passInfos.end()) {
-    return nullptr;
+    Fatal() << "Could not find pass: " << name << "\n";
   }
   std::unique_ptr<Pass> ret;
   ret.reset(passInfos[name].create());
@@ -372,144 +372,128 @@ void PassRegistry::registerPasses() {
   //   "lower-i64", "lowers i64 into pairs of i32s", createLowerInt64Pass);
 }
 
+void PassRunner::addIfDWARFAllowed(std::string passName) {
+  auto pass = PassRegistry::get()->createPass(passName);
+  if (!pass->invalidatesDWARF() ||
+      !Debug::shouldPreserveDWARF(options, *wasm)) {
+    doAdd(std::move(pass));
+  }
+}
+
 void PassRunner::addDefaultOptimizationPasses() {
   addDefaultGlobalOptimizationPrePasses();
   addDefaultFunctionOptimizationPasses();
   addDefaultGlobalOptimizationPostPasses();
 }
 
-// Check whether we should preserve valid DWARF while optimizing. If so, we
-// disable optimizations that currently cause issues with debug info.
-static bool shouldPreserveDWARF(PassOptions& options, Module& wasm) {
-  return options.debugInfo && Debug::hasDWARFSections(wasm);
-}
-
 void PassRunner::addDefaultFunctionOptimizationPasses() {
-  auto preserveDWARF = shouldPreserveDWARF(options, *wasm);
+  // All the additions here are optional if DWARF must be preserved. That is,
+  // when DWARF is relevant we run fewer optimizations.
+  // FIXME: support DWARF in all of them.
+
   // Untangling to semi-ssa form is helpful (but best to ignore merges
   // so as to not introduce new copies).
-  // FIXME DWARF updating does not handle local changes yet.
-  if (!preserveDWARF &&
-      (options.optimizeLevel >= 3 || options.shrinkLevel >= 1)) {
-    add("ssa-nomerge");
+  if (options.optimizeLevel >= 3 || options.shrinkLevel >= 1) {
+    addIfDWARFAllowed("ssa-nomerge");
   }
   // if we are willing to work very very hard, flatten the IR and do opts
   // that depend on flat IR
-  // FIXME DWARF updating does not handle local changes yet.
-  if (!preserveDWARF && options.optimizeLevel >= 4) {
-    add("flatten");
-    add("local-cse");
-  }
-  add("dce");
-  add("remove-unused-names");
-  add("remove-unused-brs");
-  add("remove-unused-names");
-  add("optimize-instructions");
+  if (options.optimizeLevel >= 4) {
+    addIfDWARFAllowed("flatten");
+    addIfDWARFAllowed("local-cse");
+  }
+  addIfDWARFAllowed("dce");
+  addIfDWARFAllowed("remove-unused-names");
+  addIfDWARFAllowed("remove-unused-brs");
+  addIfDWARFAllowed("remove-unused-names");
+  addIfDWARFAllowed("optimize-instructions");
   if (options.optimizeLevel >= 2 || options.shrinkLevel >= 2) {
-    add("pick-load-signs");
+    addIfDWARFAllowed("pick-load-signs");
   }
   // early propagation
   if (options.optimizeLevel >= 3 || options.shrinkLevel >= 2) {
-    add("precompute-propagate");
+    addIfDWARFAllowed("precompute-propagate");
   } else {
-    add("precompute");
+    addIfDWARFAllowed("precompute");
   }
   if (options.lowMemoryUnused) {
     if (options.optimizeLevel >= 3 || options.shrinkLevel >= 1) {
-      add("optimize-added-constants-propagate");
+      addIfDWARFAllowed("optimize-added-constants-propagate");
     } else {
-      add("optimize-added-constants");
+      addIfDWARFAllowed("optimize-added-constants");
     }
   }
   if (options.optimizeLevel >= 2 || options.shrinkLevel >= 2) {
-    add("code-pushing");
+    addIfDWARFAllowed("code-pushing");
   }
   // don't create if/block return values yet, as coalesce can remove copies that
   // that could inhibit
-  add("simplify-locals-nostructure");
-  add("vacuum"); // previous pass creates garbage
-  add("reorder-locals");
+  addIfDWARFAllowed("simplify-locals-nostructure");
+  addIfDWARFAllowed("vacuum"); // previous pass creates garbage
+  addIfDWARFAllowed("reorder-locals");
   // simplify-locals opens opportunities for optimizations
-  add("remove-unused-brs");
+  addIfDWARFAllowed("remove-unused-brs");
   // if we are willing to work hard, also optimize copies before coalescing
-  // FIXME DWARF updating does not handle local changes yet.
-  if (!preserveDWARF &&
-      (options.optimizeLevel >= 3 || options.shrinkLevel >= 2)) {
-    add("merge-locals"); // very slow on e.g. sqlite
-  }
-  // FIXME DWARF updating does not handle local changes yet.
-  if (!preserveDWARF) {
-    add("coalesce-locals");
-  }
-  add("simplify-locals");
-  add("vacuum");
-  add("reorder-locals");
-  // FIXME DWARF updating does not handle local changes yet.
-  if (!preserveDWARF) {
-    add("coalesce-locals");
-    add("reorder-locals");
-  }
-  add("vacuum");
+  if (options.optimizeLevel >= 3 || options.shrinkLevel >= 2) {
+    addIfDWARFAllowed("merge-locals"); // very slow on e.g. sqlite
+  }
+  addIfDWARFAllowed("coalesce-locals");
+  addIfDWARFAllowed("simplify-locals");
+  addIfDWARFAllowed("vacuum");
+  addIfDWARFAllowed("reorder-locals");
+  addIfDWARFAllowed("coalesce-locals");
+  addIfDWARFAllowed("reorder-locals");
+  addIfDWARFAllowed("vacuum");
   if (options.optimizeLevel >= 3 || options.shrinkLevel >= 1) {
-    add("code-folding");
+    addIfDWARFAllowed("code-folding");
   }
-  add("merge-blocks");        // makes remove-unused-brs more effective
-  add("remove-unused-brs");   // coalesce-locals opens opportunities
-  add("remove-unused-names"); // remove-unused-brs opens opportunities
-  add("merge-blocks");        // clean up remove-unused-brs new blocks
+  addIfDWARFAllowed("merge-blocks"); // makes remove-unused-brs more effective
+  addIfDWARFAllowed("remove-unused-brs"); // coalesce-locals opens opportunities
+  addIfDWARFAllowed(
+    "remove-unused-names");          // remove-unused-brs opens opportunities
+  addIfDWARFAllowed("merge-blocks"); // clean up remove-unused-brs new blocks
   // late propagation
   if (options.optimizeLevel >= 3 || options.shrinkLevel >= 2) {
-    add("precompute-propagate");
+    addIfDWARFAllowed("precompute-propagate");
   } else {
-    add("precompute");
+    addIfDWARFAllowed("precompute");
   }
-  add("optimize-instructions");
+  addIfDWARFAllowed("optimize-instructions");
   if (options.optimizeLevel >= 2 || options.shrinkLevel >= 1) {
-    add("rse"); // after all coalesce-locals, and before a final vacuum
+    addIfDWARFAllowed(
+      "rse"); // after all coalesce-locals, and before a final vacuum
   }
-  add("vacuum"); // just to be safe
+  addIfDWARFAllowed("vacuum"); // just to be safe
 }
 
 void PassRunner::addDefaultGlobalOptimizationPrePasses() {
-  // FIXME DWARF updating does not handle merging debug info with merged code.
-  if (!shouldPreserveDWARF(options, *wasm)) {
-    add("duplicate-function-elimination");
-  }
-  add("memory-packing");
+  addIfDWARFAllowed("duplicate-function-elimination");
+  addIfDWARFAllowed("memory-packing");
 }
 
 void PassRunner::addDefaultGlobalOptimizationPostPasses() {
-  auto preserveDWARF = shouldPreserveDWARF(options, *wasm);
-  // FIXME DWARF may be badly affected currently as DAE changes function
-  // signatures and hence params and locals.
-  if (!preserveDWARF &&
-      (options.optimizeLevel >= 2 || options.shrinkLevel >= 1)) {
-    add("dae-optimizing");
-  }
-  // FIXME DWARF updating does not handle inlining yet.
-  if (!preserveDWARF &&
-      (options.optimizeLevel >= 2 || options.shrinkLevel >= 2)) {
-    add("inlining-optimizing");
+  if (options.optimizeLevel >= 2 || options.shrinkLevel >= 1) {
+    addIfDWARFAllowed("dae-optimizing");
   }
-  // Optimizations show more functions as duplicate, so run this here in Post.
-  // FIXME DWARF updating does not handle merging debug info with merged code.
-  if (!preserveDWARF) {
-    add("duplicate-function-elimination");
+  if (options.optimizeLevel >= 2 || options.shrinkLevel >= 2) {
+    addIfDWARFAllowed("inlining-optimizing");
   }
-  add("duplicate-import-elimination");
+  // Optimizations show more functions as duplicate, so run this here in Post.
+  addIfDWARFAllowed("duplicate-function-elimination");
+  addIfDWARFAllowed("duplicate-import-elimination");
   if (options.optimizeLevel >= 2 || options.shrinkLevel >= 2) {
-    add("simplify-globals-optimizing");
+    addIfDWARFAllowed("simplify-globals-optimizing");
   } else {
-    add("simplify-globals");
+    addIfDWARFAllowed("simplify-globals");
   }
-  add("remove-unused-module-elements");
+  addIfDWARFAllowed("remove-unused-module-elements");
   // may allow more inlining/dae/etc., need --converge for that
-  add("directize");
+  addIfDWARFAllowed("directize");
   // perform Stack IR optimizations here, at the very end of the
   // optimization pipeline
   if (options.optimizeLevel >= 2 || options.shrinkLevel >= 1) {
-    add("generate-stack-ir");
-    add("optimize-stack-ir");
+    addIfDWARFAllowed("generate-stack-ir");
+    addIfDWARFAllowed("optimize-stack-ir");
   }
 }
 
@@ -670,6 +654,10 @@ void PassRunner::runOnFunction(Function* func) {
 }
 
 void PassRunner::doAdd(std::unique_ptr<Pass> pass) {
+  if (Debug::shouldPreserveDWARF(options, *wasm) && pass->invalidatesDWARF()) {
+    std::cerr << "warning: running pass '" << pass->name
+              << "' which is not fully compatible with DWARF\n";
+  }
   passes.emplace_back(std::move(pass));
 }
 

src/wasm-debug.h

@@ -23,6 +23,7 @@
 
 #include <string>
 
+#include "pass.h"
 #include "wasm.h"
 
 namespace wasm {
@@ -36,6 +37,10 @@ bool hasDWARFSections(const Module& wasm);
 // Dump the DWARF sections to stdout.
 void dumpDWARF(const Module& wasm);
 
+// Check whether we should preserve valid DWARF while optimizing. (If so, we
+// will disable optimizations that currently cause issues with debug info.)
+bool shouldPreserveDWARF(PassOptions& options, Module& wasm);
+
 // Update the DWARF sections.
 void writeDWARFSections(Module& wasm, const BinaryLocations& newLocations);
 

src/wasm/wasm-debug.cpp

@@ -86,6 +86,10 @@ void dumpDWARF(const Module& wasm) {
   info.context->dump(llvm::outs(), options);
 }
 
+bool shouldPreserveDWARF(PassOptions& options, Module& wasm) {
+  return options.debugInfo && hasDWARFSections(wasm);
+}
+
 //
 // Big picture: We use a DWARFContext to read data, then DWARFYAML support
 // code to write it. That is not the main LLVM Dwarf code used for writing