[PATCH] Fix dominator descendants for unreachable blocks.

Diego Novillo dnovillo at google.com
Mon Dec 2 06:14:36 PST 2013


  Thanks for the review.  Committed r196099.


================
Comment at: include/llvm/Analysis/Dominators.h:356
@@ -355,3 +355,3 @@
 
-    while (!WL.empty()) {
+    while (!WL.empty() && RN) {
       const DomTreeNodeBase<NodeT> *N = WL.pop_back_val();
----------------
Chandler Carruth wrote:
> No need to test RN in the loop condition, its invariant. Also, no need to build the worklist when RN is null.
> 
> I would change this to something like:
> 
>   Result.clear();
>   const DomTreeNodeBase<NodeT> *RN = getNode(R);
>   if (!RN)
>     return; // The descendants are the empty set for unreachable code.
>   WL.push_back(RN);
> 
Testing RN in the loop is not a problem, assuming code motion is doing what it should. I agree that it's best not to even try building the work list in that case.

================
Comment at: unittests/IR/DominatorTreeTest.cpp:168-173
@@ +167,8 @@
+
+        DominatedBBs.clear();
+        PostDominatedBBs.clear();
+        DT->getDescendants(BB3, DominatedBBs);
+        DT->getDescendants(BB3, PostDominatedBBs);
+        EXPECT_EQ(DominatedBBs.size(), 0UL);
+        EXPECT_EQ(PostDominatedBBs.size(), 0UL);
+
----------------
Chandler Carruth wrote:
> Comment that you're testing the behavior for unreachable basic blocks?
Done.


http://llvm-reviews.chandlerc.com/D2288



More information about the llvm-commits mailing list