Ensure ClearLoopInformation doesn't require particular ordering
The ClearLoopInformation call used to use the RPO list to find
blocks which potentially had loop-information. This meant that if one
was also clearing the dominance information (which is quite common)
one needed to be sure to call ClearLoopInformation before calling
ClearDominanceInformation or else loop information will not be fully
cleared. This could cause quite perplexing errors if dominance
information is recomputed later (also quite common). This error is in
fact present in several tests (none of which use loops which is how it
got missed).
Fix this issue by just looping over all blocks. Also add a new
GetActiveBlocks function which does the filtering of null blocks
automatically. In many cases (such as these) we use RPO purely because
it doesn't require filtering. This should be able to replace these
uses.
Test: ./test.py --host
Change-Id: I60c7defc409111471064e9bf02b7ae3a0eb10584
diff --git a/compiler/optimizing/nodes_test.cc b/compiler/optimizing/nodes_test.cc
index 9bfd250..34f0e9b 100644
--- a/compiler/optimizing/nodes_test.cc
+++ b/compiler/optimizing/nodes_test.cc
@@ -26,6 +26,118 @@
class NodeTest : public OptimizingUnitTest {};
/**
+ * Test that we can clear loop and dominator information in either order.
+ * Code is:
+ * while (true) {
+ * if (foobar) { break; }
+ * if (baz) { xyz; } else { abc; }
+ * }
+ * dosomething();
+ */
+TEST_F(NodeTest, ClearLoopThenDominanceInformation) {
+ CreateGraph();
+ AdjacencyListGraph alg(graph_,
+ GetAllocator(),
+ "entry",
+ "exit",
+ {{"entry", "loop_pre_header"},
+
+ {"loop_pre_header", "loop_header"},
+ {"loop_header", "critical_break"},
+ {"loop_header", "loop_body"},
+ {"loop_body", "loop_if_left"},
+ {"loop_body", "loop_if_right"},
+ {"loop_if_left", "loop_merge"},
+ {"loop_if_right", "loop_merge"},
+ {"loop_merge", "loop_header"},
+
+ {"critical_break", "breturn"},
+ {"breturn", "exit"}});
+ graph_->ClearDominanceInformation();
+ graph_->BuildDominatorTree();
+
+ // Test
+ EXPECT_TRUE(
+ std::all_of(graph_->GetBlocks().begin(), graph_->GetBlocks().end(), [&](HBasicBlock* b) {
+ return b == graph_->GetEntryBlock() || b == nullptr || b->GetDominator() != nullptr;
+ }));
+ EXPECT_TRUE(
+ std::any_of(graph_->GetBlocks().begin(), graph_->GetBlocks().end(), [&](HBasicBlock* b) {
+ return b != nullptr && b->GetLoopInformation() != nullptr;
+ }));
+
+ // Clear
+ graph_->ClearLoopInformation();
+ graph_->ClearDominanceInformation();
+
+ // Test
+ EXPECT_TRUE(
+ std::none_of(graph_->GetBlocks().begin(), graph_->GetBlocks().end(), [&](HBasicBlock* b) {
+ return b != nullptr && b->GetDominator() != nullptr;
+ }));
+ EXPECT_TRUE(
+ std::all_of(graph_->GetBlocks().begin(), graph_->GetBlocks().end(), [&](HBasicBlock* b) {
+ return b == nullptr || b->GetLoopInformation() == nullptr;
+ }));
+}
+
+/**
+ * Test that we can clear loop and dominator information in either order.
+ * Code is:
+ * while (true) {
+ * if (foobar) { break; }
+ * if (baz) { xyz; } else { abc; }
+ * }
+ * dosomething();
+ */
+TEST_F(NodeTest, ClearDominanceThenLoopInformation) {
+ CreateGraph();
+ AdjacencyListGraph alg(graph_,
+ GetAllocator(),
+ "entry",
+ "exit",
+ {{"entry", "loop_pre_header"},
+
+ {"loop_pre_header", "loop_header"},
+ {"loop_header", "critical_break"},
+ {"loop_header", "loop_body"},
+ {"loop_body", "loop_if_left"},
+ {"loop_body", "loop_if_right"},
+ {"loop_if_left", "loop_merge"},
+ {"loop_if_right", "loop_merge"},
+ {"loop_merge", "loop_header"},
+
+ {"critical_break", "breturn"},
+ {"breturn", "exit"}});
+ graph_->ClearDominanceInformation();
+ graph_->BuildDominatorTree();
+
+ // Test
+ EXPECT_TRUE(
+ std::all_of(graph_->GetBlocks().begin(), graph_->GetBlocks().end(), [&](HBasicBlock* b) {
+ return b == graph_->GetEntryBlock() || b == nullptr || b->GetDominator() != nullptr;
+ }));
+ EXPECT_TRUE(
+ std::any_of(graph_->GetBlocks().begin(), graph_->GetBlocks().end(), [&](HBasicBlock* b) {
+ return b != nullptr && b->GetLoopInformation() != nullptr;
+ }));
+
+ // Clear
+ graph_->ClearDominanceInformation();
+ graph_->ClearLoopInformation();
+
+ // Test
+ EXPECT_TRUE(
+ std::none_of(graph_->GetBlocks().begin(), graph_->GetBlocks().end(), [&](HBasicBlock* b) {
+ return b != nullptr && b->GetDominator() != nullptr;
+ }));
+ EXPECT_TRUE(
+ std::all_of(graph_->GetBlocks().begin(), graph_->GetBlocks().end(), [&](HBasicBlock* b) {
+ return b == nullptr || b->GetLoopInformation() == nullptr;
+ }));
+}
+
+/**
* Test that removing instruction from the graph removes itself from user lists
* and environment lists.
*/