[llvm] r311381 - Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators"

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 21 13:39:18 PDT 2017


Author: sanjoy
Date: Mon Aug 21 13:39:18 2017
New Revision: 311381

URL: http://llvm.org/viewvc/llvm-project?rev=311381&view=rev
Log:
Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators"

Summary: This partially reverts commit r311057 since it breaks ADCE.  See PR34258.

Reviewers: kuhar

Subscribers: mcrosier, david2050, llvm-commits

Differential Revision: https://reviews.llvm.org/D36979

Removed:
    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=311381&r1=311380&r2=311381&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/ADCE.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/ADCE.cpp Mon Aug 21 13:39:18 2017
@@ -27,7 +27,6 @@
 #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"
@@ -90,10 +89,6 @@ 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.
@@ -162,10 +157,9 @@ class AggressiveDeadCodeElimination {
   void makeUnconditional(BasicBlock *BB, BasicBlock *Target);
 
 public:
- AggressiveDeadCodeElimination(Function &F, DominatorTree &DT,
-                               PostDominatorTree &PDT)
-     : F(F), DT(DT), PDT(PDT) {}
- bool performDeadCodeElimination();
+  AggressiveDeadCodeElimination(Function &F, PostDominatorTree &PDT)
+      : F(F), PDT(PDT) {}
+  bool performDeadCodeElimination();
 };
 }
 
@@ -563,31 +557,14 @@ 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);
-        RemovedSuccessors.insert(Succ);
-      } else
+      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;
   }
 }
@@ -632,9 +609,6 @@ void AggressiveDeadCodeElimination::make
   InstInfo[NewTerm].Live = true;
   if (const DILocation *DL = PredTerm->getDebugLoc())
     NewTerm->setDebugLoc(DL);
-
-  InstInfo.erase(PredTerm);
-  PredTerm->eraseFromParent();
 }
 
 //===----------------------------------------------------------------------===//
@@ -643,16 +617,13 @@ 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, DT, PDT).performDeadCodeElimination())
+  if (!AggressiveDeadCodeElimination(F, PDT).performDeadCodeElimination())
     return PreservedAnalyses::all();
 
   PreservedAnalyses PA;
   PA.preserveSet<CFGAnalyses>();
   PA.preserve<GlobalsAA>();
-  PA.preserve<DominatorTreeAnalysis>();
-  PA.preserve<PostDominatorTreeAnalysis>();
   return PA;
 }
 
@@ -666,23 +637,14 @@ 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, DT, PDT)
-        .performDeadCodeElimination();
+    return AggressiveDeadCodeElimination(F, 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>();
   }
 };
@@ -691,7 +653,6 @@ 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)

Removed: llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll?rev=311380&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll (original)
+++ llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll (removed)
@@ -1,39 +0,0 @@
-; 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
-}

Removed: llvm/trunk/test/Transforms/ADCE/unreachable.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ADCE/unreachable.ll?rev=311380&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/ADCE/unreachable.ll (original)
+++ llvm/trunk/test/Transforms/ADCE/unreachable.ll (removed)
@@ -1,18 +0,0 @@
-; 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
-}




More information about the llvm-commits mailing list