From e15dd597ef5746094330e60fcddd027ca06683b3 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Fri, 17 Jul 2020 14:09:01 -0400
Subject: [PATCH 311/315] FIXME: implement constraint_manager2::canonicalize

---
 cover-letter.txt                    |   1 +
 gcc/analyzer/constraint-manager2.cc | 107 ++++++++++++++++++----------
 gcc/analyzer/constraint-manager2.h  |   2 +
 gcc/analyzer/program-state.cc       |   4 +-
 gcc/analyzer/region-model2.cc       |  14 +++-
 gcc/analyzer/region-model2.h        |   2 +
 6 files changed, 86 insertions(+), 44 deletions(-)

diff --git a/cover-letter.txt b/cover-letter.txt
index 1ed779340c6..c8dac3a6d62 100644
--- a/cover-letter.txt
+++ b/cover-letter.txt
@@ -21,6 +21,7 @@ time trying to fix canonicalization bugs, and there always seemed to
 be one more bug.  By eliminating IDs in this new implementation, no
 canonicalization or ID remapping should be needed, and a lot of tricky
 code goes away.
+FIXME: there's still some canonicalization in constraint_manager
 
 
 Ownership of regions and svalues
diff --git a/gcc/analyzer/constraint-manager2.cc b/gcc/analyzer/constraint-manager2.cc
index e1b33aa8f07..45cdbed94eb 100644
--- a/gcc/analyzer/constraint-manager2.cc
+++ b/gcc/analyzer/constraint-manager2.cc
@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "sbitmap.h"
 #include "bitmap.h"
 #include "tristate.h"
+#include "analyzer/region-model.h" // for one_way_id_map
 #include "analyzer/region-model2.h"
 #include "analyzer/constraint-manager2.h"
 #include "analyzer/analyzer-selftests.h"
@@ -404,25 +405,27 @@ equiv_class2::get_representative () const
     }
 }
 
-#if 0
 /* Comparator for use by equiv_class2::canonicalize.  */
 
 static int
-svalue2_id_cmp_by_id (const void *p1, const void *p2)
+svalue2_cmp_by_ptr (const void *p1, const void *p2)
 {
-  const svalue2 *sval1 = (const svalue2_id *)p1;
-  const svalue2 *sval2 = (const svalue2_id *)p2;
-  return sid1->as_int () - sid2->as_int ();
+  const svalue2 *sval1 = *(const svalue2 * const *)p1;
+  const svalue2 *sval2 = *(const svalue2 * const *)p2;
+  if (sval1 < sval2)
+    return 1;
+  if (sval1 > sval2)
+    return -1;
+  return 0;
 }
 
-/* Sort the svalue2s_ids within this equiv_class2.  */
+/* Sort the svalue2s within this equiv_class2.  */
 
 void
 equiv_class2::canonicalize ()
 {
-  m_vars.qsort (svalue2_id_cmp_by_id);
+  m_vars.qsort (svalue2_cmp_by_ptr);
 }
-#endif
 
 /* Get a debug string for C_OP.  */
 
@@ -1608,16 +1611,17 @@ constraint_manager2::purge (const purge_criteria2 &p, purge_stats2 *stats)
 static int
 equiv_class2_cmp (const void *p1, const void *p2)
 {
-  gcc_unreachable ();
-#if 0
   const equiv_class2 *ec1 = *(const equiv_class2 * const *)p1;
   const equiv_class2 *ec2 = *(const equiv_class2 * const *)p2;
 
-  svalue2_id rep1 = ec1->get_representative ();
-  svalue2_id rep2 = ec2->get_representative ();
+  const svalue2 *rep1 = ec1->get_representative ();
+  const svalue2 *rep2 = ec2->get_representative ();
 
-  return rep1.as_int () - rep2.as_int ();
-#endif
+  if (rep1 < rep2)
+    return 1;
+  if (rep1 > rep2)
+    return -1;
+  return 0;
 }
 
 /* Comparator for use by constraint_manager2::canonicalize.
@@ -1637,50 +1641,77 @@ constraint_cmp (const void *p1, const void *p2)
   return c1->m_op - c2->m_op;
 }
 
-/* Reorder the equivalence classes and constraints within this
-   constraint_manager2 into a canonical order, to increase the
+/* Purge redundant equivalence classes and constraints, and reorder them
+   within this constraint_manager2 into a canonical order, to increase the
    chances of finding equality with another instance.  */
