[llvm] r291882 - Track validity of pass results
Tobias Grosser via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 13 01:13:17 PST 2017
Hi Serge,
this change broke the RegionPasses used by Polly. I tried to fix the
problem in
r291887, but would appreciate if you could post-commit review the fix I
committed.
Best,
Tobias
On Fri, Jan 13, 2017, at 07:09 AM, Serge Pavlov via llvm-commits wrote:
> Author: sepavloff
> Date: Fri Jan 13 00:09:54 2017
> New Revision: 291882
>
> URL: http://llvm.org/viewvc/llvm-project?rev=291882&view=rev
> Log:
> Track validity of pass results
>
> Running tests with expensive checks enabled exhibits some problems with
> verification of pass results.
>
> First, the pass verification may require results of analysis that are not
> available. For instance, verification of loop info requires results of
> dominator
> tree analysis. A pass may be marked as conserving loop info but does not
> need to
> be dependent on DominatorTreePass. When a pass manager tries to verify
> that loop
> info is valid, it needs dominator tree, but corresponding analysis may be
> already destroyed as no user of it remained.
>
> Another case is a pass that is skipped. For instance, entities with
> linkage
> available_externally do not need code generation and such passes are
> skipped for
> them. In this case result verification must also be skipped.
>
> To solve these problems this change introduces a special flag to the Pass
> structure to mark passes that have valid results. If this flag is reset,
> verifications dependent on the pass result are skipped.
>
> Differential Revision: https://reviews.llvm.org/D27190
>
> Modified:
> llvm/trunk/include/llvm/Pass.h
> llvm/trunk/include/llvm/PassAnalysisSupport.h
> llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp
> llvm/trunk/lib/Analysis/LoopInfo.cpp
> llvm/trunk/lib/Analysis/LoopPass.cpp
> llvm/trunk/lib/CodeGen/MachineDominators.cpp
> llvm/trunk/lib/CodeGen/MachineFunctionPass.cpp
> llvm/trunk/lib/IR/LegacyPassManager.cpp
> llvm/trunk/lib/IR/Pass.cpp
> llvm/trunk/test/CodeGen/Generic/externally_available.ll
> llvm/trunk/test/CodeGen/Mips/mul.ll
>
> Modified: llvm/trunk/include/llvm/Pass.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Pass.h?rev=291882&r1=291881&r2=291882&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Pass.h (original)
> +++ llvm/trunk/include/llvm/Pass.h Fri Jan 13 00:09:54 2017
> @@ -29,6 +29,7 @@
> #ifndef LLVM_PASS_H
> #define LLVM_PASS_H
>
> +#include <assert.h>
> #include <string>
>
> namespace llvm {
> @@ -82,17 +83,40 @@ class Pass {
> AnalysisResolver *Resolver; // Used to resolve analysis
> const void *PassID;
> PassKind Kind;
> + bool Executed;
> +
> void operator=(const Pass&) = delete;
> Pass(const Pass &) = delete;
>
> public:
> explicit Pass(PassKind K, char &pid)
> - : Resolver(nullptr), PassID(&pid), Kind(K) { }
> + : Resolver(nullptr), PassID(&pid), Kind(K), Executed(false) { }
> virtual ~Pass();
>
> -
> PassKind getPassKind() const { return Kind; }
>
> + /// Returns true if the pass has already executed.
> + ///
> + /// For an analysis pass it means the result is available. If the
> function
> + /// returns false, the pass was not run, was skipped or freed.
> + ///
> + bool isExecuted() const { return Executed; }
> +
> + /// Marks the pass as executed or not.
> + ///
> + /// A pass should be marked as executed, if its 'runOn*' method
> successfully
> + /// finished. When the pass is not needed anymore, it is marked as
> + /// 'non-executed', it takes place in \c freePass. It also occurs when
> the
> + /// pass is skipped for some reason.
> + ///
> + /// The flag should be set prior to call to 'runOn*' method. If it
> decides
> + /// that the pass should be skipped, it will reset the flag.
> + ///
> + void setExecuted(bool x) {
> + assert(x || !getAsImmutablePass()); // Immutable pass cannot be
> invalidated.
> + Executed = x;
> + }
> +
> /// getPassName - Return a nice clean name for a pass. This usually
> /// implemented in terms of the name that is registered by one of the
> /// Registration templates, but can be overloaded directly.
> @@ -279,8 +303,7 @@ public:
> ///
> bool runOnModule(Module &) override { return false; }
>
> - explicit ImmutablePass(char &pid)
> - : ModulePass(pid) {}
> + explicit ImmutablePass(char &pid) : ModulePass(pid) {
> setExecuted(true); }
>
> // Force out-of-line virtual method.
> ~ImmutablePass() override;
> @@ -316,8 +339,9 @@ public:
> protected:
> /// Optional passes call this function to check whether the pass
> should be
> /// skipped. This is the case when Attribute::OptimizeNone is set or
> when
> - /// optimization bisect is over the limit.
> - bool skipFunction(const Function &F) const;
> + /// optimization bisect is over the limit. It also resets flag
> Executed on
> + /// the pass.
> + bool skipFunction(const Function &F);
> };
>
>
>
> Modified: llvm/trunk/include/llvm/PassAnalysisSupport.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/PassAnalysisSupport.h?rev=291882&r1=291881&r2=291882&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/PassAnalysisSupport.h (original)
> +++ llvm/trunk/include/llvm/PassAnalysisSupport.h Fri Jan 13 00:09:54
> 2017
> @@ -206,6 +206,9 @@ AnalysisType *Pass::getAnalysisIfAvailab
> Pass *ResultPass = Resolver->getAnalysisIfAvailable(PI, true);
> if (!ResultPass) return nullptr;
>
> + if (!ResultPass->isExecuted())
> + return nullptr;
> +
> // Because the AnalysisType may not be a subclass of pass (for
> // AnalysisGroups), we use getAdjustedAnalysisPointer here to
> potentially
> // adjust the return pointer (because the class may multiply inherit,
> once
> @@ -234,6 +237,8 @@ AnalysisType &Pass::getAnalysisID(Analys
> assert (ResultPass &&
> "getAnalysis*() called on an analysis that was not "
> "'required' by pass!");
> + assert(ResultPass->isExecuted() &&
> + "getAnalysis*() called on an analysis that was freed");
>
> // Because the AnalysisType may not be a subclass of pass (for
> // AnalysisGroups), we use getAdjustedAnalysisPointer here to
> potentially
>
> Modified: llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp?rev=291882&r1=291881&r2=291882&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp (original)
> +++ llvm/trunk/lib/Analysis/CallGraphSCCPass.cpp Fri Jan 13 00:09:54 2017
> @@ -414,6 +414,7 @@ bool CGPassManager::RunAllPassesOnSCC(Ca
> initializeAnalysisImpl(P);
>
> // Actually run this pass on the current SCC.
> + P->setExecuted(true);
> Changed |= RunPassOnSCC(P, CurSCC, CG,
> CallGraphUpToDate, DevirtualizedCall);
>
>
> Modified: llvm/trunk/lib/Analysis/LoopInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LoopInfo.cpp?rev=291882&r1=291881&r2=291882&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/LoopInfo.cpp (original)
> +++ llvm/trunk/lib/Analysis/LoopInfo.cpp Fri Jan 13 00:09:54 2017
> @@ -722,8 +722,10 @@ void LoopInfoWrapperPass::verifyAnalysis
> // checking by default, LoopPass has been taught to call verifyLoop
> manually
> // during loop pass sequences.
> if (VerifyLoopInfo) {
> - auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
> - LI.verify(DT);
> + if (auto *Analysis =
> getAnalysisIfAvailable<DominatorTreeWrapperPass>()) {
> + auto &DT = Analysis->getDomTree();
> + LI.verify(DT);
> + }
> }
> }
>
>
> Modified: llvm/trunk/lib/Analysis/LoopPass.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LoopPass.cpp?rev=291882&r1=291881&r2=291882&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/LoopPass.cpp (original)
> +++ llvm/trunk/lib/Analysis/LoopPass.cpp Fri Jan 13 00:09:54 2017
> @@ -198,6 +198,7 @@ bool LPPassManager::runOnFunction(Functi
> PassManagerPrettyStackEntry X(P, *CurrentLoop->getHeader());
> TimeRegion PassTimer(getPassTimer(P));
>
> + P->setExecuted(true);
> Changed |= P->runOnLoop(CurrentLoop, *this);
> }
> LoopWasDeleted = CurrentLoop->isInvalid();
>
> Modified: llvm/trunk/lib/CodeGen/MachineDominators.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineDominators.cpp?rev=291882&r1=291881&r2=291882&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineDominators.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineDominators.cpp Fri Jan 13 00:09:54 2017
> @@ -69,7 +69,7 @@ void MachineDominatorTree::releaseMemory
> }
>
> void MachineDominatorTree::verifyAnalysis() const {
> - if (VerifyMachineDomInfo)
> + if (VerifyMachineDomInfo && isExecuted())
> verifyDomTree();
> }
>
>
> Modified: llvm/trunk/lib/CodeGen/MachineFunctionPass.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineFunctionPass.cpp?rev=291882&r1=291881&r2=291882&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineFunctionPass.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineFunctionPass.cpp Fri Jan 13 00:09:54
> 2017
> @@ -38,8 +38,10 @@ Pass *MachineFunctionPass::createPrinter
> bool MachineFunctionPass::runOnFunction(Function &F) {
> // Do not codegen any 'available_externally' functions at all, they
> have
> // definitions outside the translation unit.
> - if (F.hasAvailableExternallyLinkage())
> + if (F.hasAvailableExternallyLinkage()) {
> + setExecuted(false);
> return false;
> + }
>
> MachineModuleInfo &MMI = getAnalysis<MachineModuleInfo>();
> MachineFunction &MF = MMI.getMachineFunction(F);
>
> Modified: llvm/trunk/lib/IR/LegacyPassManager.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LegacyPassManager.cpp?rev=291882&r1=291881&r2=291882&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/LegacyPassManager.cpp (original)
> +++ llvm/trunk/lib/IR/LegacyPassManager.cpp Fri Jan 13 00:09:54 2017
> @@ -955,6 +955,9 @@ void PMDataManager::freePass(Pass *P, St
> AvailableAnalysis.erase(Pos);
> }
> }
> +
> + if (!P->getAsImmutablePass())
> + P->setExecuted(false);
> }
>
> /// Add pass P into the PassVector. Update
> @@ -1293,6 +1296,7 @@ bool BBPassManager::runOnFunction(Functi
> PassManagerPrettyStackEntry X(BP, *I);
> TimeRegion PassTimer(getPassTimer(BP));
>
> + BP->setExecuted(true);
> LocalChanged |= BP->runOnBasicBlock(*I);
> }
>
> @@ -1459,7 +1463,9 @@ bool FunctionPassManagerImpl::run(Functi
>
> initializeAllAnalysisInfo();
> for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) {
> - Changed |= getContainedManager(Index)->runOnFunction(F);
> + FPPassManager *P = getContainedManager(Index);
> + P->setExecuted(true);
> + Changed |= P->runOnFunction(F);
> F.getContext().yield();
> }
>
> @@ -1510,6 +1516,7 @@ bool FPPassManager::runOnFunction(Functi
> PassManagerPrettyStackEntry X(FP, F);
> TimeRegion PassTimer(getPassTimer(FP));
>
> + FP->setExecuted(true);
> LocalChanged |= FP->runOnFunction(F);
> }
>
> @@ -1530,8 +1537,10 @@ bool FPPassManager::runOnFunction(Functi
> bool FPPassManager::runOnModule(Module &M) {
> bool Changed = false;
>
> - for (Function &F : M)
> + for (Function &F : M) {
> + setExecuted(true);
> Changed |= runOnFunction(F);
> + }
>
> return Changed;
> }
> @@ -1587,6 +1596,7 @@ MPPassManager::runOnModule(Module &M) {
> PassManagerPrettyStackEntry X(MP, M);
> TimeRegion PassTimer(getPassTimer(MP));
>
> + MP->setExecuted(true);
> LocalChanged |= MP->runOnModule(M);
> }
>
> @@ -1690,7 +1700,9 @@ bool PassManagerImpl::run(Module &M) {
>
> initializeAllAnalysisInfo();
> for (unsigned Index = 0; Index < getNumContainedManagers(); ++Index) {
> - Changed |= getContainedManager(Index)->runOnModule(M);
> + MPPassManager *P = getContainedManager(Index);
> + P->setExecuted(true);
> + Changed |= P->runOnModule(M);
> M.getContext().yield();
> }
>
>
> Modified: llvm/trunk/lib/IR/Pass.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Pass.cpp?rev=291882&r1=291881&r2=291882&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Pass.cpp (original)
> +++ llvm/trunk/lib/IR/Pass.cpp Fri Jan 13 00:09:54 2017
> @@ -146,13 +146,16 @@ PassManagerType FunctionPass::getPotenti
> return PMT_FunctionPassManager;
> }
>
> -bool FunctionPass::skipFunction(const Function &F) const {
> - if (!F.getContext().getOptBisect().shouldRunPass(this, F))
> +bool FunctionPass::skipFunction(const Function &F) {
> + if (!F.getContext().getOptBisect().shouldRunPass(this, F)) {
> + setExecuted(false);
> return true;
> + }
>
> if (F.hasFnAttribute(Attribute::OptimizeNone)) {
> DEBUG(dbgs() << "Skipping pass '" << getPassName() << "' on function
> "
> << F.getName() << "\n");
> + setExecuted(false);
> return true;
> }
> return false;
>
> Modified: llvm/trunk/test/CodeGen/Generic/externally_available.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Generic/externally_available.ll?rev=291882&r1=291881&r2=291882&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/Generic/externally_available.ll (original)
> +++ llvm/trunk/test/CodeGen/Generic/externally_available.ll Fri Jan 13
> 00:09:54 2017
> @@ -1,4 +1,4 @@
> -; RUN: llc < %s | not grep test_
> +; RUN: llc -verify-machine-dom-info < %s | not grep test_
>
> ; test_function should not be emitted to the .s file.
> define available_externally i32 @test_function() {
>
> Modified: llvm/trunk/test/CodeGen/Mips/mul.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/mul.ll?rev=291882&r1=291881&r2=291882&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/Mips/mul.ll (original)
> +++ llvm/trunk/test/CodeGen/Mips/mul.ll Fri Jan 13 00:09:54 2017
> @@ -1,4 +1,4 @@
> -; RUN: llc -march=mipsel -mattr=mips16 -relocation-model=pic -O3 < %s |
> FileCheck %s -check-prefix=16
> +; RUN: llc -march=mipsel -mattr=mips16 -relocation-model=pic -O3
> -verify-loop-info < %s | FileCheck %s -check-prefix=16
>
> @iiii = global i32 5, align 4
> @jjjj = global i32 -6, align 4
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list