<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi,<div class=""><br class=""></div><div class="">I'm currently seeing a crash in ADCE which I've narrowed down to this commit. Given this bugpoint-reduced test case:</div><div class=""><font face="Courier New" class="">define void @foo() {<br class="">entry:<br class=""> br label %switch<br class="">switch: ; preds = %entry<br class=""> switch i32 undef, label %default [<br class=""> i32 2, label %two<br class=""> i32 5, label %five<br class=""> i32 4, label %four<br class=""> ]<br class="">four: ; preds = %switch<br class=""> br label %exit<br class="">five: ; preds = %switch<br class=""> br label %exit<br class="">two: ; preds = %switch<br class=""> br label %exit<br class="">default: ; preds = %switch<br class=""> br label %exit<br class="">exit: ; preds = %default, %two, %five, %four<br class=""> ret void<br class="">}</font></div><div class="">the command '<font face="Courier New" class="">./bin/opt bugpoint-reduced-simplified.ll -adce -S -o -</font>' asserts with:</div><div class=""><font face="Courier New" class="">Assertion failed: (TN), function operator(), file ../llvm/include/llvm/Support/GenericDomTreeConstruction.h, line 1031.<br class="">Stack dump:<br class="">0.<span class="Apple-tab-span" style="white-space:pre"> </span>Program arguments: ./bin/opt bugpoint-reduced-simplified.ll -adce -S -o - <br class="">1.<span class="Apple-tab-span" style="white-space:pre"> </span>Running pass 'Function Pass Manager' on module 'bugpoint-reduced-simplified.ll'.<br class="">2.<span class="Apple-tab-span" style="white-space:pre"> </span>Running pass 'Aggressive Dead Code Elimination' on function '@foo'<br class="">Abort trap: 6</font><br class=""><br class=""></div><div class="">According to the -debug output and some additional instrumentation, ADCE starts by re-writing the switch to a 'br label %four'. I was expecting it to choose %default but all choices are equivalent so it doesn't matter much. At this point it deletes the %switch->%default CFG edge, erases %default, and updates the dominator tree to indicate that %four dominates %exit. It then deletes the %switch->%two CFG edge and it's at this point that things start to go wrong.</div><div class=""><br class=""></div><div class="">After deleting the %switch->%two edge it erases %two but it also erases %exit which is still being used by %four (as well as %default, %two, and %five). The dominator tree is correctly updated to reflect the removal of both %two and %exit. It then proceeds to delete the %switch->%five edge and hits the assertion when it attempts to look up a TreeNodePtr for %exit and doesn't find one.</div><div class=""><br class=""></div><div class="">I'm currently thinking that this is a processing order issue. Would I be right in thinking that TreeNodePtr::getLevel() returns the depth in the dominator tree? If so, I think that the update to indicate that %four dominates %exit when the %switch->%default path is deleted is causing %exit to be misidentified as unreachable in the later updates because DescendAndCollect() is only checking the depth of the successors it finds. If %exit had been updated after all the removals then it would have turned out ok.</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On 17 Aug 2017, at 02:41, Jakub Kuderski via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Author: kuhar<br class="">Date: Wed Aug 16 18:41:49 2017<br class="">New Revision: 311057<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=311057&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=311057&view=rev</a><br class="">Log:<br class="">Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators<br class=""><br class="">Summary:<br class="">This patch teaches ADCE to preserve both DominatorTrees and PostDominatorTrees.<br class=""><br class="">I didn't notice any performance impact when bootstrapping clang with this patch.<br class=""><br class="">The patch was originally committed in r311039 and reverted in r311049.<br class="">This revision fixes the problem with not adding a dependency on the<br class="">DominatorTreeWrapperPass for the LegacyPassManager.<br class=""><br class="">Reviewers: dberlin, chandlerc, sanjoy, davide, grosser, brzycki<br class=""><br class="">Reviewed By: davide<br class=""><br class="">Subscribers: grandinj, zhendongsu, llvm-commits, david2050<br class=""><br class="">Differential Revision: <a href="https://reviews.llvm.org/D35869" class="">https://reviews.llvm.org/D35869</a><br class=""><br class="">Added:<br class=""> llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll<br class=""> llvm/trunk/test/Transforms/ADCE/unreachable.ll<br class="">Modified:<br class=""> llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h<br class=""> llvm/trunk/lib/Transforms/Scalar/ADCE.cpp<br class=""><br class="">Modified: llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h?rev=311057&r1=311056&r2=311057&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h?rev=311057&r1=311056&r2=311057&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h (original)<br class="">+++ llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h Wed Aug 16 18:41:49 2017<br class="">@@ -914,7 +914,12 @@ struct SemiNCAInfo {<br class=""> if (!FromTN) return;<br class=""><br class=""> const TreeNodePtr ToTN = DT.getNode(To);<br class="">- assert(ToTN && "To already unreachable -- there is no edge to delete");<br class="">+ if (!ToTN) {<br class="">+ DEBUG(dbgs() << "\tTo (" << BlockNamePrinter(To)<br class="">+ << ") already unreachable -- there is no edge to delete\n");<br class="">+ return;<br class="">+ }<br class="">+<br class=""> const NodePtr NCDBlock = DT.findNearestCommonDominator(From, To);<br class=""> const TreeNodePtr NCD = DT.getNode(NCDBlock);<br class=""><br class=""><br class="">Modified: llvm/trunk/lib/Transforms/Scalar/ADCE.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ADCE.cpp?rev=311057&r1=311056&r2=311057&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ADCE.cpp?rev=311057&r1=311056&r2=311057&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/Transforms/Scalar/ADCE.cpp (original)<br class="">+++ llvm/trunk/lib/Transforms/Scalar/ADCE.cpp Wed Aug 16 18:41:49 2017<br class="">@@ -27,6 +27,7 @@<br class=""> #include "llvm/IR/BasicBlock.h"<br class=""> #include "llvm/IR/CFG.h"<br class=""> #include "llvm/IR/DebugInfoMetadata.h"<br class="">+#include "llvm/IR/Dominators.h"<br class=""> #include "llvm/IR/IRBuilder.h"<br class=""> #include "llvm/IR/InstIterator.h"<br class=""> #include "llvm/IR/Instructions.h"<br class="">@@ -89,6 +90,10 @@ struct BlockInfoType {<br class=""><br class=""> class AggressiveDeadCodeElimination {<br class=""> Function &F;<br class="">+<br class="">+ // ADCE does not use DominatorTree per se, but it updates it to preserve the<br class="">+ // analysis.<br class="">+ DominatorTree &DT;<br class=""> PostDominatorTree &PDT;<br class=""><br class=""> /// Mapping of blocks to associated information, an element in BlockInfoVec.<br class="">@@ -157,9 +162,10 @@ class AggressiveDeadCodeElimination {<br class=""> void makeUnconditional(BasicBlock *BB, BasicBlock *Target);<br class=""><br class=""> public:<br class="">- AggressiveDeadCodeElimination(Function &F, PostDominatorTree &PDT)<br class="">- : F(F), PDT(PDT) {}<br class="">- bool performDeadCodeElimination();<br class="">+ AggressiveDeadCodeElimination(Function &F, DominatorTree &DT,<br class="">+ PostDominatorTree &PDT)<br class="">+ : F(F), DT(DT), PDT(PDT) {}<br class="">+ bool performDeadCodeElimination();<br class=""> };<br class=""> }<br class=""><br class="">@@ -557,14 +563,31 @@ void AggressiveDeadCodeElimination::upda<br class=""> }<br class=""> assert((PreferredSucc && PreferredSucc->PostOrder > 0) &&<br class=""> "Failed to find safe successor for dead branch");<br class="">+<br class="">+ // Collect removed successors to update the (Post)DominatorTrees.<br class="">+ SmallPtrSet<BasicBlock *, 4> RemovedSuccessors;<br class=""> bool First = true;<br class=""> for (auto *Succ : successors(BB)) {<br class="">- if (!First || Succ != PreferredSucc->BB)<br class="">+ if (!First || Succ != PreferredSucc->BB) {<br class=""> Succ->removePredecessor(BB);<br class="">- else<br class="">+ RemovedSuccessors.insert(Succ);<br class="">+ } else<br class=""> First = false;<br class=""> }<br class=""> makeUnconditional(BB, PreferredSucc->BB);<br class="">+<br class="">+ // Inform the dominators about the deleted CFG edges.<br class="">+ for (auto *Succ : RemovedSuccessors) {<br class="">+ // It might have happened that the same successor appeared multiple times<br class="">+ // and the CFG edge wasn't really removed.<br class="">+ if (Succ != PreferredSucc->BB) {<br class="">+ DEBUG(dbgs() << "ADCE: Removing (Post)DomTree edge " << BB->getName()<br class="">+ << " -> " << Succ->getName() << "\n");<br class="">+ DT.deleteEdge(BB, Succ);<br class="">+ PDT.deleteEdge(BB, Succ);<br class="">+ }<br class="">+ }<br class="">+<br class=""> NumBranchesRemoved += 1;<br class=""> }<br class=""> }<br class="">@@ -609,6 +632,9 @@ void AggressiveDeadCodeElimination::make<br class=""> InstInfo[NewTerm].Live = true;<br class=""> if (const DILocation *DL = PredTerm->getDebugLoc())<br class=""> NewTerm->setDebugLoc(DL);<br class="">+<br class="">+ InstInfo.erase(PredTerm);<br class="">+ PredTerm->eraseFromParent();<br class=""> }<br class=""><br class=""> //===----------------------------------------------------------------------===//<br class="">@@ -617,13 +643,16 @@ void AggressiveDeadCodeElimination::make<br class=""> //<br class=""> //===----------------------------------------------------------------------===//<br class=""> PreservedAnalyses ADCEPass::run(Function &F, FunctionAnalysisManager &FAM) {<br class="">+ auto &DT = FAM.getResult<DominatorTreeAnalysis>(F);<br class=""> auto &PDT = FAM.getResult<PostDominatorTreeAnalysis>(F);<br class="">- if (!AggressiveDeadCodeElimination(F, PDT).performDeadCodeElimination())<br class="">+ if (!AggressiveDeadCodeElimination(F, DT, PDT).performDeadCodeElimination())<br class=""> return PreservedAnalyses::all();<br class=""><br class=""> PreservedAnalyses PA;<br class=""> PA.preserveSet<CFGAnalyses>();<br class=""> PA.preserve<GlobalsAA>();<br class="">+ PA.preserve<DominatorTreeAnalysis>();<br class="">+ PA.preserve<PostDominatorTreeAnalysis>();<br class=""> return PA;<br class=""> }<br class=""><br class="">@@ -637,14 +666,23 @@ struct ADCELegacyPass : public FunctionP<br class=""> bool runOnFunction(Function &F) override {<br class=""> if (skipFunction(F))<br class=""> return false;<br class="">+<br class="">+ auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();<br class=""> auto &PDT = getAnalysis<PostDominatorTreeWrapperPass>().getPostDomTree();<br class="">- return AggressiveDeadCodeElimination(F, PDT).performDeadCodeElimination();<br class="">+ return AggressiveDeadCodeElimination(F, DT, PDT)<br class="">+ .performDeadCodeElimination();<br class=""> }<br class=""><br class=""> void getAnalysisUsage(AnalysisUsage &AU) const override {<br class="">+ // We require DominatorTree here only to update and thus preserve it.<br class="">+ AU.addRequired<DominatorTreeWrapperPass>();<br class=""> AU.addRequired<PostDominatorTreeWrapperPass>();<br class=""> if (!RemoveControlFlowFlag)<br class=""> AU.setPreservesCFG();<br class="">+ else {<br class="">+ AU.addPreserved<DominatorTreeWrapperPass>();<br class="">+ AU.addPreserved<PostDominatorTreeWrapperPass>();<br class="">+ }<br class=""> AU.addPreserved<GlobalsAAWrapperPass>();<br class=""> }<br class=""> };<br class="">@@ -653,6 +691,7 @@ struct ADCELegacyPass : public FunctionP<br class=""> char ADCELegacyPass::ID = 0;<br class=""> INITIALIZE_PASS_BEGIN(ADCELegacyPass, "adce",<br class=""> "Aggressive Dead Code Elimination", false, false)<br class="">+INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)<br class=""> INITIALIZE_PASS_DEPENDENCY(PostDominatorTreeWrapperPass)<br class=""> INITIALIZE_PASS_END(ADCELegacyPass, "adce", "Aggressive Dead Code Elimination",<br class=""> false, false)<br class=""><br class="">Added: llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll?rev=311057&view=auto" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll?rev=311057&view=auto</a><br class="">==============================================================================<br class="">--- llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll (added)<br class="">+++ llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll Wed Aug 16 18:41:49 2017<br class="">@@ -0,0 +1,39 @@<br class="">+; RUN: opt < %s -gvn -simplifycfg -adce | llvm-dis<br class="">+; RUN: opt < %s -gvn -simplifycfg -adce -verify-dom-info | llvm-dis<br class="">+<br class="">+; This test makes sure that the DominatorTree properly handles<br class="">+; deletion of edges that go to forward-unreachable regions.<br class="">+; In this case, %land.end is already forward unreachable when<br class="">+; the DT gets informed about the deletion of %entry -> %land.end.<br class="">+<br class="">+@a = common global i32 0, align 4<br class="">+<br class="">+define i32 @main() {<br class="">+entry:<br class="">+ %retval = alloca i32, align 4<br class="">+ store i32 0, i32* %retval, align 4<br class="">+ %0 = load i32, i32* @a, align 4<br class="">+ %cmp = icmp ne i32 %0, 1<br class="">+ br i1 %cmp, label %land.rhs, label %land.end4<br class="">+<br class="">+land.rhs: ; preds = %entry<br class="">+ %1 = load i32, i32* @a, align 4<br class="">+ %tobool = icmp ne i32 %1, 0<br class="">+ br i1 %tobool, label %land.rhs1, label %land.end<br class="">+<br class="">+land.rhs1: ; preds = %land.rhs<br class="">+ br label %land.end<br class="">+<br class="">+land.end: ; preds = %land.rhs1, %land.rhs<br class="">+ %2 = phi i1 [ false, %land.rhs ], [ true, %land.rhs1 ]<br class="">+ %land.ext = zext i1 %2 to i32<br class="">+ %conv = trunc i32 %land.ext to i16<br class="">+ %conv2 = sext i16 %conv to i32<br class="">+ %tobool3 = icmp ne i32 %conv2, 0<br class="">+ br label %land.end4<br class="">+<br class="">+land.end4: ; preds = %land.end, %entry<br class="">+ %3 = phi i1 [ false, %entry ], [ %tobool3, %land.end ]<br class="">+ %land.ext5 = zext i1 %3 to i32<br class="">+ ret i32 0<br class="">+}<br class=""><br class="">Added: llvm/trunk/test/Transforms/ADCE/unreachable.ll<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ADCE/unreachable.ll?rev=311057&view=auto" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ADCE/unreachable.ll?rev=311057&view=auto</a><br class="">==============================================================================<br class="">--- llvm/trunk/test/Transforms/ADCE/unreachable.ll (added)<br class="">+++ llvm/trunk/test/Transforms/ADCE/unreachable.ll Wed Aug 16 18:41:49 2017<br class="">@@ -0,0 +1,18 @@<br class="">+; RUN: opt < %s -adce -simplifycfg | llvm-dis<br class="">+; RUN: opt < %s -passes=adce | llvm-dis<br class="">+<br class="">+define i32 @Test(i32 %A, i32 %B) {<br class="">+BB1:<br class="">+ br label %BB4<br class="">+<br class="">+BB2: ; No predecessors!<br class="">+ br label %BB3<br class="">+<br class="">+BB3: ; preds = %BB4, %BB2<br class="">+ %ret = phi i32 [ %X, %BB4 ], [ %B, %BB2 ] ; <i32> [#uses=1]<br class="">+ ret i32 %ret<br class="">+<br class="">+BB4: ; preds = %BB1<br class="">+ %X = phi i32 [ %A, %BB1 ] ; <i32> [#uses=1]<br class="">+ br label %BB3<br class="">+}<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></div></div></blockquote></div><br class=""></div></body></html>