[passmanager] Change the verifier analysis to use function names inst…

…ead of SILFunction pointers for its internal state.

This enables us to have state independent of the liveness of the SILFunction's
that we are tracking.

I also changed the verifier to implement only verifyFull instead of verify to
ensure that when we run with sil-verify-all this only runs at the end of pass
manager pipelines.

rdar://42301529

include/swift/SILOptimizer/Analysis/PassManagerVerifierAnalysis.h

@@ -16,6 +16,7 @@
 #include "swift/SIL/SILFunction.h"
 #include "swift/SILOptimizer/Analysis/Analysis.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/StringSet.h"
 
 namespace swift {
 
@@ -28,10 +29,12 @@ class PassManagerVerifierAnalysis : public SILAnalysis {
   LLVM_ATTRIBUTE_UNUSED
   SILModule &mod;
 
-  /// The set of "live" functions that we are tracking.
+  /// The set of "live" functions that we are tracking. We store the names of
+  /// the functions so that if a function is deleted we do not need to touch its
+  /// memory to get its name.
   ///
   /// All functions in mod must be in liveFunctions and vis-a-versa.
-  llvm::DenseSet<SILFunction *> liveFunctions;
+  llvm::StringSet<> liveFunctionNames;
 
 public:
   PassManagerVerifierAnalysis(SILModule *mod);
@@ -58,7 +61,7 @@ class PassManagerVerifierAnalysis : public SILAnalysis {
   void invalidateFunctionTables() override final;
 
   /// Run the entire verification.
-  void verify() const override final;
+  void verifyFull() const override final;
 };
 
 } // namespace swift

lib/SILOptimizer/Analysis/PassManagerVerifierAnalysis.cpp

@@ -31,7 +31,7 @@ PassManagerVerifierAnalysis::PassManagerVerifierAnalysis(SILModule *mod)
   for (auto &fn : *mod) {
     LLVM_DEBUG(llvm::dbgs() << "PMVerifierAnalysis. Add: " << fn.getName()
                             << '\n');
-    liveFunctions.insert(&fn);
+    liveFunctionNames.insert(fn.getName());
   }
 #endif
 }
@@ -52,7 +52,7 @@ void PassManagerVerifierAnalysis::notifyAddedOrModifiedFunction(
     return;
   LLVM_DEBUG(llvm::dbgs() << "PMVerifierAnalysis. Add|Mod: " << f->getName()
                           << '\n');
-  liveFunctions.insert(f);
+  liveFunctionNames.insert(f->getName());
 #endif
 }
 
@@ -63,9 +63,13 @@ void PassManagerVerifierAnalysis::notifyWillDeleteFunction(SILFunction *f) {
     return;
   LLVM_DEBUG(llvm::dbgs() << "PMVerifierAnalysis. Delete: " << f->getName()
                           << '\n');
-  assert(liveFunctions.count(f) &&
-         "Tried to delete function that analysis was not aware of?!");
-  liveFunctions.erase(f);
+  if (liveFunctionNames.erase(f->getName()))
+    return;
+
+  llvm::errs()
+      << "Error! Tried to delete function that analysis was not aware of: "
+      << f->getName() << '\n';
+  llvm_unreachable("triggering standard assertion failure routine");
 #endif
 }
 
@@ -74,22 +78,49 @@ void PassManagerVerifierAnalysis::notifyWillDeleteFunction(SILFunction *f) {
 void PassManagerVerifierAnalysis::invalidateFunctionTables() {}
 
 /// Run the entire verification.
-void PassManagerVerifierAnalysis::verify() const {
+void PassManagerVerifierAnalysis::verifyFull() const {
 #ifndef NDEBUG
   if (!EnableVerifier)
     return;
 
-  // We check that all functions in the module are in liveFunctions /and/ then
-  // make sure that liveFunctions has the same number of elements. If we have
-  // too many elements, this means we missed a delete event.
-  unsigned funcCount = 0;
+  // We check that liveFunctionNames is in sync with the module's function list
+  // by going through the module's function list and attempting to remove all
+  // functions in the module. If we fail to remove fn, then we know that a
+  // function was added to the module without an appropriate message being sent
+  // by the pass manager.
+  bool foundError = false;
+
+  unsigned count = 0;
   for (auto &fn : mod) {
-    ++funcCount;
-    assert(liveFunctions.count(&fn) &&
-           "Found function in module that verifier is not aware of?!");
+    if (liveFunctionNames.count(fn.getName())) {
+      ++count;
+      continue;
+    }
+    llvm::errs() << "Found function in module that was not added to verifier: "
+                 << fn.getName() << '\n';
+    foundError = true;
   }
-  assert(liveFunctions.size() == funcCount &&
-         "Analysis has state for deleted functions?!");
+
+  // Ok, so now we know that function(mod) is a subset of
+  // liveFunctionNames. Relying on the uniqueness provided by the module's
+  // function list, we know that liveFunction should be exactly count in
+  // size. Otherwise, we must have an error. If and only if we detect this
+  // error, do the expensive work of finding the missing deletes. This is an
+  // important performance optimization to avoid a large copy on the hot path.
+  if (liveFunctionNames.size() != count) {
+    auto liveFunctionNamesCopy = llvm::StringSet<>(liveFunctionNames);
+    for (auto &fn : mod) {
+      liveFunctionNamesCopy.erase(fn.getName());
+    }
+    for (auto &iter : liveFunctionNamesCopy) {
+      llvm::errs() << "Missing delete message for function: " << iter.first()
+                   << '\n';
+      foundError = true;
+    }
+  }
+
+  // We assert here so we emit /all/ errors before asserting.
+  assert(!foundError && "triggering standard assertion failure routine");
 #endif
 }