<div dir="ltr">The problem was that ADCE didn't use the macro <span style="font-family:"DejaVu Sans Mono";font-size:9pt;background-color:rgb(255,255,255)"><font color="#000000">INITIALIZE_PASS_DEPENDENCY </font></span>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.<br><br>I fixed that and recommitted in r311057<span style="font-size:17.6px">.</span></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Aug 16, 2017 at 3:13 PM, Jakub (Kuba) Kuderski <span dir="ltr"><<a href="mailto:kubakuderski@gmail.com" target="_blank">kubakuderski@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Reverted in r311049<span style="font-size:17.6px">.</span></div><div class="gmail_extra"><div><div class="h5"><br><div class="gmail_quote">On Wed, Aug 16, 2017 at 3:07 PM, Jakub (Kuba) Kuderski <span dir="ltr"><<a href="mailto:kubakuderski@gmail.com" target="_blank">kubakuderski@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">This patch broke ocaml bindings:<br><a href="http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/6054" target="_blank">http://lab.llvm.org:8011/build<wbr>ers/clang-x86_64-debian-fast/<wbr>builds/6054</a>.<br><br>I don't have ocaml environment set up, so I'm going to revert it and investigate the failure.<br><br><br></div><div class="gmail_extra"><div><div class="m_7060354758183325962h5"><br><div class="gmail_quote">On Wed, Aug 16, 2017 at 1:50 PM, Jakub Kuderski via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: kuhar<br>
Date: Wed Aug 16 13:50:23 2017<br>
New Revision: 311039<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=311039&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=311039&view=rev</a><br>
Log:<br>
[ADCE][Dominators] Teach ADCE to preserve dominators<br>
<br>
Summary:<br>
This patch teaches ADCE to preserve both DominatorTrees and PostDominatorTrees.<br>
<br>
I didn't notice any performance impact when bootstrapping clang with this patch.<br>
<br>
Reviewers: dberlin, chandlerc, sanjoy, davide, grosser, brzycki<br>
<br>
Reviewed By: davide<br>
<br>
Subscribers: grandinj, zhendongsu, llvm-commits, david2050<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D35869" rel="noreferrer" target="_blank">https://reviews.llvm.org/D3586<wbr>9</a><br>
<br>
Added:<br>
llvm/trunk/test/Transforms/ADC<wbr>E/domtree-DoubleDeletion.ll<br>
llvm/trunk/test/Transforms/ADC<wbr>E/unreachable.ll<br>
Modified:<br>
llvm/trunk/include/llvm/Suppor<wbr>t/GenericDomTreeConstruction.h<br>
llvm/trunk/lib/Transforms/Scal<wbr>ar/ADCE.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/Suppor<wbr>t/GenericDomTreeConstruction.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h?rev=311039&r1=311038&r2=311039&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/include/llvm/<wbr>Support/GenericDomTreeConstruc<wbr>tion.h?rev=311039&r1=311038&r2<wbr>=311039&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/include/llvm/Suppor<wbr>t/GenericDomTreeConstruction.h (original)<br>
+++ llvm/trunk/include/llvm/Suppor<wbr>t/GenericDomTreeConstruction.h Wed Aug 16 13:50:23 2017<br>
@@ -914,7 +914,12 @@ struct SemiNCAInfo {<br>
if (!FromTN) return;<br>
<br>
const TreeNodePtr ToTN = DT.getNode(To);<br>
- assert(ToTN && "To already unreachable -- there is no edge to delete");<br>
+ if (!ToTN) {<br>
+ DEBUG(dbgs() << "\tTo (" << BlockNamePrinter(To)<br>
+ << ") already unreachable -- there is no edge to delete\n");<br>
+ return;<br>
+ }<br>
+<br>
const NodePtr NCDBlock = DT.findNearestCommonDominator(<wbr>From, To);<br>
const TreeNodePtr NCD = DT.getNode(NCDBlock);<br>
<br>
<br>
Modified: llvm/trunk/lib/Transforms/Scal<wbr>ar/ADCE.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ADCE.cpp?rev=311039&r1=311038&r2=311039&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Transform<wbr>s/Scalar/ADCE.cpp?rev=311039&r<wbr>1=311038&r2=311039&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Transforms/Scal<wbr>ar/ADCE.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Scal<wbr>ar/ADCE.cpp Wed Aug 16 13:50:23 2017<br>
@@ -27,6 +27,7 @@<br>
#include "llvm/IR/BasicBlock.h"<br>
#include "llvm/IR/CFG.h"<br>
#include "llvm/IR/DebugInfoMetadata.h"<br>
+#include "llvm/IR/Dominators.h"<br>
#include "llvm/IR/IRBuilder.h"<br>
#include "llvm/IR/InstIterator.h"<br>
#include "llvm/IR/Instructions.h"<br>
@@ -89,6 +90,10 @@ struct BlockInfoType {<br>
<br>
class AggressiveDeadCodeElimination {<br>
Function &F;<br>
+<br>
+ // ADCE does not use DominatorTree per se, but it updates it to preserve the<br>
+ // analysis.<br>
+ DominatorTree &DT;<br>
PostDominatorTree &PDT;<br>
<br>
/// Mapping of blocks to associated information, an element in BlockInfoVec.<br>
@@ -157,9 +162,10 @@ class AggressiveDeadCodeElimination {<br>
void makeUnconditional(BasicBlock *BB, BasicBlock *Target);<br>
<br>
public:<br>
- AggressiveDeadCodeElimination(<wbr>Function &F, PostDominatorTree &PDT)<br>
- : F(F), PDT(PDT) {}<br>
- bool performDeadCodeElimination();<br>
+ AggressiveDeadCodeElimination(<wbr>Function &F, DominatorTree &DT,<br>
+ PostDominatorTree &PDT)<br>
+ : F(F), DT(DT), PDT(PDT) {}<br>
+ bool performDeadCodeElimination();<br>
};<br>
}<br>
<br>
@@ -557,14 +563,31 @@ void AggressiveDeadCodeElimination:<wbr>:upda<br>
}<br>
assert((PreferredSucc && PreferredSucc->PostOrder > 0) &&<br>
"Failed to find safe successor for dead branch");<br>
+<br>
+ // Collect removed successors to update the (Post)DominatorTrees.<br>
+ SmallPtrSet<BasicBlock *, 4> RemovedSuccessors;<br>
bool First = true;<br>
for (auto *Succ : successors(BB)) {<br>
- if (!First || Succ != PreferredSucc->BB)<br>
+ if (!First || Succ != PreferredSucc->BB) {<br>
Succ->removePredecessor(BB);<br>
- else<br>
+ RemovedSuccessors.insert(Succ)<wbr>;<br>
+ } else<br>
First = false;<br>
}<br>
makeUnconditional(BB, PreferredSucc->BB);<br>
+<br>
+ // Inform the dominators about the deleted CFG edges.<br>
+ for (auto *Succ : RemovedSuccessors) {<br>
+ // It might have happened that the same successor appeared multiple times<br>
+ // and the CFG edge wasn't really removed.<br>
+ if (Succ != PreferredSucc->BB) {<br>
+ DEBUG(dbgs() << "ADCE: Removing (Post)DomTree edge " << BB->getName()<br>
+ << " -> " << Succ->getName() << "\n");<br>
+ DT.deleteEdge(BB, Succ);<br>
+ PDT.deleteEdge(BB, Succ);<br>
+ }<br>
+ }<br>
+<br>
NumBranchesRemoved += 1;<br>
}<br>
}<br>
@@ -609,6 +632,9 @@ void AggressiveDeadCodeElimination:<wbr>:make<br>
InstInfo[NewTerm].Live = true;<br>
if (const DILocation *DL = PredTerm->getDebugLoc())<br>
NewTerm->setDebugLoc(DL);<br>
+<br>
+ InstInfo.erase(PredTerm);<br>
+ PredTerm->eraseFromParent();<br>
}<br>
<br>
//===------------------------<wbr>------------------------------<wbr>----------------===//<br>
@@ -617,13 +643,16 @@ void AggressiveDeadCodeElimination:<wbr>:make<br>
//<br>
//===------------------------<wbr>------------------------------<wbr>----------------===//<br>
PreservedAnalyses ADCEPass::run(Function &F, FunctionAnalysisManager &FAM) {<br>
+ auto &DT = FAM.getResult<DominatorTreeAna<wbr>lysis>(F);<br>
auto &PDT = FAM.getResult<PostDominatorTre<wbr>eAnalysis>(F);<br>
- if (!AggressiveDeadCodeEliminatio<wbr>n(F, PDT).performDeadCodeEliminatio<wbr>n())<br>
+ if (!AggressiveDeadCodeEliminatio<wbr>n(F, DT, PDT).performDeadCodeEliminatio<wbr>n())<br>
return PreservedAnalyses::all();<br>
<br>
PreservedAnalyses PA;<br>
PA.preserveSet<CFGAnalyses>()<wbr>;<br>
PA.preserve<GlobalsAA>();<br>
+ PA.preserve<DominatorTreeAnaly<wbr>sis>();<br>
+ PA.preserve<PostDominatorTreeA<wbr>nalysis>();<br>
return PA;<br>
}<br>
<br>
@@ -637,15 +666,22 @@ struct ADCELegacyPass : public FunctionP<br>
bool runOnFunction(Function &F) override {<br>
if (skipFunction(F))<br>
return false;<br>
+<br>
+ auto &DT = getAnalysis<DominatorTreeWrapp<wbr>erPass>().getDomTree();<br>
auto &PDT = getAnalysis<PostDominatorTreeW<wbr>rapperPass>().getPostDomTree()<wbr>;<br>
- return AggressiveDeadCodeElimination(<wbr>F, PDT).performDeadCodeEliminatio<wbr>n();<br>
+ return AggressiveDeadCodeElimination(<wbr>F, DT, PDT)<br>
+ .performDeadCodeElimination();<br>
}<br>
<br>
void getAnalysisUsage(AnalysisUsage &AU) const override {<br>
+ // We require DominatorTree here only to update and thus preserve it.<br>
+ AU.addRequired<DominatorTreeWr<wbr>apperPass>();<br>
AU.addRequired<PostDominatorT<wbr>reeWrapperPass>();<br>
if (!RemoveControlFlowFlag)<br>
AU.setPreservesCFG();<br>
AU.addPreserved<GlobalsAAWrap<wbr>perPass>();<br>
+ AU.addPreserved<DominatorTreeW<wbr>rapperPass>();<br>
+ AU.addPreserved<PostDominatorT<wbr>reeWrapperPass>();<br>
}<br>
};<br>
}<br>
<br>
Added: llvm/trunk/test/Transforms/ADC<wbr>E/domtree-DoubleDeletion.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ADCE/domtree-DoubleDeletion.ll?rev=311039&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/Transfor<wbr>ms/ADCE/domtree-DoubleDeletion<wbr>.ll?rev=311039&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Transforms/ADC<wbr>E/domtree-DoubleDeletion.ll (added)<br>
+++ llvm/trunk/test/Transforms/ADC<wbr>E/domtree-DoubleDeletion.ll Wed Aug 16 13:50:23 2017<br>
@@ -0,0 +1,39 @@<br>
+; RUN: opt < %s -gvn -simplifycfg -adce | llvm-dis<br>
+; RUN: opt < %s -gvn -simplifycfg -adce -verify-dom-info | llvm-dis<br>
+<br>
+; This test makes sure that the DominatorTree properly handles<br>
+; deletion of edges that go to forward-unreachable regions.<br>
+; In this case, %land.end is already forward unreachable when<br>
+; the DT gets informed about the deletion of %entry -> %land.end.<br>
+<br>
+@a = common global i32 0, align 4<br>
+<br>
+define i32 @main() {<br>
+entry:<br>
+ %retval = alloca i32, align 4<br>
+ store i32 0, i32* %retval, align 4<br>
+ %0 = load i32, i32* @a, align 4<br>
+ %cmp = icmp ne i32 %0, 1<br>
+ br i1 %cmp, label %land.rhs, label %land.end4<br>
+<br>
+land.rhs: ; preds = %entry<br>
+ %1 = load i32, i32* @a, align 4<br>
+ %tobool = icmp ne i32 %1, 0<br>
+ br i1 %tobool, label %land.rhs1, label %land.end<br>
+<br>
+land.rhs1: ; preds = %land.rhs<br>
+ br label %land.end<br>
+<br>
+land.end: ; preds = %land.rhs1, %land.rhs<br>
+ %2 = phi i1 [ false, %land.rhs ], [ true, %land.rhs1 ]<br>
+ %land.ext = zext i1 %2 to i32<br>
+ %conv = trunc i32 %land.ext to i16<br>
+ %conv2 = sext i16 %conv to i32<br>
+ %tobool3 = icmp ne i32 %conv2, 0<br>
+ br label %land.end4<br>
+<br>
+land.end4: ; preds = %land.end, %entry<br>
+ %3 = phi i1 [ false, %entry ], [ %tobool3, %land.end ]<br>
+ %land.ext5 = zext i1 %3 to i32<br>
+ ret i32 0<br>
+}<br>
<br>
Added: llvm/trunk/test/Transforms/ADC<wbr>E/unreachable.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ADCE/unreachable.ll?rev=311039&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/Transfor<wbr>ms/ADCE/unreachable.ll?rev=311<wbr>039&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Transforms/ADC<wbr>E/unreachable.ll (added)<br>
+++ llvm/trunk/test/Transforms/ADC<wbr>E/unreachable.ll Wed Aug 16 13:50:23 2017<br>
@@ -0,0 +1,18 @@<br>
+; RUN: opt < %s -adce -simplifycfg | llvm-dis<br>
+; RUN: opt < %s -passes=adce | llvm-dis<br>
+<br>
+define i32 @Test(i32 %A, i32 %B) {<br>
+BB1:<br>
+ br label %BB4<br>
+<br>
+BB2: ; No predecessors!<br>
+ br label %BB3<br>
+<br>
+BB3: ; preds = %BB4, %BB2<br>
+ %ret = phi i32 [ %X, %BB4 ], [ %B, %BB2 ] ; <i32> [#uses=1]<br>
+ ret i32 %ret<br>
+<br>
+BB4: ; preds = %BB1<br>
+ %X = phi i32 [ %A, %BB1 ] ; <i32> [#uses=1]<br>
+ br label %BB3<br>
+}<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br><br clear="all"><div><br></div></div></div><span class="m_7060354758183325962HOEnZb"><font color="#888888">-- <br><div class="m_7060354758183325962m_-4566766320090269238gmail_signature" data-smartmail="gmail_signature"><div>Jakub Kuderski</div></div>
</font></span></div>
</blockquote></div><br><br clear="all"><div><br></div></div></div><span class="HOEnZb"><font color="#888888">-- <br><div class="m_7060354758183325962gmail_signature" data-smartmail="gmail_signature"><div>Jakub Kuderski</div></div>
</font></span></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div>Jakub Kuderski</div></div>
</div>