[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:13:16 PDT 2017
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170816/75569ded/attachment-0001.html>
More information about the llvm-commits
mailing list