[llvm] r291882 - Track validity of pass results
Serge Pavlov via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 13 03:20:11 PST 2017
Hi Tobias,
Thank you for fixing it.
It looks like none of the region passes in main llvm repository is an
analysis, otherwise the bug should have been caught during check-llvm.
Thanks,
--Serge
2017-01-13 16:13 GMT+07:00 Tobias Grosser <tobias.grosser at inf.ethz.ch>:
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170113/59f6c110/attachment.html>
More information about the llvm-commits
mailing list