[llvm] b1f4e59 - (Expensive) Check for Loop, SCC and Region pass return status
via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 27 22:56:49 PDT 2020
Author: serge-sans-paille
Date: 2020-08-28T07:56:35+02:00
New Revision: b1f4e5979b74ccc6e2228b8ba54c40ea4af73907
URL: https://github.com/llvm/llvm-project/commit/b1f4e5979b74ccc6e2228b8ba54c40ea4af73907
DIFF: https://github.com/llvm/llvm-project/commit/b1f4e5979b74ccc6e2228b8ba54c40ea4af73907.diff
LOG: (Expensive) Check for Loop, SCC and Region pass return status
This generalizes the logic introduced in https://reviews.llvm.org/D80916 to
other passes.
It's needed by https://reviews.llvm.org/D86442 to assert passes correctly report
their status.
Differential Revision: https://reviews.llvm.org/D86589
Added:
llvm/include/llvm/IR/StructuralHash.h
llvm/lib/IR/StructuralHash.cpp
Modified:
llvm/lib/Analysis/CallGraphSCCPass.cpp
llvm/lib/Analysis/LoopPass.cpp
llvm/lib/Analysis/RegionPass.cpp
llvm/lib/IR/CMakeLists.txt
llvm/lib/IR/LegacyPassManager.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/StructuralHash.h b/llvm/include/llvm/IR/StructuralHash.h
new file mode 100644
index 000000000000..eb63a2140310
--- /dev/null
+++ b/llvm/include/llvm/IR/StructuralHash.h
@@ -0,0 +1,34 @@
+//===- llvm/IR/StructuralHash.h - IR Hash for expensive checks --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides hashing of the LLVM IR structure to be used to check
+// Passes modification status.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_IR_STRUCTURALHASH_H
+#define LLVM_IR_STRUCTURALHASH_H
+
+#ifdef EXPENSIVE_CHECKS
+
+#include <cstdint>
+
+// This header is only meant to be used when -DEXPENSIVE_CHECKS is set
+namespace llvm {
+
+class Function;
+class Module;
+
+uint64_t StructuralHash(const Function &F);
+uint64_t StructuralHash(const Module &M);
+
+} // end namespace llvm
+
+#endif
+
+#endif // LLVM_IR_STRUCTURALHASH_H
diff --git a/llvm/lib/Analysis/CallGraphSCCPass.cpp b/llvm/lib/Analysis/CallGraphSCCPass.cpp
index 91f8029cc326..17dd4dd389d5 100644
--- a/llvm/lib/Analysis/CallGraphSCCPass.cpp
+++ b/llvm/lib/Analysis/CallGraphSCCPass.cpp
@@ -28,6 +28,7 @@
#include "llvm/IR/Module.h"
#include "llvm/IR/OptBisect.h"
#include "llvm/IR/PassTimingInfo.h"
+#include "llvm/IR/StructuralHash.h"
#include "llvm/Pass.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
@@ -466,11 +467,24 @@ bool CGPassManager::RunAllPassesOnSCC(CallGraphSCC &CurSCC, CallGraph &CG,
initializeAnalysisImpl(P);
+#ifdef EXPENSIVE_CHECKS
+ uint64_t RefHash = StructuralHash(CG.getModule());
+#endif
+
// Actually run this pass on the current SCC.
- Changed |= RunPassOnSCC(P, CurSCC, CG,
- CallGraphUpToDate, DevirtualizedCall);
+ bool LocalChanged =
+ RunPassOnSCC(P, CurSCC, CG, CallGraphUpToDate, DevirtualizedCall);
- if (Changed)
+ Changed |= LocalChanged;
+
+#ifdef EXPENSIVE_CHECKS
+ if (!LocalChanged && (RefHash != StructuralHash(CG.getModule()))) {
+ llvm::errs() << "Pass modifies its input and doesn't report it: "
+ << P->getPassName() << "\n";
+ llvm_unreachable("Pass modifies its input and doesn't report it");
+ }
+#endif
+ if (LocalChanged)
dumpPassInfo(P, MODIFICATION_MSG, ON_CG_MSG, "");
dumpPreservedSet(P);
diff --git a/llvm/lib/Analysis/LoopPass.cpp b/llvm/lib/Analysis/LoopPass.cpp
index 520f06003dd2..317b9577d791 100644
--- a/llvm/lib/Analysis/LoopPass.cpp
+++ b/llvm/lib/Analysis/LoopPass.cpp
@@ -20,6 +20,7 @@
#include "llvm/IR/OptBisect.h"
#include "llvm/IR/PassManager.h"
#include "llvm/IR/PassTimingInfo.h"
+#include "llvm/IR/StructuralHash.h"
#include "llvm/InitializePasses.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/TimeProfiler.h"
@@ -191,7 +192,19 @@ bool LPPassManager::runOnFunction(Function &F) {
{
PassManagerPrettyStackEntry X(P, *CurrentLoop->getHeader());
TimeRegion PassTimer(getPassTimer(P));
+#ifdef EXPENSIVE_CHECKS
+ uint64_t RefHash = StructuralHash(F);
+#endif
LocalChanged = P->runOnLoop(CurrentLoop, *this);
+
+#ifdef EXPENSIVE_CHECKS
+ if (!LocalChanged && (RefHash != StructuralHash(F))) {
+ llvm::errs() << "Pass modifies its input and doesn't report it: "
+ << P->getPassName() << "\n";
+ llvm_unreachable("Pass modifies its input and doesn't report it");
+ }
+#endif
+
Changed |= LocalChanged;
if (EmitICRemark) {
unsigned NewSize = F.getInstructionCount();
diff --git a/llvm/lib/Analysis/RegionPass.cpp b/llvm/lib/Analysis/RegionPass.cpp
index 6c0d17b45c62..1e1971f119a0 100644
--- a/llvm/lib/Analysis/RegionPass.cpp
+++ b/llvm/lib/Analysis/RegionPass.cpp
@@ -15,6 +15,7 @@
#include "llvm/Analysis/RegionPass.h"
#include "llvm/IR/OptBisect.h"
#include "llvm/IR/PassTimingInfo.h"
+#include "llvm/IR/StructuralHash.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Timer.h"
#include "llvm/Support/raw_ostream.h"
@@ -90,15 +91,29 @@ bool RGPassManager::runOnFunction(Function &F) {
initializeAnalysisImpl(P);
+ bool LocalChanged = false;
{
PassManagerPrettyStackEntry X(P, *CurrentRegion->getEntry());
TimeRegion PassTimer(getPassTimer(P));
- Changed |= P->runOnRegion(CurrentRegion, *this);
+#ifdef EXPENSIVE_CHECKS
+ uint64_t RefHash = StructuralHash(F);
+#endif
+ LocalChanged = P->runOnRegion(CurrentRegion, *this);
+
+#ifdef EXPENSIVE_CHECKS
+ if (!LocalChanged && (RefHash != StructuralHash(F))) {
+ llvm::errs() << "Pass modifies its input and doesn't report it: "
+ << P->getPassName() << "\n";
+ llvm_unreachable("Pass modifies its input and doesn't report it");
+ }
+#endif
+
+ Changed |= LocalChanged;
}
if (isPassDebuggingExecutionsOrMore()) {
- if (Changed)
+ if (LocalChanged)
dumpPassInfo(P, MODIFICATION_MSG, ON_REGION_MSG,
skipThisRegion ? "<deleted>" :
CurrentRegion->getNameStr());
diff --git a/llvm/lib/IR/CMakeLists.txt b/llvm/lib/IR/CMakeLists.txt
index 8fcc10fa38af..49805d5b8c27 100644
--- a/llvm/lib/IR/CMakeLists.txt
+++ b/llvm/lib/IR/CMakeLists.txt
@@ -47,6 +47,7 @@ add_llvm_component_library(LLVMCore
SafepointIRVerifier.cpp
ProfileSummary.cpp
Statepoint.cpp
+ StructuralHash.cpp
Type.cpp
TypeFinder.cpp
Use.cpp
diff --git a/llvm/lib/IR/LegacyPassManager.cpp b/llvm/lib/IR/LegacyPassManager.cpp
index 96434ae3306b..8d9ed917bb61 100644
--- a/llvm/lib/IR/LegacyPassManager.cpp
+++ b/llvm/lib/IR/LegacyPassManager.cpp
@@ -20,6 +20,7 @@
#include "llvm/IR/LegacyPassNameParser.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/PassTimingInfo.h"
+#include "llvm/IR/StructuralHash.h"
#include "llvm/Support/Chrono.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
@@ -1475,75 +1476,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
/// the function, and if so, return true.
@@ -1590,7 +1522,7 @@ bool FPPassManager::runOnFunction(Function &F) {
if (!LocalChanged && (RefHash != StructuralHash(F))) {
llvm::errs() << "Pass modifies its input and doesn't report it: "
<< FP->getPassName() << "\n";
- assert(false && "Pass modifies its input and doesn't report it.");
+ llvm_unreachable("Pass modifies its input and doesn't report it");
}
#endif
diff --git a/llvm/lib/IR/StructuralHash.cpp b/llvm/lib/IR/StructuralHash.cpp
new file mode 100644
index 000000000000..5a6e07451326
--- /dev/null
+++ b/llvm/lib/IR/StructuralHash.cpp
@@ -0,0 +1,84 @@
+//===-- StructuralHash.cpp - IR Hash for expensive checks -------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+
+#ifdef EXPENSIVE_CHECKS
+
+#include "llvm/IR/StructuralHash.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/Module.h"
+
+using namespace llvm;
+
+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(const 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(const Module &M) {
+ for (const Function &F : M)
+ update(F);
+ }
+
+ uint64_t getHash() const { return Hash; }
+};
+
+} // namespace details
+
+} // namespace
+
+uint64_t llvm::StructuralHash(const Function &F) {
+ details::StructuralHash H;
+ H.update(F);
+ return H.getHash();
+}
+
+uint64_t llvm::StructuralHash(const Module &M) {
+ details::StructuralHash H;
+ H.update(M);
+ return H.getHash();
+}
+
+#endif
More information about the llvm-commits
mailing list