[llvm] r311039 - [ADCE][Dominators] Teach ADCE to preserve dominators
Jakub (Kuba) Kuderski via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 16 18:45:35 PDT 2017
The problem was that ADCE didn't use the macro INITIALIZE_PASS_DEPENDENCY to
specify a dependency on the DominatorTreeWrapperPass. It failed in the
OCaml binding test, as it scheduled ADCE to run as the very first pass.
I fixed that and recommitted in r311057.
On Wed, Aug 16, 2017 at 3:13 PM, Jakub (Kuba) Kuderski <
kubakuderski at gmail.com> wrote:
> Reverted in r311049.
>
> On Wed, Aug 16, 2017 at 3:07 PM, Jakub (Kuba) Kuderski <
> kubakuderski at gmail.com> wrote:
>
>> 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/Transform
>>> s/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).performDeadCodeEliminatio
>>> n())
>>> + 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<PostDominatorTreeW
>>> rapperPass>().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/Transfor
>>> ms/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/Transfor
>>> ms/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
>>
>
>
>
> --
> Jakub Kuderski
>
--
Jakub Kuderski
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170816/c14236ad/attachment.html>
More information about the llvm-commits
mailing list