-#if 0
+
 void
-constraint_manager2::canonicalize (unsigned num_svalue2_ids)
+constraint_manager2::canonicalize ()
 {
-  gcc_unreachable ();
-#if 0
-  /* First, sort svalue2_ids within the ECs.  */
+  /* First, sort svalue2s within the ECs.  */
   unsigned i;
   equiv_class2 *ec;
   FOR_EACH_VEC_ELT (m_equiv_classes, i, ec)
     ec->canonicalize ();
 
-  /* Next, sort the ECs into a canonical order.  */
+  /* TODO: remove constraints where both sides have a constant, and are
+     thus implicit.  But does this break transitivity?  */
 
-  /* We will need to remap the equiv_class2_ids in the constraints,
+  /* We will be purging and reordering ECs.
+     We will need to remap the equiv_class2_ids in the constraints,
      so we need to store the original index of each EC.
-     Build a lookup table, mapping from representative svalue2_id
-     to the original equiv_class2_id of that svalue2_id.  */
-  auto_vec<equiv_class2_id> original_ec_id (num_svalue2_ids);
-  for (i = 0; i < num_svalue2_ids; i++)
-    original_ec_id.quick_push (equiv_class2_id::null ());
+     Build a lookup table, mapping from the representative svalue2
+     to the original equiv_class2_id of that svalue2.  */
+  hash_map<const svalue2 *, equiv_class2_id> original_ec_id;
+  const unsigned orig_num_equiv_classes = m_equiv_classes.length ();
   FOR_EACH_VEC_ELT (m_equiv_classes, i, ec)
     {
-      svalue2_id rep = ec->get_representative ();
-      gcc_assert (!rep.null_p ());
-      original_ec_id[rep.as_int ()] = i;
+      const svalue2 *rep = ec->get_representative ();
+      original_ec_id.put (rep, i);
+    }
+
+  /* Find ECs used by constraints.  */
+  hash_set<const equiv_class2 *> used_ecs;
+  constraint2 *c;
+  FOR_EACH_VEC_ELT (m_constraints, i, c)
+    {
+      used_ecs.add (m_equiv_classes[c->m_lhs.as_int ()]);
+      used_ecs.add (m_equiv_classes[c->m_rhs.as_int ()]);
     }
 
-  /* Sort the equivalence classes.  */
+  /* Purge unused ECs: those that aren't used by constraints and
+     that effectively have only one svalue2 (either in m_constant
+     or in m_vars).  */
+  {
+    /* "unordered remove if" from a vec.  */
+    unsigned i = 0;
+    while (i < m_equiv_classes.length ())
+      {
+	equiv_class2 *ec = m_equiv_classes[i];
+	if (!used_ecs.contains (ec)
+	    && ((ec->m_vars.length () < 2 && ec->m_constant == NULL_TREE)
+		|| (ec->m_vars.length () == 0)))
+	  {
+	    m_equiv_classes.unordered_remove (i);
+	    delete ec;
+	  }
+	else
+	  i++;
+      }
+  }
+
+  /* Next, sort the surviving ECs into a canonical order.  */
   m_equiv_classes.qsort (equiv_class2_cmp);
 
   /* Populate ec_id_map based on the old vs new EC ids.  */
-  one_way_id_map<equiv_class2_id> ec_id_map (m_equiv_classes.length ());
+  one_way_id_map<equiv_class2_id> ec_id_map (orig_num_equiv_classes);
   FOR_EACH_VEC_ELT (m_equiv_classes, i, ec)
     {
-      svalue2_id rep = ec->get_representative ();
-      ec_id_map.put (original_ec_id[rep.as_int ()], i);
+      const svalue2 *rep = ec->get_representative ();
+      ec_id_map.put (*original_ec_id.get (rep), i);
     }
 
-  /* Update the EC ids within the constraints.  */
-  constraint2 *c;
+  /* Use ec_id_map to update the EC ids within the constraints.  */
   FOR_EACH_VEC_ELT (m_constraints, i, c)
     {
       ec_id_map.update (&c->m_lhs);
@@ -1689,9 +1720,7 @@ constraint_manager2::canonicalize (unsigned num_svalue2_ids)
 
   /* Finally, sort the constraints. */
   m_constraints.qsort (constraint_cmp);
-#endif
 }
-#endif
 
 /* A concrete subclass of constraint_manager2 for use when
    merging two constraint_manager2 into a third constraint_manager2,
diff --git a/gcc/analyzer/constraint-manager2.h b/gcc/analyzer/constraint-manager2.h
index cc7ea81d809..dcf209edbd5 100644
--- a/gcc/analyzer/constraint-manager2.h
+++ b/gcc/analyzer/constraint-manager2.h
@@ -270,6 +270,8 @@ public:
 
   void purge (const purge_criteria2 &p, purge_stats2 *stats);
 
+  void canonicalize ();
+
   static void merge (const constraint_manager2 &cm_a,
 		     const constraint_manager2 &cm_b,
 		     constraint_manager2 *out,
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index fc3c938e44b..e71837e86ab 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -1875,10 +1875,10 @@ program_state::prune_for_point2 (exploded_graph &eg,
       logger->log ("num constraints purged: %i", stats.m_num_constraints);
       logger->log ("num sm map items purged: %i", stats.m_num_client_items);
     }
-
-  new_state.m_region_model->canonicalize (&ctxt);
 #endif
 
+  new_state.m_region_model2->canonicalize ();
+
   return new_state;
 }
 
diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc
index 0faced37619..5e6badd9fad 100644
--- a/gcc/analyzer/region-model2.cc
+++ b/gcc/analyzer/region-model2.cc
@@ -4631,6 +4631,14 @@ region_model2::validate () const
 
 /* FIXME.  */
 
+void
+region_model2::canonicalize ()
+{
+  m_constraints2->canonicalize ();
+}
+
+/* FIXME.  */
+
 bool
 region_model2::on_stmt (const gimple *stmt,
 			region_model2_context *ctxt)
@@ -9604,7 +9612,7 @@ test_model_equality_2 ()
      x > 3 && y > 42
    and
      y > 42 && x > 3
-   are equal.  */
+   are equal (after canonicalization).  */
 
 static void
 test_model_equality_3 ()
@@ -9623,9 +9631,9 @@ test_model_equality_3 ()
   model1.add_constraint (y, GT_EXPR, int_42, NULL);
   model1.add_constraint (x, GT_EXPR, int_3, NULL);
 
-#if 0
+  model0.canonicalize ();
+  model1.canonicalize ();
   ASSERT_EQ (model0, model1);
-#endif
 }
 
 #if 0
diff --git a/gcc/analyzer/region-model2.h b/gcc/analyzer/region-model2.h
index b71a9a41a6f..45986012ed0 100644
--- a/gcc/analyzer/region-model2.h
+++ b/gcc/analyzer/region-model2.h
@@ -2563,6 +2563,8 @@ class region_model2
 
   bool region_exists_p (const region2 *reg) const;
 
+  void canonicalize ();
+
 private:
   const region2 *get_lvalue_1 (path_var pv,
 			       region_model2_context *ctxt);
-- 
2.26.2

