[llvm] r311467 - [ADCE][Dominators] Reapply: Teach ADCE to preserve dominators
Jakub (Kuba) Kuderski via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 23 10:38:07 PDT 2017
Hi Adrian,
Expensive checks make .verifyDominatorTree to run O(n^3) sibling property
verification, which can cause some major slowndowns on big CFG's. I believe
that the question is how expensive we want the expensive checks to be -- if
that' unacceptable, then we can introduce a new flag just for that.
Please let me know what you think.
Best,
Kuba
On Aug 23, 2017 19:27, "Adrian Prantl" <aprantl at apple.com> wrote:
> A lot of jobs on green dragon are currently timing out, for example:
>
> http://green.lab.llvm.org/green/job/clang-stage1-cmake-
> RA-expensive/buildTimeTrend
>
> I logged into one of the build nodes and found several instances of clang,
> all running for >15min and attached LLDB to it:
>
> They were all stuck in llvm::DominatorTree::verifyDomTree().
>
> (lldb) bt
> * thread #1: tid = 0x1607b1d, 0x0000000108ea1776 clang-6.0`llvm::
> DomTreeNodeBase<llvm::BasicBlock>::compare(llvm::DomTreeNodeBase<llvm::BasicBlock>
> const*) const + 118, queue = 'com.apple.main-thread', stop reason = signal
> SIGSTOP
> * frame #0: 0x0000000108ea1776 clang-6.0`llvm::DomTreeNodeBase<llvm::
> BasicBlock>::compare(llvm::DomTreeNodeBase<llvm::BasicBlock> const*)
> const + 118
> frame #1: 0x0000000108ea1fb7 clang-6.0`llvm::DominatorTreeBase<llvm::BasicBlock,
> false>::compare(llvm::DominatorTreeBase<llvm::BasicBlock, false> const&)
> const + 327
> frame #2: 0x0000000108eac121 clang-6.0`llvm::DominatorTree::verifyDomTree()
> const + 225
> frame #3: 0x0000000108f060b9 clang-6.0`llvm::PMDataManager:
> :verifyPreservedAnalysis(llvm::Pass*) + 313
> frame #4: 0x00000001089e6ccb clang-6.0`llvm::LPPassManager:
> :runOnFunction(llvm::Function&) + 1515
> frame #5: 0x0000000108f09ea3 clang-6.0`llvm::FPPassManager:
> :runOnFunction(llvm::Function&) + 547
> frame #6: 0x0000000108f0a103 clang-6.0`llvm::FPPassManager::runOnModule(llvm::Module&)
> + 51
> frame #7: 0x0000000108f0a64e clang-6.0`llvm::legacy::
> PassManagerImpl::run(llvm::Module&) + 958
> frame #8: 0x000000010971c264 clang-6.0`clang::
> EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions
> const&, clang::CodeGenOptions const&, clang::TargetOptions const&,
> clang::LangOptions const&, llvm::DataLayout const&, llvm::Module*,
> clang::BackendAction, std::__1::unique_ptr<llvm::raw_pwrite_stream,
> std::__1::default_delete<llvm::raw_pwrite_stream> >) + 15444
> frame #9: 0x0000000109974633 clang-6.0`clang::BackendConsumer::
> HandleTranslationUnit(clang::ASTContext&) + 947
> frame #10: 0x000000010a3c2ab5 clang-6.0`clang::ParseAST(clang::Sema&,
> bool, bool) + 469
> frame #11: 0x0000000109bfcfec clang-6.0`clang::FrontendAction::Execute()
> + 76
> frame #12: 0x0000000109bb6d91 clang-6.0`clang::CompilerInstance::
> ExecuteAction(clang::FrontendAction&) + 1217
> frame #13: 0x0000000109c6401a clang-6.0`clang::
> ExecuteCompilerInvocation(clang::CompilerInstance*) + 4970
> frame #14: 0x0000000107c1c732 clang-6.0`cc1_main(llvm::ArrayRef<char
> const*>, char const*, void*) + 1394
> frame #15: 0x0000000107c1ade3 clang-6.0`main + 11939
> frame #16: 0x00007fff8fa725ad libdyld.dylib`start + 1
>
> Since you contributed a lot of changes to this code recently, do you think
> that this could be related to your commits? You should be able to reproduce
> this by building with expensive checks enabled and watching out for the
> builds of the ASAN testcases. ("Generating ASAN_.*_TEST_.*" in the build
> log)
>
> thanks,
> Adrian
>
> > On Aug 22, 2017, at 9:30 AM, Jakub Kuderski via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >
> > Author: kuhar
> > Date: Tue Aug 22 09:30:21 2017
> > New Revision: 311467
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=311467&view=rev
> > Log:
> > [ADCE][Dominators] Reapply: Teach ADCE to preserve dominators
> >
> > Summary:
> > This patch teaches ADCE to preserve both DominatorTrees and
> PostDominatorTrees.
> >
> > This is reapplies the original patch r311057 that was reverted in
> r311381.
> > The previous version wasn't using the batch update api for updating
> dominators,
> > which in vary rare cases caused assertion failures.
> >
> > This also fixes PR34258.
> >
> > Reviewers: dberlin, chandlerc, sanjoy, davide, grosser, brzycki
> >
> > Reviewed By: davide
> >
> > Subscribers: grandinj, zhendongsu, llvm-commits, david2050
> >
> > Differential Revision: https://reviews.llvm.org/D35869
> >
> > Added:
> > llvm/trunk/test/Transforms/ADCE/2017-08-21-DomTree-deletions.ll
> > llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll
> > llvm/trunk/test/Transforms/ADCE/unreachable.ll
> > Modified:
> > llvm/trunk/lib/Transforms/Scalar/ADCE.cpp
> >
> > Modified: llvm/trunk/lib/Transforms/Scalar/ADCE.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> Transforms/Scalar/ADCE.cpp?rev=311467&r1=311466&r2=311467&view=diff
> > ============================================================
> ==================
> > --- llvm/trunk/lib/Transforms/Scalar/ADCE.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Scalar/ADCE.cpp Tue Aug 22 09:30:21 2017
> > @@ -27,6 +27,7 @@
> > #include "llvm/IR/BasicBlock.h"
> > #include "llvm/IR/CFG.h"
> > #include "llvm/IR/DebugInfoMetadata.h"
> > +#include "llvm/IR/Dominators.h"
> > #include "llvm/IR/IRBuilder.h"
> > #include "llvm/IR/InstIterator.h"
> > #include "llvm/IR/Instructions.h"
> > @@ -89,6 +90,10 @@ struct BlockInfoType {
> >
> > class AggressiveDeadCodeElimination {
> > Function &F;
> > +
> > + // ADCE does not use DominatorTree per se, but it updates it to
> preserve the
> > + // analysis.
> > + DominatorTree &DT;
> > PostDominatorTree &PDT;
> >
> > /// Mapping of blocks to associated information, an element in
> BlockInfoVec.
> > @@ -157,9 +162,10 @@ class AggressiveDeadCodeElimination {
> > void makeUnconditional(BasicBlock *BB, BasicBlock *Target);
> >
> > public:
> > - AggressiveDeadCodeElimination(Function &F, PostDominatorTree &PDT)
> > - : F(F), PDT(PDT) {}
> > - bool performDeadCodeElimination();
> > + AggressiveDeadCodeElimination(Function &F, DominatorTree &DT,
> > + PostDominatorTree &PDT)
> > + : F(F), DT(DT), PDT(PDT) {}
> > + bool performDeadCodeElimination();
> > };
> > }
> >
> > @@ -557,14 +563,34 @@ void AggressiveDeadCodeElimination::upda
> > }
> > assert((PreferredSucc && PreferredSucc->PostOrder > 0) &&
> > "Failed to find safe successor for dead branch");
> > +
> > + // Collect removed successors to update the (Post)DominatorTrees.
> > + SmallPtrSet<BasicBlock *, 4> RemovedSuccessors;
> > bool First = true;
> > for (auto *Succ : successors(BB)) {
> > - if (!First || Succ != PreferredSucc->BB)
> > + if (!First || Succ != PreferredSucc->BB) {
> > Succ->removePredecessor(BB);
> > - else
> > + RemovedSuccessors.insert(Succ);
> > + } else
> > First = false;
> > }
> > makeUnconditional(BB, PreferredSucc->BB);
> > +
> > + // Inform the dominators about the deleted CFG edges.
> > + SmallVector<DominatorTree::UpdateType, 4> DeletedEdges;
> > + for (auto *Succ : RemovedSuccessors) {
> > + // It might have happened that the same successor appeared
> multiple times
> > + // and the CFG edge wasn't really removed.
> > + if (Succ != PreferredSucc->BB) {
> > + DEBUG(dbgs() << "ADCE: (Post)DomTree edge enqueued for deletion"
> > + << BB->getName() << " -> " << Succ->getName() <<
> "\n");
> > + DeletedEdges.push_back({DominatorTree::Delete, BB, Succ});
> > + }
> > + }
> > +
> > + DT.applyUpdates(DeletedEdges);
> > + PDT.applyUpdates(DeletedEdges);
> > +
> > NumBranchesRemoved += 1;
> > }
> > }
> > @@ -609,6 +635,9 @@ void AggressiveDeadCodeElimination::make
> > InstInfo[NewTerm].Live = true;
> > if (const DILocation *DL = PredTerm->getDebugLoc())
> > NewTerm->setDebugLoc(DL);
> > +
> > + InstInfo.erase(PredTerm);
> > + PredTerm->eraseFromParent();
> > }
> >
> > //===-------------------------------------------------------
> ---------------===//
> > @@ -617,13 +646,16 @@ void AggressiveDeadCodeElimination::make
> > //
> > //===-------------------------------------------------------
> ---------------===//
> > PreservedAnalyses ADCEPass::run(Function &F, FunctionAnalysisManager
> &FAM) {
> > + auto &DT = FAM.getResult<DominatorTreeAnalysis>(F);
> > auto &PDT = FAM.getResult<PostDominatorTreeAnalysis>(F);
> > - if (!AggressiveDeadCodeElimination(F, PDT).
> performDeadCodeElimination())
> > + if (!AggressiveDeadCodeElimination(F, DT, PDT).
> performDeadCodeElimination())
> > return PreservedAnalyses::all();
> >
> > PreservedAnalyses PA;
> > PA.preserveSet<CFGAnalyses>();
> > PA.preserve<GlobalsAA>();
> > + PA.preserve<DominatorTreeAnalysis>();
> > + PA.preserve<PostDominatorTreeAnalysis>();
> > return PA;
> > }
> >
> > @@ -637,14 +669,23 @@ struct ADCELegacyPass : public FunctionP
> > bool runOnFunction(Function &F) override {
> > if (skipFunction(F))
> > return false;
> > +
> > + auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
> > auto &PDT = getAnalysis<PostDominatorTreeWrapperPass>(
> ).getPostDomTree();
> > - return AggressiveDeadCodeElimination(F, PDT).
> performDeadCodeElimination();
> > + return AggressiveDeadCodeElimination(F, DT, PDT)
> > + .performDeadCodeElimination();
> > }
> >
> > void getAnalysisUsage(AnalysisUsage &AU) const override {
> > + // We require DominatorTree here only to update and thus preserve
> it.
> > + AU.addRequired<DominatorTreeWrapperPass>();
> > AU.addRequired<PostDominatorTreeWrapperPass>();
> > if (!RemoveControlFlowFlag)
> > AU.setPreservesCFG();
> > + else {
> > + AU.addPreserved<DominatorTreeWrapperPass>();
> > + AU.addPreserved<PostDominatorTreeWrapperPass>();
> > + }
> > AU.addPreserved<GlobalsAAWrapperPass>();
> > }
> > };
> > @@ -653,6 +694,7 @@ struct ADCELegacyPass : public FunctionP
> > char ADCELegacyPass::ID = 0;
> > INITIALIZE_PASS_BEGIN(ADCELegacyPass, "adce",
> > "Aggressive Dead Code Elimination", false, false)
> > +INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
> > INITIALIZE_PASS_DEPENDENCY(PostDominatorTreeWrapperPass)
> > INITIALIZE_PASS_END(ADCELegacyPass, "adce", "Aggressive Dead Code
> Elimination",
> > false, false)
> >
> > Added: llvm/trunk/test/Transforms/ADCE/2017-08-21-DomTree-deletions.ll
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> Transforms/ADCE/2017-08-21-DomTree-deletions.ll?rev=311467&view=auto
> > ============================================================
> ==================
> > --- llvm/trunk/test/Transforms/ADCE/2017-08-21-DomTree-deletions.ll
> (added)
> > +++ llvm/trunk/test/Transforms/ADCE/2017-08-21-DomTree-deletions.ll Tue
> Aug 22 09:30:21 2017
> > @@ -0,0 +1,24 @@
> > +; RUN: opt < %s -adce | llvm-dis
> > +; RUN: opt < %s -adce -verify-dom-info | llvm-dis
> > +
> > +define void @foo() {
> > +entry:
> > + br label %switch
> > +switch: ; preds = %entry
> > + switch i32 undef, label %default [
> > + i32 2, label %two
> > + i32 5, label %five
> > + i32 4, label %four
> > + ]
> > +four: ; preds = %switch
> > + br label %exit
> > +five: ; preds = %switch
> > + br label %exit
> > +two: ; preds = %switch
> > + br label %exit
> > +default: ; preds = %switch
> > + br label %exit
> > +exit: ; preds = %default, %two, %five, %four
> > + ret void
> > +}
> > +
> >
> > Added: llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> Transforms/ADCE/domtree-DoubleDeletion.ll?rev=311467&view=auto
> > ============================================================
> ==================
> > --- llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll (added)
> > +++ llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll Tue Aug
> 22 09:30:21 2017
> > @@ -0,0 +1,39 @@
> > +; RUN: opt < %s -gvn -simplifycfg -adce | llvm-dis
> > +; RUN: opt < %s -gvn -simplifycfg -adce -verify-dom-info | llvm-dis
> > +
> > +; This test makes sure that the DominatorTree properly handles
> > +; deletion of edges that go to forward-unreachable regions.
> > +; In this case, %land.end is already forward unreachable when
> > +; the DT gets informed about the deletion of %entry -> %land.end.
> > +
> > + at a = common global i32 0, align 4
> > +
> > +define i32 @main() {
> > +entry:
> > + %retval = alloca i32, align 4
> > + store i32 0, i32* %retval, align 4
> > + %0 = load i32, i32* @a, align 4
> > + %cmp = icmp ne i32 %0, 1
> > + br i1 %cmp, label %land.rhs, label %land.end4
> > +
> > +land.rhs: ; preds = %entry
> > + %1 = load i32, i32* @a, align 4
> > + %tobool = icmp ne i32 %1, 0
> > + br i1 %tobool, label %land.rhs1, label %land.end
> > +
> > +land.rhs1: ; preds = %land.rhs
> > + br label %land.end
> > +
> > +land.end: ; preds = %land.rhs1,
> %land.rhs
> > + %2 = phi i1 [ false, %land.rhs ], [ true, %land.rhs1 ]
> > + %land.ext = zext i1 %2 to i32
> > + %conv = trunc i32 %land.ext to i16
> > + %conv2 = sext i16 %conv to i32
> > + %tobool3 = icmp ne i32 %conv2, 0
> > + br label %land.end4
> > +
> > +land.end4: ; preds = %land.end,
> %entry
> > + %3 = phi i1 [ false, %entry ], [ %tobool3, %land.end ]
> > + %land.ext5 = zext i1 %3 to i32
> > + ret i32 0
> > +}
> >
> > Added: llvm/trunk/test/Transforms/ADCE/unreachable.ll
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> Transforms/ADCE/unreachable.ll?rev=311467&view=auto
> > ============================================================
> ==================
> > --- llvm/trunk/test/Transforms/ADCE/unreachable.ll (added)
> > +++ llvm/trunk/test/Transforms/ADCE/unreachable.ll Tue Aug 22 09:30:21
> 2017
> > @@ -0,0 +1,18 @@
> > +; RUN: opt < %s -adce -simplifycfg | llvm-dis
> > +; RUN: opt < %s -passes=adce | llvm-dis
> > +
> > +define i32 @Test(i32 %A, i32 %B) {
> > +BB1:
> > + br label %BB4
> > +
> > +BB2: ; No predecessors!
> > + br label %BB3
> > +
> > +BB3: ; preds = %BB4, %BB2
> > + %ret = phi i32 [ %X, %BB4 ], [ %B, %BB2 ] ; <i32>
> [#uses=1]
> > + ret i32 %ret
> > +
> > +BB4: ; preds = %BB1
> > + %X = phi i32 [ %A, %BB1 ] ; <i32> [#uses=1]
> > + br label %BB3
> > +}
> >
> >
> > _______________________________________________
> > 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/20170823/27d5c523/attachment.html>
More information about the llvm-commits
mailing list