[PATCH] Fix dominator descendants for unreachable blocks.

Chandler Carruth chandlerc at gmail.com
Fri Nov 29 16:28:48 PST 2013


  Feel free to submit with a change along the lines of what I suggest in my first comment. I agree with your analysis, the expected result, and the added test cases look great.


================
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);
+
----------------
Comment that you're testing the behavior for unreachable basic blocks?

================
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();
----------------
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);



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



More information about the llvm-commits mailing list