[llvm] r311039 - [ADCE][Dominators] Teach ADCE to preserve dominators
Jakub (Kuba) Kuderski via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 16 15:07:19 PDT 2017
This patch broke ocaml bindings:
http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/6054.
I don't have ocaml environment set up, so I'm going to revert it and
investigate the failure.
On Wed, Aug 16, 2017 at 1:50 PM, Jakub Kuderski via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: kuhar
> Date: Wed Aug 16 13:50:23 2017
> New Revision: 311039
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311039&view=rev
> Log:
> [ADCE][Dominators] Teach ADCE to preserve dominators
>
> Summary:
> This patch teaches ADCE to preserve both DominatorTrees and
> PostDominatorTrees.
>
> I didn't notice any performance impact when bootstrapping clang with this
> patch.
>
> 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/domtree-DoubleDeletion.ll
> llvm/trunk/test/Transforms/ADCE/unreachable.ll
> Modified:
> llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
> llvm/trunk/lib/Transforms/Scalar/ADCE.cpp
>
> Modified: llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/
> GenericDomTreeConstruction.h?rev=311039&r1=311038&r2=311039&view=diff
> ============================================================
> ==================
> --- llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
> (original)
> +++ llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h Wed Aug
> 16 13:50:23 2017
> @@ -914,7 +914,12 @@ struct SemiNCAInfo {
> if (!FromTN) return;
>
> const TreeNodePtr ToTN = DT.getNode(To);
> - assert(ToTN && "To already unreachable -- there is no edge to
> delete");
> + if (!ToTN) {
> + DEBUG(dbgs() << "\tTo (" << BlockNamePrinter(To)
> + << ") already unreachable -- there is no edge to
> delete\n");
> + return;
> + }
> +
> const NodePtr NCDBlock = DT.findNearestCommonDominator(From, To);
> const TreeNodePtr NCD = DT.getNode(NCDBlock);
>
>
> Modified: llvm/trunk/lib/Transforms/Scalar/ADCE.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> Transforms/Scalar/ADCE.cpp?rev=311039&r1=311038&r2=311039&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Transforms/Scalar/ADCE.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/ADCE.cpp Wed Aug 16 13:50:23 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,31 @@ 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.
> + 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: Removing (Post)DomTree edge " <<
> BB->getName()
> + << " -> " << Succ->getName() << "\n");
> + DT.deleteEdge(BB, Succ);
> + PDT.deleteEdge(BB, Succ);
> + }
> + }
> +
> NumBranchesRemoved += 1;
> }
> }
> @@ -609,6 +632,9 @@ void AggressiveDeadCodeElimination::make
> InstInfo[NewTerm].Live = true;
> if (const DILocation *DL = PredTerm->getDebugLoc())
> NewTerm->setDebugLoc(DL);
> +
> + InstInfo.erase(PredTerm);
> + PredTerm->eraseFromParent();
> }
>
> //===-------------------------------------------------------
> ---------------===//
> @@ -617,13 +643,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,15 +666,22 @@ 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();
> AU.addPreserved<GlobalsAAWrapperPass>();
> + AU.addPreserved<DominatorTreeWrapperPass>();
> + AU.addPreserved<PostDominatorTreeWrapperPass>();
> }
> };
> }
>
> 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=311039&view=auto
> ============================================================
> ==================
> --- llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll (added)
> +++ llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll Wed Aug 16
> 13:50:23 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=311039&view=auto
> ============================================================
> ==================
> --- llvm/trunk/test/Transforms/ADCE/unreachable.ll (added)
> +++ llvm/trunk/test/Transforms/ADCE/unreachable.ll Wed Aug 16 13:50:23
> 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
>
--
Jakub Kuderski
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170816/a367668e/attachment.html>
More information about the llvm-commits
mailing list