[llvm-branch-commits] [llvm] e62452b - Revert "Double check that passes correctly set their Modified status"

[Hans Wennborg via llvm-branch-commits]

Author: Hans Wennborg
Date: 2020-09-15T16:47:29+02:00
New Revision: e62452bb3e1e163daf75914cdf2e86deb4debf50

URL: https://github.com/llvm/llvm-project/commit/e62452bb3e1e163daf75914cdf2e86deb4debf50
DIFF: https://github.com/llvm/llvm-project/commit/e62452bb3e1e163daf75914cdf2e86deb4debf50.diff

LOG: Revert "Double check that passes correctly set their Modified status"

This check fires during self-host.

> The approach is simple: if a pass reports that it's not modifying a
> Function/Module, compute a loose hash of that Function/Module and compare it
> with the original one. If we report no change but there's a hash change, then we
> have an error.
>
> This approach misses a lot of change but it's not super intrusive and can
> detect most of the simple mistakes.
>
> Differential Revision: https://reviews.llvm.org/D80916

This reverts commit 3667d87a33d3c8d4072a41fd84bb880c59347dc0.

Added: 
    

Modified: 
    llvm/lib/IR/LegacyPassManager.cpp
    llvm/unittests/IR/LegacyPassManagerTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/LegacyPassManager.cpp b/llvm/lib/IR/LegacyPassManager.cpp
index 74869fa62c66..4189aea46294 100644
--- a/llvm/lib/IR/LegacyPassManager.cpp
+++ b/llvm/lib/IR/LegacyPassManager.cpp
@@ -1475,74 +1475,6 @@ void FPPassManager::dumpPassStructure(unsigned Offset) {
   }
 }
 
-#ifdef EXPENSIVE_CHECKS
-namespace {
-namespace details {
-
-// Basic hashing mechanism to detect structural change to the IR, used to verify
-// pass return status consistency with actual change. Loosely copied from
-// llvm/lib/Transforms/Utils/FunctionComparator.cpp
-
-class StructuralHash {
-  uint64_t Hash = 0x6acaa36bef8325c5ULL;
-
-  void update(uint64_t V) { Hash = hashing::detail::hash_16_bytes(Hash, V); }
-
-public:
-  StructuralHash() = default;
-
-  void update(Function &F) {
-    if (F.empty())
-      return;
-
-    update(F.isVarArg());
-    update(F.arg_size());
-
-    SmallVector<const BasicBlock *, 8> BBs;
-    SmallPtrSet<const BasicBlock *, 16> VisitedBBs;
-
-    BBs.push_back(&F.getEntryBlock());
-    VisitedBBs.insert(BBs[0]);
-    while (!BBs.empty()) {
-      const BasicBlock *BB = BBs.pop_back_val();
-      update(45798); // Block header
-      for (auto &Inst : *BB)
-        update(Inst.getOpcode());
-
-      const Instruction *Term = BB->getTerminator();
-      for (unsigned i = 0, e = Term->getNumSuccessors(); i != e; ++i) {
-        if (!VisitedBBs.insert(Term->getSuccessor(i)).second)
-          continue;
-        BBs.push_back(Term->getSuccessor(i));
-      }
-    }
-  }
-
-  void update(Module &M) {
-    for (Function &F : M)
-      update(F);
-  }
-
-  uint64_t getHash() const { return Hash; }
-};
-
-} // namespace details
-
-uint64_t StructuralHash(Function &F) {
-  details::StructuralHash H;
-  H.update(F);
-  return H.getHash();
-}
-
-uint64_t StructuralHash(Module &M) {
-  details::StructuralHash H;
-  H.update(M);
-  return H.getHash();
-}
-
-} // end anonymous namespace
-
-#endif
 
 /// Execute all of the passes scheduled for execution by invoking
 /// runOnFunction method.  Keep track of whether any of the passes modifies
@@ -1581,16 +1513,7 @@ bool FPPassManager::runOnFunction(Function &F) {
     {
       PassManagerPrettyStackEntry X(FP, F);
       TimeRegion PassTimer(getPassTimer(FP));
-#ifdef EXPENSIVE_CHECKS
-      uint64_t RefHash = StructuralHash(F);
-#endif
       LocalChanged |= FP->runOnFunction(F);
-
-#ifdef EXPENSIVE_CHECKS
-      assert((LocalChanged || (RefHash == StructuralHash(F))) &&
-             "Pass modifies its input and doesn't report it.");
-#endif
-
       if (EmitICRemark) {
         unsigned NewSize = F.getInstructionCount();
 
@@ -1691,17 +1614,7 @@ MPPassManager::runOnModule(Module &M) {
       PassManagerPrettyStackEntry X(MP, M);
       TimeRegion PassTimer(getPassTimer(MP));
 
-#ifdef EXPENSIVE_CHECKS
-      uint64_t RefHash = StructuralHash(M);
-#endif
-
       LocalChanged |= MP->runOnModule(M);
-
-#ifdef EXPENSIVE_CHECKS
-      assert((LocalChanged || (RefHash == StructuralHash(M))) &&
-             "Pass modifies its input and doesn't report it.");
-#endif
-
       if (EmitICRemark) {
         // Update the size of the module.
         unsigned ModuleCount = M.getInstructionCount();

diff  --git a/llvm/unittests/IR/LegacyPassManagerTest.cpp b/llvm/unittests/IR/LegacyPassManagerTest.cpp
index 8dda94b1b032..b7801b52481d 100644
--- a/llvm/unittests/IR/LegacyPassManagerTest.cpp
+++ b/llvm/unittests/IR/LegacyPassManagerTest.cpp
@@ -680,7 +680,7 @@ namespace llvm {
       ASSERT_EQ(M->getFunctionList().size(), 4U);
       Function *F = M->getFunction("test2");
       Function *SF = splitSimpleFunction(*F);
-      CallInst::Create(F, "", &*SF->getEntryBlock().getFirstInsertionPt());
+      CallInst::Create(F, "", &SF->getEntryBlock());
       ASSERT_EQ(M->getFunctionList().size(), 5U);
       CGModifierPass *P = new CGModifierPass();
       legacy::PassManager Passes;