[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