[PATCH] D86589: (Expensive) Check for Loop, SCC and Region pass return status

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/include/llvm/IR/StructuralHash.h:28-29
+
+uint64_t StructuralHash(Function const &F);
+uint64_t StructuralHash(Module const &M);
+
----------------
Style nit: I believe LLVM uses leading `const` where possible.


================
Comment at: llvm/lib/Analysis/CallGraphSCCPass.cpp:481
+
+#if defined(EXPENSIVE_CHECKS) && !defined(NDEBUG)
+    if (!LocalChanged && (RefHash != StructuralHash(CG.getModule()))) {
----------------
There is a mismatch between `#if` checks here. Above only EXPENSIVE_CHECKS is checked, here !NDEBUG is checked as well.

Aren't we guaranteed that !NDEBUG if EXPENSIVE_CHECKS is set? In any case, both should be consistent.


================
Comment at: llvm/lib/Analysis/CallGraphSCCPass.cpp:485
+                   << P->getPassName() << "\n";
+      assert(false && "Pass modifies its input and doesn't report it.");
+    }
----------------
Style nit: Typically `llvm_unreachable()` is used instead of `assert(false)`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86589/new/

https://reviews.llvm.org/D86589