diff --git a/llvm/include/llvm/IR/StructuralHash.h b/llvm/include/llvm/IR/StructuralHash.h --- a/llvm/include/llvm/IR/StructuralHash.h +++ b/llvm/include/llvm/IR/StructuralHash.h @@ -1,4 +1,4 @@ -//===- llvm/IR/StructuralHash.h - IR Hash for expensive checks --*- C++ -*-===// +//===- llvm/IR/StructuralHash.h - IR Hashing --------------------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -14,11 +14,8 @@ #ifndef LLVM_IR_STRUCTURALHASH_H #define LLVM_IR_STRUCTURALHASH_H -#ifdef EXPENSIVE_CHECKS - #include -// This header is only meant to be used when -DEXPENSIVE_CHECKS is set namespace llvm { class Function; @@ -30,5 +27,3 @@ } // end namespace llvm #endif - -#endif // LLVM_IR_STRUCTURALHASH_H diff --git a/llvm/lib/IR/StructuralHash.cpp b/llvm/lib/IR/StructuralHash.cpp --- a/llvm/lib/IR/StructuralHash.cpp +++ b/llvm/lib/IR/StructuralHash.cpp @@ -1,13 +1,10 @@ -//===-- StructuralHash.cpp - IR Hash for expensive checks -------*- C++ -*-===// +//===-- StructuralHash.cpp - IR Hashing -------------------------*- 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" @@ -77,5 +74,3 @@ H.update(M); return H.getHash(); } - -#endif diff --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp --- a/llvm/lib/Passes/StandardInstrumentations.cpp +++ b/llvm/lib/Passes/StandardInstrumentations.cpp @@ -24,6 +24,7 @@ #include "llvm/IR/PassInstrumentation.h" #include "llvm/IR/PassManager.h" #include "llvm/IR/PrintPasses.h" +#include "llvm/IR/StructuralHash.h" #include "llvm/IR/Verifier.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/CrashRecoveryContext.h" @@ -1049,6 +1050,23 @@ AnalysisKey PreservedCFGCheckerAnalysis::Key; +struct PreservedFunctionHashAnalysis + : public AnalysisInfoMixin { + static AnalysisKey Key; + + struct FunctionHash { + uint64_t Hash; + }; + + using Result = FunctionHash; + + Result run(Function &F, FunctionAnalysisManager &FAM) { + return Result{StructuralHash(F)}; + } +}; + +AnalysisKey PreservedFunctionHashAnalysis::Key; + bool PreservedCFGCheckerInstrumentation::CFG::invalidate( Function &F, const PreservedAnalyses &PA, FunctionAnalysisManager::Invalidator &) { @@ -1063,6 +1081,7 @@ return; FAM.registerPass([&] { return PreservedCFGCheckerAnalysis(); }); + FAM.registerPass([&] { return PreservedFunctionHashAnalysis(); }); PIC.registerBeforeNonSkippedPassCallback( [this, &FAM](StringRef P, Any IR) { @@ -1076,6 +1095,8 @@ // Make sure a fresh CFG snapshot is available before the pass. FAM.getResult(*const_cast(*F)); + FAM.getResult( + *const_cast(*F)); }); PIC.registerAfterPassInvalidatedCallback( @@ -1095,9 +1116,19 @@ #endif (void)this; - const auto **F = any_cast(&IR); - if (!F) + const auto **MaybeF = any_cast(&IR); + if (!MaybeF) return; + Function &F = *const_cast(*MaybeF); + + if (auto *HashBefore = + FAM.getCachedResult(F)) { + if (HashBefore->Hash != StructuralHash(F)) { + report_fatal_error(formatv( + "Function @{0} changed by {1} without invalidating analyses", + F.getName(), P)); + } + } auto CheckCFG = [](StringRef Pass, StringRef FuncName, const CFG &GraphBefore, const CFG &GraphAfter) { @@ -1112,10 +1143,9 @@ report_fatal_error(Twine("CFG unexpectedly changed by ", Pass)); }; - if (auto *GraphBefore = FAM.getCachedResult( - *const_cast(*F))) - CheckCFG(P, (*F)->getName(), *GraphBefore, - CFG(*F, /* TrackBBLifetime */ false)); + if (auto *GraphBefore = FAM.getCachedResult(F)) + CheckCFG(P, F.getName(), *GraphBefore, + CFG(&F, /* TrackBBLifetime */ false)); }); } @@ -2151,18 +2181,17 @@ TimePasses.registerCallbacks(PIC); OptNone.registerCallbacks(PIC); OptPassGate.registerCallbacks(PIC); - if (FAM) - PreservedCFGChecker.registerCallbacks(PIC, *FAM); PrintChangedIR.registerCallbacks(PIC); PseudoProbeVerification.registerCallbacks(PIC); if (VerifyEach) Verify.registerCallbacks(PIC); PrintChangedDiff.registerCallbacks(PIC); WebsiteChangeReporter.registerCallbacks(PIC); - ChangeTester.registerCallbacks(PIC); - PrintCrashIR.registerCallbacks(PIC); + if (FAM) + PreservedCFGChecker.registerCallbacks(PIC, *FAM); + // TimeProfiling records the pass running time cost. // Its 'BeforePassCallback' can be appended at the tail of all the // BeforeCallbacks by calling `registerCallbacks` in the end. diff --git a/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll b/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll --- a/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll +++ b/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-markloopasdeleted.ll @@ -1,4 +1,4 @@ -; RUN: opt < %s -enable-loop-distribute -passes='loop-distribute,loop-mssa(simple-loop-unswitch),loop-distribute' -o /dev/null -S -verify-analysis-invalidation -debug-pass-manager=verbose 2>&1 | FileCheck %s +; RUN: opt < %s -enable-loop-distribute -passes='loop-distribute,loop-mssa(simple-loop-unswitch),loop-distribute' -o /dev/null -S -verify-analysis-invalidation=0 -debug-pass-manager=verbose 2>&1 | FileCheck %s ; Running loop-distribute will result in LoopAccessAnalysis being required and @@ -29,7 +29,6 @@ ; ; CHECK: Invalidating analysis: LoopAccessAnalysis on test6 ; CHECK-NEXT: Running pass: LoopDistributePass on test6 -; CHECK-NEXT: Running analysis: PreservedCFGCheckerAnalysis on test6 ; CHECK-NEXT: Running analysis: LoopAccessAnalysis on test6 diff --git a/llvm/unittests/IR/PassManagerTest.cpp b/llvm/unittests/IR/PassManagerTest.cpp --- a/llvm/unittests/IR/PassManagerTest.cpp +++ b/llvm/unittests/IR/PassManagerTest.cpp @@ -950,4 +950,37 @@ FPM.addPass(TestSimplifyCFGWrapperPass(InnerFPM)); FPM.run(*F, FAM); } + +#ifdef EXPENSIVE_CHECKS + +struct WrongFunctionPass : PassInfoMixin { + PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM) { + F.getEntryBlock().begin()->eraseFromParent(); + return PreservedAnalyses::all(); + } + static StringRef name() { return "WrongFunctionPass"; } +}; + +TEST_F(PassManagerTest, FunctionAnalysisMissedInvalidation) { + LLVMContext Context; + auto M = parseIR(Context, "define void @foo() {\n" + " %a = add i32 0, 0\n" + " ret void\n" + "}\n"); + + FunctionAnalysisManager FAM; + PassInstrumentationCallbacks PIC; + StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ false); + SI.registerCallbacks(PIC, &FAM); + FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); }); + + FunctionPassManager FPM; + FPM.addPass(WrongFunctionPass()); + + auto *F = M->getFunction("foo"); + EXPECT_DEATH(FPM.run(*F, FAM), "Function @foo changed by WrongFunctionPass without invalidating analyses"); +} + +#endif + }