[llvm] r291882 - Track validity of pass results
Mehdi Amini via llvm-commits
llvm-commits at lists.llvm.org
Sat Jan 14 22:27:21 PST 2017
Hi,
Did you get Chandler approval before committing as it was requested during the review?
—
Mehdi
> On Jan 13, 2017, at 3:20 AM, Serge Pavlov via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> 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 <mailto: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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <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 <mailto:llvm-commits at lists.llvm.org>
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>
> _______________________________________________
> 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/20170114/2c79b01f/attachment.html>
More information about the llvm-commits
mailing list