ScheduleTree: replace operator== with treeEquals

In tree-like structures, the behavior of the default comparison
operators (operator== and operator!=) is not intuitive.  They may
compare individual tree nodes or the subtrees rooted at those nodes.
Replace the equality comparison operator on ScheduleTree with the
treeEquals method, which makes it clear that subtrees are compared (as
opposed to nodeEquals introduced previously).

Removing the overloaded comparison operators may make it harder to use
standard containers and algorithms on ScheduleTrees.  However, the
caller is never supposed to operate on ScheduleTrees by-value, and
pointers are trivially comparable.  Internal functions may define and
use a comparator class with clear intended behavior when necessary.  For
external uses, explicitly-named functions offer a better alternative.

Author: ftynse

tc/core/polyhedral/schedule_isl_conversion.cc

@@ -311,7 +311,7 @@ std::unique_ptr<ScheduleTree> fromIslSchedule(isl::schedule schedule) {
 // Note that the children of set and sequence nodes are always filters, so
 // they cannot be replaced by empty trees.
 bool validateSchedule(const ScheduleTree* st) {
-  return *st == *fromIslSchedule(toIslSchedule(st));
+  return st->treeEquals(fromIslSchedule(toIslSchedule(st)).get());
 }
 
 bool validateSchedule(isl::schedule sc) {

tc/core/polyhedral/schedule_tree.cc

@@ -336,21 +336,17 @@ vector<const ScheduleTree*> ScheduleTree::collectDFSPreorder(
   return functional::Filter(filterType, collectDFSPreorder(tree));
 }
 
-bool ScheduleTree::operator==(const ScheduleTree& other) const {
-  // ctx_ cmp ?
-  if (type_ != other.type_) {
+bool ScheduleTree::treeEquals(const ScheduleTree* other) const {
+  if (!nodeEquals(other)) {
     return false;
   }
-  if (children_.size() != other.children_.size()) {
+  if (numChildren() != other->numChildren()) {
     return false;
   }
-  if (!this->nodeEquals(&other)) {
-    return false;
-  }
-  TC_CHECK(!other.as<ScheduleTreeSet>())
+  TC_CHECK(!other->as<ScheduleTreeSet>())
       << "NYI: ScheduleTreeType::Set comparison";
-  for (size_t i = 0; i < children_.size(); ++i) {
-    if (*children_[i] != *other.children_[i]) {
+  for (size_t i = 0, e = numChildren(); i < e; ++i) {
+    if (!child({i})->treeEquals(other->child({i}))) {
       return false;
     }
   }

tc/core/polyhedral/schedule_tree.h

@@ -156,11 +156,6 @@ struct ScheduleTree {
  public:
   virtual ~ScheduleTree();
 
-  bool operator==(const ScheduleTree& other) const;
-  bool operator!=(const ScheduleTree& other) const {
-    return !(*this == other);
-  }
-
   // Swap a tree with with the given tree.
   void swapChild(size_t pos, ScheduleTreeUPtr& swappee) {
     TC_CHECK_GE(pos, 0u) << "position out of children bounds";
@@ -474,6 +469,10 @@ struct ScheduleTree {
   // use treeEquals() instead to compare entire trees.
   virtual bool nodeEquals(const ScheduleTree* other) const = 0;
 
+  // Comapre the subtree rooted at the current node to the subtree
+  // rooted at "other".
+  bool treeEquals(const ScheduleTree* other) const;
+
   //
   // Data members
   //

test/test_cuda_mapper.cc

@@ -144,8 +144,9 @@ struct PolyhedralMapperTest : public ::testing::Test {
       islNode = islNode.as<isl::schedule_node_band>().tile(mv);
       auto scheduleISL = fromIslSchedule(islNode.get_schedule().reset_user());
 
-      ASSERT_TRUE(*scheduleISL == *scheduleISLPP) << *scheduleISL << "\nVS\n"
-                                                  << *scheduleISLPP;
+      ASSERT_TRUE(scheduleISL->treeEquals(scheduleISLPP.get()))
+          << *scheduleISL << "\nVS\n"
+          << *scheduleISLPP;
     }
   }