From eab616a19025c3ec29915b0811277193ff68528e Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 18 Feb 2020 17:02:58 -0500
Subject: [PATCH 19/31] FIXME: WIP on rewrite of purging/canonicalization

---
 gcc/analyzer/canonicalization.h    |  62 +++++
 gcc/analyzer/constraint-manager.cc |  50 +++-
 gcc/analyzer/constraint-manager.h  |  13 +
 gcc/analyzer/program-state.cc      |  10 +-
 gcc/analyzer/region-model.cc       | 422 ++++++++++++++++-------------
 gcc/analyzer/region-model.h        |  40 +--
 6 files changed, 363 insertions(+), 234 deletions(-)
 create mode 100644 gcc/analyzer/canonicalization.h

diff --git a/gcc/analyzer/canonicalization.h b/gcc/analyzer/canonicalization.h
new file mode 100644
index 00000000000..205036623e1
--- /dev/null
+++ b/gcc/analyzer/canonicalization.h
@@ -0,0 +1,62 @@
+/* FIXME.
+   Copyright (C) 2019-2020 Free Software Foundation, Inc.
+   Contributed by David Malcolm <dmalcolm@redhat.com>.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_ANALYZER_CANONICALIZATION_H
+#define GCC_ANALYZER_CANONICALIZATION_H
+
+// FIXME: only used by region-model.cc, I think
+
+namespace ana {
+
+/* A bundle of data used when canonicalizing a region_model so that the
+   order of regions and svalues is in a predictable order (thus increasing
+   the chance of two region_models being equal).
+
+   This object is used to keep track of a recursive traversal across the
+   svalues and regions within the model, made in a deterministic order,
+   assigning new ids the first time each region or svalue is
+   encountered.  */
+
+struct canonicalization
+{
+  canonicalization (const region_model &model);
+  void walk_rid (region_id rid);
+  void walk_sid (svalue_id sid);
+  void walk_ec_id (equiv_class_id ec_id);
+
+  void dump_to_pp (pretty_printer *pp) const;
+  void dump (FILE *fp) const;
+  void dump () const;
+
+  const region_model &m_model;
+  /* Maps from existing IDs to new IDs.  */
+  region_id_map m_rid_map;
+  svalue_id_map m_sid_map;
+  equiv_class_id_map m_ec_id_map;
+
+  /* The next IDs to hand out.  */
+  int m_next_rid_int;
+  int m_next_sid_int;
+  int m_next_ec_id_int;
+};
+
+} // namespace ana
+
+#endif /* GCC_ANALYZER_CANONICALIZATION_H */
diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc
index a2ced7e49b9..b4b9e7106c7 100644
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -892,6 +892,9 @@ constraint_manager::add_constraint_internal (equiv_class_id lhs_id,
 bool
 constraint_manager::get_equiv_class_by_sid (svalue_id sid, equiv_class_id *out) const
 {
+  gcc_assert (!sid.null_p ());
+  gcc_assert (out);
+
   /* TODO: should we have a map, rather than these searches?  */
   int i;
   equiv_class *ec;
@@ -1132,11 +1135,10 @@ constraint_manager::purge (const purge_criteria &p, purge_stats *stats)
   for (unsigned con_idx = 0; con_idx < m_constraints.length (); )
     {
       constraint *c = &m_constraints[con_idx];
-      // FIXME: does the length == 0 ever occur anymore?
-      if ((m_equiv_classes[c->m_lhs.m_idx]->m_vars.length () == 0
-	   && m_equiv_classes[c->m_rhs.m_idx]->m_vars.length () == 0)
-	  || (m_equiv_classes[c->m_lhs.m_idx]->m_constant
-	      && m_equiv_classes[c->m_rhs.m_idx]->m_constant))
+      gcc_assert (m_equiv_classes[c->m_lhs.m_idx]->m_vars.length () > 0);
+      gcc_assert (m_equiv_classes[c->m_rhs.m_idx]->m_vars.length () > 0);
+      if (m_equiv_classes[c->m_lhs.m_idx]->m_constant
+	  && m_equiv_classes[c->m_rhs.m_idx]->m_constant)
 	{
 	  m_constraints.ordered_remove (con_idx);
 	  if (stats)
@@ -1204,6 +1206,37 @@ constraint_manager::remap_svalue_ids (const svalue_id_map &map)
     ec->remap_svalue_ids (map);
 }
 
+/* Remap all equiv_class_ids within this constraint_manager using MAP.  */
+
+void
+constraint_manager::remap_equiv_class_ids (const equiv_class_id_map &map)
+{
+  /* Update IDs within constraints.  */
+  int i;
+  constraint *c;
+  FOR_EACH_VEC_ELT (m_constraints, i, c)
+    {
+      map.update (&c->m_lhs);
+      map.update (&c->m_rhs);
+    }
+
+  /* Build a reordered equiv_classes vector.  */
+  auto_vec<equiv_class *> new_equiv_classes (m_equiv_classes.length ());
+  for (unsigned i = 0; i < m_equiv_classes.length (); i++)
+    {
+      equiv_class_id dst (equiv_class_id::from_int (i));
+      equiv_class_id src = map.get_src_for_dst (dst);
+      new_equiv_classes.quick_push (&src.get_obj (*this));
+    }
+
+  /* Copy over the reordered vec to m_equiv_classes.  */
+  m_equiv_classes.truncate (0);
+  gcc_assert (m_equiv_classes.space (new_equiv_classes.length ()));
+  equiv_class *ec;
+  FOR_EACH_VEC_ELT (new_equiv_classes, i, ec)
+    m_equiv_classes.quick_push (ec);
+}
+
 /* Comparator for use by constraint_manager::canonicalize.
    Sort a pair of equiv_class instances, using the representative
    svalue_id as a sort key.  */
@@ -1240,7 +1273,7 @@ constraint_cmp (const void *p1, const void *p2)
 /* Reorder the equivalence classes and constraints within this
    constraint_manager into a canonical order, to increase the
    chances of finding equality with another instance.  */
-
+#if 0
 void
 constraint_manager::canonicalize (unsigned num_svalue_ids)
 {
@@ -1288,6 +1321,7 @@ constraint_manager::canonicalize (unsigned num_svalue_ids)
   /* Finally, sort the constraints. */
   m_constraints.qsort (constraint_cmp);
 }
+#endif
 
 /* A concrete subclass of constraint_manager for use when
    merging two constraint_manager into a third constraint_manager,
@@ -2219,8 +2253,8 @@ test_many_constants ()
 
       region_model merged;
       ASSERT_TRUE (model.can_merge_with_p (other, &merged));
-      model.canonicalize (NULL);
-      merged.canonicalize (NULL);
+      model.canonicalize_and_purge (NULL);
+      merged.canonicalize_and_purge (NULL);
       ASSERT_EQ (model, merged);
 
       for (int j = 0; j <= i; j++)
diff --git a/gcc/analyzer/constraint-manager.h b/gcc/analyzer/constraint-manager.h
index 0f4506cc40d..916c45f928e 100644
--- a/gcc/analyzer/constraint-manager.h
+++ b/gcc/analyzer/constraint-manager.h
@@ -118,6 +118,8 @@ public:
   int m_idx;
 };
 
+typedef id_map<equiv_class_id> equiv_class_id_map;
+
 /* A relationship between two equivalence classes in a constraint_manager.  */
 
 class constraint
@@ -141,6 +143,14 @@ class constraint
     return m_op != CONSTRAINT_NE;
   }
 
+  bool compares_constants_p (const constraint_manager &cm) const
+  {
+    if (!m_lhs.get_obj (cm).m_cst_sid.null_p ()
+	&& !m_rhs.get_obj (cm).m_cst_sid.null_p ())
+      return true;
+    return false;
+  }
+
   equiv_class_id m_lhs;
   enum constraint_op m_op;
   equiv_class_id m_rhs;
@@ -173,6 +183,8 @@ public:
   virtual svalue_id get_sid_for_constant (tree cst) const = 0;
   virtual int get_num_svalues () const = 0;
 
+  int get_num_equiv_classes () const { return m_equiv_classes.length (); }
+
   constraint_manager& operator= (const constraint_manager &other);
 
   hashval_t hash () const;
@@ -220,6 +232,7 @@ public:
   void purge (const purge_criteria &p, purge_stats *stats);
 
   void remap_svalue_ids (const svalue_id_map &map);
+  void remap_equiv_class_ids (const equiv_class_id_map &map);
 
   void canonicalize (unsigned num_svalue_ids);
 
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index 971e8e0a7d6..ad8ea0407fc 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -920,7 +920,7 @@ program_state::prune_for_point (exploded_graph &eg,
 				  &new_state,
 				  change,
 				  NULL);
-  new_state.m_region_model->purge_unused_svalues (&stats, &ctxt);
+  new_state.m_region_model->canonicalize_and_purge (&ctxt, &stats);
   if (logger)
     {
       logger->log ("num svalues purged: %i", stats.m_num_svalues);
@@ -930,8 +930,6 @@ program_state::prune_for_point (exploded_graph &eg,
       logger->log ("num sm map items purged: %i", stats.m_num_client_items);
     }
 
-  new_state.m_region_model->canonicalize (&ctxt);
-
   return new_state;
 }
 
@@ -1034,7 +1032,7 @@ program_state::can_merge_with_p (const program_state &other,
     }
 
   impl_region_model_context ctxt (out, NULL, ext_state);
-  out->m_region_model->canonicalize (&ctxt);
+  out->m_region_model->canonicalize_and_purge (&ctxt);
 
   return true;
 }
@@ -1392,7 +1390,7 @@ test_program_state_merging ()
   smap->impl_set_state (ptr_sid, TEST_STATE, svalue_id::null ());
   ASSERT_EQ (smap->get_state (ptr_sid), TEST_STATE);
 
-  model0->canonicalize (&ctxt);
+  model0->canonicalize_and_purge (&ctxt);
 
   /* Verify that canonicalization preserves sm-state.  */
   ASSERT_EQ (smap->get_state (model0->get_rvalue (p, NULL)), TEST_STATE);
@@ -1416,7 +1414,7 @@ test_program_state_merging ()
 
   /* Try canonicalizing.  */
   impl_region_model_context merged_ctxt (&merged, NULL, ext_state);
-  merged.m_region_model->canonicalize (&merged_ctxt);
+  merged.m_region_model->canonicalize_and_purge (&merged_ctxt);
   merged.validate (ext_state);
 
   /* Verify that the merged state still has the sm-state for "p".  */
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 46b3ee56c14..0916cae03f9 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -59,6 +59,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "analyzer/sm.h"
 #include "analyzer/pending-diagnostic.h"
 #include "analyzer/analyzer-selftests.h"
+#include "analyzer/canonicalization.h"
 
 #if ENABLE_ANALYZER
 
@@ -2599,7 +2600,7 @@ stack_region::pop_frame (region_model *model, bool purge, purge_stats *stats,
 
   /* Delete unused svalues, but don't delete the return value.  */
   if (purge)
-    model->purge_unused_svalues (stats, ctxt, &result_sid);
+    model->canonicalize_and_purge (ctxt, stats, &result_sid);
 
   model->validate ();
 
@@ -3728,9 +3729,10 @@ region_model::validate () const
 
 /* Global data for use by svalue_id_cmp_by_constant_svalue.  */
 
-static region_model *svalue_id_cmp_by_constant_svalue_model = NULL;
+static const region_model *svalue_id_cmp_by_constant_svalue_model = NULL;
 
 /* Comparator for use by region_model::canonicalize.  */
+// FIXME: ^^^
 
 static int
 svalue_id_cmp_by_constant_svalue (const void *p1, const void *p2)
@@ -3752,77 +3754,13 @@ svalue_id_cmp_by_constant_svalue (const void *p1, const void *p2)
   return tree_cmp (cst1, cst2);
 }
 
-/* Reorder the regions and svalues into a deterministic "canonical" order,
-   to maximize the chance of equality.
-   If non-NULL, notify CTXT about the svalue id remapping.  */
-
-void
-region_model::canonicalize (region_model_context *ctxt)
-{
-  /* Walk all regions and values in a deterministic order, visiting
-     rids and sids, generating a rid and sid map.  */
-  canonicalization c (*this);
-
-  /* (1): Walk all svalues, putting constants first, sorting the constants
-     (thus imposing an ordering on any constants that are purely referenced
-     by constraints).
-     Ignore other svalues for now.  */
-  {
-    unsigned i;
-    auto_vec<svalue_id> sids;
-    svalue *sval;
-    FOR_EACH_VEC_ELT (m_svalues, i, sval)
-      {
-	if (sval->get_kind () == SK_CONSTANT)
-	  sids.safe_push (svalue_id::from_int (i));
-      }
-    svalue_id_cmp_by_constant_svalue_model = this;
-    sids.qsort (svalue_id_cmp_by_constant_svalue);
-    svalue_id_cmp_by_constant_svalue_model = NULL;
-    svalue_id *sid;
-    FOR_EACH_VEC_ELT (sids, i, sid)
-      c.walk_sid (*sid);
-  }
-
-  /* (2): Walk all regions (and thus their values) in a deterministic
-     order.  */
-  c.walk_rid (m_root_rid);
-
-  /* (3): Ensure we've visited everything, as we don't want to purge
-     at this stage.  Anything we visit for the first time here has
-     arbitrary order.  */
-  {
-    unsigned i;
-    region *region;
-    FOR_EACH_VEC_ELT (m_regions, i, region)
-      c.walk_rid (region_id::from_int (i));
-    svalue *sval;
-    FOR_EACH_VEC_ELT (m_svalues, i, sval)
-      c.walk_sid (svalue_id::from_int (i));
-  }
-
-  /* (4): We now have a reordering of the regions and values.
-     Apply it.  */
-  remap_svalue_ids (c.m_sid_map);
-  remap_region_ids (c.m_rid_map);
-  if (ctxt)
-    ctxt->remap_svalue_ids (c.m_sid_map);
-
-  /* (5): Canonicalize the constraint_manager (it has already had its
-     svalue_ids remapped above).  This makes use of the new svalue_id
-     values, and so must happen last.  */
-  m_constraints->canonicalize (get_num_svalues ());
-
-  validate ();
-}
-
 /* Return true if this region_model is in canonical form.  */
 
 bool
 region_model::canonicalized_p () const
 {
   region_model copy (*this);
-  copy.canonicalize (NULL);
+  copy.canonicalize_and_purge (NULL);
   return *this == copy;
 }
 
@@ -4301,7 +4239,7 @@ region_model::on_call_post (const gcall *call,
 		delete_region_and_descendents (pointee_rid,
 					       POISON_KIND_FREED,
 					       &stats, ctxt->get_logger ());
-		purge_unused_svalues (&stats, ctxt);
+		canonicalize_and_purge (ctxt, &stats);
 		validate ();
 		// TODO: do anything with stats?
 	      }
@@ -4586,7 +4524,7 @@ region_model::on_longjmp (const gcall *longjmp_call, const gcall *setjmp_call,
 
   /* Now that we've assigned the fake_retval, we can purge the unused
      svalues, which could detect leaks.  */
-  purge_unused_svalues (NULL, ctxt, NULL);
+  canonicalize_and_purge (ctxt);
   validate ();
 }
 
@@ -5270,7 +5208,7 @@ region_model::eval_condition (svalue_id lhs_sid,
    this model, without resorting to the constraint_manager.
 
    This is exposed so that impl_region_model_context::on_state_leak can
-   check for equality part-way through region_model::purge_unused_svalues
+   check for equality part-way through region_model::canonicalize_and_purge
    without risking creating new ECs.  */
 
 tristate
@@ -5793,7 +5731,7 @@ region_model::update_for_return_superedge (const return_superedge &return_edge,
       /* This could be a leak; try purging again, but this time,
 	 don't special-case the result_sid.  */
       purge_stats stats;
-      purge_unused_svalues (&stats, ctxt);
+      canonicalize_and_purge (ctxt, &stats);
     }
 }
 
@@ -6105,32 +6043,37 @@ region_model::add_region_for_type (region_id parent_rid, tree type)
   return add_region (new_region);
 }
 
-/* Helper class for region_model::purge_unused_svalues.  */
+/* Comparator for constraints to impose a deterministic ordering
+   during canonicalization.  */
 
-class restrict_to_used_svalues : public purge_criteria
+static int
+constraint_cmp (const void *p1, const void *p2)
 {
-public:
-  restrict_to_used_svalues (const auto_sbitmap &used) : m_used (used) {}
+  const constraint *c1 = (const constraint *)p1;
+  const constraint *c2 = (const constraint *)p2;
 
-  bool should_purge_p (svalue_id sid) const FINAL OVERRIDE
-  {
-    gcc_assert (!sid.null_p ());
-    return !bitmap_bit_p (m_used, sid.as_int ());
-  }
+  if (int cmp_lhs = c1->m_lhs.as_int () - c2->m_lhs.as_int ())
+    return cmp_lhs;
+  if (int cmp_rhs = c1->m_rhs.as_int () - c2->m_rhs.as_int ())
+    return cmp_rhs;
+  return 0; // FIXME
+}
 
-private:
-  const auto_sbitmap &m_used;
-};
+/* Walk the regions and svalues, reordering those that are expressible
+   from user code into a deterministic "canonical" order, and deleting
+   any that aren't reachable, in both cases to maximize the chance of
+   equality when comparing region_model instances.
 
-/* Remove unused svalues from this model, accumulating stats into STATS.
-   Unused svalues are deleted.  Doing so could reorder the svalues, and
-   thus change the meaning of svalue_ids.
+   The surviving svalues and regions are likely to have been reordered,
+   thus changing the meaning of svalue_ids and region_ids.
 
-   If CTXT is non-NULL, then it is notified about svalue_id remappings,
-   and about svalue_ids that are about to be deleted.  This allows e.g.
-   for warning about resource leaks, for the case where the svalue
-   represents a resource handle in the user code (e.g. a FILE * or a malloc
-   buffer).
+   If CTXT is non-NULL, notify it about the svalue_id and region_id remappings,
+   and about svalue_ids that are about to be deleted.
+   This allows e.g. for warning about resource leaks, for the case where the
+   svalue represents a resource handle in the user code (e.g. a FILE * or a
+   malloc buffer).
+
+   If STATS is non-NULL, accumulate stats into it.
 
    Amongst other things, removing unused svalues is important for ensuring
    that the analysis of loops terminates.  Otherwise, we could generate a
@@ -6146,117 +6089,139 @@ private:
    lvalue and rvalue lookups implicitly relative to a "current" frame).  */
 
 void
-region_model::purge_unused_svalues (purge_stats *stats,
-				    region_model_context *ctxt,
-				    svalue_id *known_used_sid)
+region_model::canonicalize_and_purge (region_model_context *ctxt,
+				      purge_stats *stats,
+				      svalue_id *known_used_sid)
 {
   // TODO: might want to avoid a vfunc call just to do logging here:
   logger *logger = ctxt ? ctxt->get_logger () : NULL;
 
   LOG_SCOPE (logger);
 
-  auto_sbitmap used (m_svalues.length ());
-  bitmap_clear (used);
 
+  /* Walk all regions (and thus their values) in a deterministic
+     order.  */
+  canonicalization c (*this);
+  c.walk_rid (m_root_rid);
+
+  /* Walk the extra KNOWN_USED_SID, if there is one.  */
   if (known_used_sid)
     if (!known_used_sid->null_p ())
-      bitmap_set_bit (used, known_used_sid->as_int ());
-
-  /* Walk the regions, marking sids that are used.  */
-  unsigned i;
-  region *r;
-  FOR_EACH_VEC_ELT (m_regions, i, r)
-    {
-      svalue_id sid = r->get_value_direct ();
-      if (!sid.null_p ())
-	bitmap_set_bit (used, sid.as_int ());
-    }
-
-  /* Now purge any constraints involving svalues we don't care about.  */
-  restrict_to_used_svalues criterion (used);
-  m_constraints->purge (criterion, stats);
-
-  /* Mark any sids that are in constraints that survived.  */
+      c.walk_sid (*known_used_sid);
+
+  /* Mark where we got to.  Anything beyond this point will be purged.  */
+  svalue_id first_unused_sid = svalue_id::from_int (c.m_next_sid_int);
+  region_id first_unused_rid = region_id::from_int (c.m_next_rid_int);
+  equiv_class_id first_unused_ec_id
+    = equiv_class_id::from_int (c.m_next_ec_id_int);
+
+  /* Now walk all regions, svalues and equiv_classes, so that we visit all of
+     them.
+     Any that we're now visiting for the first time will be moved to the
+     end and purged.  */
   {
+    unsigned i;
+    region *region;
+    FOR_EACH_VEC_ELT (m_regions, i, region)
+      c.walk_rid (region_id::from_int (i));
+    svalue *sval;
+    FOR_EACH_VEC_ELT (m_svalues, i, sval)
+      c.walk_sid (svalue_id::from_int (i));
     equiv_class *ec;
-    FOR_EACH_VEC_ELT (m_constraints->m_equiv_classes, i, ec)
-      {
-	int j;
-	svalue_id *sid;
-	FOR_EACH_VEC_ELT (ec->m_vars, j, sid)
-	  {
-	    gcc_assert (!sid->null_p ());
-	    bitmap_set_bit (used, sid->as_int ());
-	  }
-      }
+    FOR_EACH_VEC_ELT (get_constraints ()->m_equiv_classes, i, ec)
+      c.walk_ec_id (equiv_class_id::from_int (i));
   }
 
-  /* Build a mapping from old-sid to new-sid so that we can preserve
-     order of the used IDs and move all redundant ones to the end.
-     Iterate though svalue IDs, adding used ones to the front of
-     the new list, and unused ones to the back.  */
-  svalue_id_map map (m_svalues.length ());
-  int next_used_new_sid = 0;
-  int after_next_unused_new_sid = m_svalues.length ();
-  for (unsigned i = 0; i < m_svalues.length (); i++)
-    {
-      svalue_id src (svalue_id::from_int (i));
-      if (bitmap_bit_p (used, i))
-	{
-	  if (logger)
-	    logger->log ("sv%i is used", i);
-	  map.put (src, svalue_id::from_int (next_used_new_sid++));
-	}
-      else
-	{
-	  if (logger)
-	    logger->log ("sv%i is unused", i);
-	  map.put (src, svalue_id::from_int (--after_next_unused_new_sid));
-	}
-    }
-  /* The two insertion points should have met.  */
-  gcc_assert (next_used_new_sid == after_next_unused_new_sid);
-
-  /* Now walk the regions and the constraints, remapping sids,
-     so that all the redundant svalues are at the end.  */
-  remap_svalue_ids (map);
-
   if (logger)
-    {
-      logger->start_log_line ();
-      logger->log_partial ("map: ");
-      map.dump_to_pp (logger->get_printer ());
-      logger->end_log_line ();
-    }
+    c.dump_to_pp (logger->get_printer ());
+
+  /* We now have a reordering of the regions, values, and equiv classes.
+     Apply it.  */
+  remap_svalue_ids (c.m_sid_map);
+  if (known_used_sid)
+    c.m_sid_map.update (known_used_sid);
+  remap_region_ids (c.m_rid_map);
+  get_constraints ()->remap_equiv_class_ids (c.m_ec_id_map);
 
   /* Notify any client about the remapping and pending deletion.
      Potentially this could trigger leak warnings.  */
   if (ctxt)
     {
-      ctxt->remap_svalue_ids (map);
+      ctxt->remap_svalue_ids (c.m_sid_map);
       int num_client_items_purged
-	= ctxt->on_svalue_purge (svalue_id::from_int (next_used_new_sid), map);
+	= ctxt->on_svalue_purge (first_unused_sid, c.m_sid_map);
       if (stats)
 	stats->m_num_client_items += num_client_items_purged;
     }
 
+  /* Drop constraints that refer to anything ECs being purged, or that
+     purely compare constants.  */ // FIXME: cst vs cst
+  // FIXME: move to constraint_manager?
+  // FIXME: should we reorder them?
+  for (unsigned con_idx = 0;
+       con_idx < get_constraints ()->m_constraints.length (); )
+    {
+      constraint *c = &get_constraints ()->m_constraints[con_idx];
+      if (c->m_lhs.as_int () >= first_unused_ec_id.as_int ()
+	  || c->m_rhs.as_int () >= first_unused_ec_id.as_int ()
+	  || c->compares_constants_p (*get_constraints ()))
+	{
+	  get_constraints ()->m_constraints.ordered_remove (con_idx);
+	  if (stats)
+	    stats->m_num_constraints++;
+	}
+      else
+	con_idx++;
+    }
+  /* Sort the remaining constraints by sid.  */
+  get_constraints ()->m_constraints.qsort (constraint_cmp);
+
+  /* Drop the redundant equiv_classes from the end of the vector.  */
+  while ((signed)get_constraints ()->m_equiv_classes.length ()
+	 > first_unused_ec_id.as_int ())
+    {
+      if (logger)
+	{
+	  equiv_class_id victim = equiv_class_id::from_int
+	    (get_constraints ()->m_equiv_classes.length () - 1);
+	  logger->log ("deleting ec%i (was ec%i)",
+		       victim.as_int (),
+		       c.m_ec_id_map.get_src_for_dst (victim).as_int ());
+	}
+      delete get_constraints ()->m_equiv_classes.pop ();
+      if (stats)
+	stats->m_num_equiv_classes++;
+    }
+
   /* Drop the redundant svalues from the end of the vector.  */
-  while ((signed)m_svalues.length () > next_used_new_sid)
+  while ((signed)m_svalues.length () > first_unused_sid.as_int ())
     {
       if (logger)
 	{
 	  svalue_id victim = svalue_id::from_int (m_svalues.length () - 1);
 	  logger->log ("deleting sv%i (was sv%i)",
 		       victim.as_int (),
-		       map.get_src_for_dst (victim).as_int ());
+		       c.m_sid_map.get_src_for_dst (victim).as_int ());
 	}
       delete m_svalues.pop ();
       if (stats)
 	stats->m_num_svalues++;
     }
 
-  if (known_used_sid)
-    map.update (known_used_sid);
+  /* Drop the redundant regions from the end of the vector.  */
+  while ((signed)m_regions.length () > first_unused_rid.as_int ())
+    {
+      if (logger)
+	{
+	  region_id victim = region_id::from_int (m_regions.length () - 1);
+	  logger->log ("deleting r%i (was r%i)",
+		       victim.as_int (),
+		       c.m_rid_map.get_src_for_dst (victim).as_int ());
+	}
+      delete m_regions.pop ();
+      if (stats)
+	stats->m_num_regions++;
+    }
 
   validate ();
 }
@@ -6306,7 +6271,8 @@ region_model::remap_region_ids (const region_id_map &map)
   /* Update IDs within svalues.  */
   svalue *sval;
   FOR_EACH_VEC_ELT (m_svalues, i, sval)
-    sval->remap_region_ids (map);
+    if (sval)
+      sval->remap_region_ids (map);
 
   /* Build a reordered regions vector.  */
   auto_vec<region *> new_regions (m_regions.length ());
@@ -6948,8 +6914,10 @@ canonicalization::canonicalization (const region_model &model)
 : m_model (model),
   m_rid_map (model.get_num_regions ()),
   m_sid_map (model.get_num_svalues ()),
+  m_ec_id_map (model.get_constraints ()->get_num_equiv_classes ()),
   m_next_rid_int (0),
-  m_next_sid_int (0)
+  m_next_sid_int (0),
+  m_next_ec_id_int (0)
 {
 }
 
@@ -6988,6 +6956,72 @@ canonicalization::walk_sid (svalue_id sid)
       m_sid_map.put (sid, svalue_id::from_int (m_next_sid_int++));
       /* Potentially walk regions e.g. for ptrs.  */
       sval->walk_for_canonicalization (this);
+
+      equiv_class_id ec_id (-1);
+      if (m_model.get_constraints ()->get_equiv_class_by_sid (sid, &ec_id))
+	walk_ec_id (ec_id);
+    }
+}
+
+/* FIXME.  */
+
+void
+canonicalization::walk_ec_id (equiv_class_id ec_id)
+{
+  /* Stop if we've already seen EC_ID.  */
+  if (!m_ec_id_map.get_dst_for_src (ec_id).null_p ())
+    return;
+
+  m_ec_id_map.put (ec_id, equiv_class_id::from_int (m_next_ec_id_int++));
+
+  // FIXME: also walk sids in ec? what order?
+#if 0
+  // TODO: get the sids, sort them somehow, then walk them?
+  const equiv_class &ec = ec_id.get_obj (*m_model.get_constraints ());
+  unsigned var_idx;
+  svalue_id *var_in_ec;
+  FOR_EACH_VEC_ELT (ec.m_vars, var_idx, var_in_ec)
+    walk_sid (*var_in_ec);
+#endif
+
+  const equiv_class &ec = ec_id.get_obj (*m_model.get_constraints ());
+  if (!ec.m_cst_sid.null_p ())
+    walk_sid (ec.m_cst_sid);
+  else
+    {
+      /* If non-constant, then find constraints that compare ec_id against
+	 constants, gathering a list of such constants.
+	 Ignore constraints that compare this non-constant against other
+	 non-constants, to ensure that we purge redundant svalues.  */
+      // FIXME: is this O(n^2) ?
+      auto_vec<svalue_id> constant_sids;
+      unsigned con_idx;
+      constraint *c;
+      FOR_EACH_VEC_ELT (m_model.get_constraints ()->m_constraints, con_idx, c)
+	{
+	  if (c->m_lhs == ec_id)
+	    {
+	      svalue_id rhs_cst_sid = c->m_rhs.get_obj (*m_model.get_constraints ()).m_cst_sid;
+	      if (!rhs_cst_sid.null_p ())
+		constant_sids.safe_push (rhs_cst_sid);
+	    }
+	  if (c->m_rhs == ec_id)
+	    {
+	      svalue_id lhs_cst_sid = c->m_lhs.get_obj (*m_model.get_constraints ()).m_cst_sid;
+	      if (!lhs_cst_sid.null_p ())
+		constant_sids.safe_push (lhs_cst_sid);
+	    }
+	}
+
+      /* Sort the constants into a canonical order.  */
+      svalue_id_cmp_by_constant_svalue_model = &m_model;
+      constant_sids.qsort (svalue_id_cmp_by_constant_svalue);
+      svalue_id_cmp_by_constant_svalue_model = NULL;
+
+      int i;
+      svalue_id *cst_sid;
+      FOR_EACH_VEC_ELT (constant_sids, i, cst_sid)
+	walk_sid (*cst_sid);
     }
 }
 
@@ -7005,6 +7039,11 @@ canonicalization::dump_to_pp (pretty_printer *pp) const
   pp_newline (pp);
   m_sid_map.dump_to_pp (pp);
   pp_newline (pp);
+
+  pp_string (pp, "equiv_class_id map:");
+  pp_newline (pp);
+  m_ec_id_map.dump_to_pp (pp);
+  pp_newline (pp);
 }
 
 /* Dump a multiline representation of this to FILE.  */
@@ -7283,6 +7322,7 @@ test_region_equality ()
   // TODO: test coverage for the map within a map_region
 }
 
+// FIXME: is the following used anywhere except in selftests?
 /* A subclass of purge_criteria for selftests: purge all svalue_id instances.  */
 
 class purge_all_svalue_ids : public purge_criteria
@@ -7412,10 +7452,10 @@ test_purging_by_criteria ()
 
 }
 
-/* Test that region_model::purge_unused_svalues works as expected.  */
+/* Test that region_model::purge_unused works as expected.  */
 
 static void
-test_purge_unused_svalues ()
+test_purge_unused ()
 {
   tree int_42 = build_int_cst (integer_type_node, 42);
   tree int_0 = build_int_cst (integer_type_node, 0);
@@ -7439,7 +7479,7 @@ test_purge_unused_svalues ()
 
   /* The redundant unknown values should have been purged.  */
   purge_stats purged;
-  model.purge_unused_svalues (&purged, NULL);
+  model.canonicalize_and_purge (&ctxt, &purged);
   ASSERT_EQ (purged.m_num_svalues, 3);
 
   /* and the redundant constraint on an old, unknown value for x should
@@ -7588,14 +7628,20 @@ test_stack_frames ()
   purge_stats purged;
   model.pop_frame (true, &purged, &ctxt);
 
-  /* We should have purged the unknown values for x and y. */
-  ASSERT_EQ (purged.m_num_svalues, 2);
+  /* We should have purged:
+     - the constant 0 for the value of x,
+     - the unknown value for y,
+     - the constant 5 for the constraint on y.  */
+  ASSERT_EQ (purged.m_num_svalues, 3);
 
   /* We should have purged the frame region and the regions for x and y. */
   ASSERT_EQ (purged.m_num_regions, 3);
 
-  /* We should have purged the constraint on y.  */
-  ASSERT_EQ (purged.m_num_equiv_classes, 1);
+  /* We should have purged the ECs for y and for the constant 5.  */
+  ASSERT_EQ (purged.m_num_equiv_classes, 2);
+
+  /* We should have purged the constraint y != 5, and also the constraint
+     comparing constants 5 and 10.  */
   ASSERT_EQ (purged.m_num_constraints, 2);
 
   /* Verify that p (which was pointing at the local "x" in the popped
@@ -7695,8 +7741,8 @@ test_canonicalization_1 ()
   model1.get_root_region ()->ensure_globals_region (&model1);
   model1.get_root_region ()->ensure_stack_region (&model1);
 
-  model0.canonicalize (NULL);
-  model1.canonicalize (NULL);
+  model0.canonicalize_and_purge (NULL);
+  model1.canonicalize_and_purge (NULL);
   ASSERT_EQ (model0, model1);
 }
 
@@ -7730,8 +7776,8 @@ test_canonicalization_2 ()
 		    model1.get_rvalue (int_42, NULL),
 		    NULL);
 
-  model0.canonicalize (NULL);
-  model1.canonicalize (NULL);
+  model0.canonicalize_and_purge (NULL);
+  model1.canonicalize_and_purge (NULL);
   ASSERT_EQ (model0, model1);
 }
 
@@ -7757,8 +7803,8 @@ test_canonicalization_3 ()
   model1.add_constraint (y, GT_EXPR, int_42, NULL);
   model1.add_constraint (x, GT_EXPR, int_3, NULL);
 
-  model0.canonicalize (NULL);
-  model1.canonicalize (NULL);
+  model0.canonicalize_and_purge (NULL);
+  model1.canonicalize_and_purge (NULL);
   ASSERT_EQ (model0, model1);
 }
 
@@ -7778,7 +7824,7 @@ test_canonicalization_4 ()
   FOR_EACH_VEC_ELT (csts, i, cst)
     model.get_rvalue (cst, NULL);
 
-  model.canonicalize (NULL);
+  model.canonicalize_and_purge (NULL);
 }
 
 /* Assert that if we have two region_model instances
@@ -8029,7 +8075,7 @@ test_state_merging ()
       = model0.get_or_create_ptr_svalue (ptr_type_node, new_rid);
     model0.set_value (model0.get_lvalue (p, &ctxt),
 		      ptr_sid, &ctxt);
-    model0.canonicalize (&ctxt);
+    model0.canonicalize_and_purge (&ctxt);
 
     region_model model1 (model0);
 
@@ -8038,7 +8084,7 @@ test_state_merging ()
     region_model merged;
     ASSERT_TRUE (model0.can_merge_with_p (model1, &merged));
 
-    merged.canonicalize (&ctxt);
+    merged.canonicalize_and_purge (&ctxt);
 
     /* The merged model ought to be identical (after canonicalization,
        at least).  */
@@ -8134,7 +8180,7 @@ test_state_merging ()
       = model0.get_svalue (sid_ptr)->dyn_cast_region_svalue ()->get_pointee ();
     ASSERT_EQ (model0.get_region (rid_pointee)->get_parent (), rid_2nd_frame);
 
-    model0.canonicalize (NULL);
+    model0.canonicalize_and_purge (NULL);
 
     region_model model1 (model0);
     ASSERT_EQ (model0, model1);
@@ -8143,7 +8189,7 @@ test_state_merging ()
        (after canonicalization, at least).  */
     region_model merged;
     ASSERT_TRUE (model0.can_merge_with_p (model1, &merged));
-    merged.canonicalize (NULL);
+    merged.canonicalize_and_purge (NULL);
     ASSERT_EQ (model0, merged);
   }
 
@@ -8154,7 +8200,7 @@ test_state_merging ()
     model0.set_value (model0.get_lvalue (q, NULL),
 		      model0.get_rvalue (addr_of_y, NULL), NULL);
 
-    model0.canonicalize (NULL);
+    model0.canonicalize_and_purge (NULL);
 
     region_model model1 (model0);
     ASSERT_EQ (model0, model1);
@@ -8163,7 +8209,7 @@ test_state_merging ()
        (after canonicalization, at least).  */
     region_model merged;
     ASSERT_TRUE (model0.can_merge_with_p (model1, &merged));
-    merged.canonicalize (NULL);
+    merged.canonicalize_and_purge (NULL);
     ASSERT_EQ (model0, merged);
   }
 }
@@ -8226,7 +8272,7 @@ analyzer_region_model_cc_tests ()
   test_svalue_equality ();
   test_region_equality ();
   test_purging_by_criteria ();
-  test_purge_unused_svalues ();
+  test_purge_unused ();
   test_assignment ();
   test_stack_frames ();
   test_get_representative_path_var ();
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 3b2b43b595f..130561a5c61 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -1670,7 +1670,9 @@ class region_model
 
   void validate () const;
 
-  void canonicalize (region_model_context *ctxt);
+  void canonicalize_and_purge (region_model_context *ctxt,
+			       purge_stats *out = NULL,
+			       svalue_id *known_used_sid = NULL);
   bool canonicalized_p () const;
 
   void check_for_poison (tree expr, region_model_context *ctxt);
@@ -1780,9 +1782,6 @@ class region_model
   path_var get_representative_path_var (region_id rid) const;
   void get_path_vars_for_svalue (svalue_id sid, vec<path_var> *out) const;
 
-  void purge_unused_svalues (purge_stats *out,
-			     region_model_context *ctxt,
-			     svalue_id *known_used_sid = NULL);
   void remap_svalue_ids (const svalue_id_map &map);
   void remap_region_ids (const region_id_map &map);
 
@@ -1799,6 +1798,11 @@ class region_model
     return m_constraints;
   }
 
+  const constraint_manager *get_constraints () const
+  {
+    return m_constraints;
+  }
+
   void get_descendents (region_id rid, region_id_set *out,
 			region_id exclude_rid) const;
 
@@ -2022,34 +2026,6 @@ struct svalue_id_merger_mapping
   one_way_svalue_id_map m_map_from_b_to_m;
 };
 
-/* A bundle of data used when canonicalizing a region_model so that the
-   order of regions and svalues is in a predictable order (thus increasing
-   the chance of two region_models being equal).
-
-   This object is used to keep track of a recursive traversal across the
-   svalues and regions within the model, made in a deterministic order,
-   assigning new ids the first time each region or svalue is
-   encountered.  */
-
-struct canonicalization
-{
-  canonicalization (const region_model &model);
-  void walk_rid (region_id rid);
-  void walk_sid (svalue_id sid);
-
-  void dump_to_pp (pretty_printer *pp) const;
-  void dump (FILE *fp) const;
-  void dump () const;
-
-  const region_model &m_model;
-  /* Maps from existing IDs to new IDs.  */
-  region_id_map m_rid_map;
-  svalue_id_map m_sid_map;
-  /* The next IDs to hand out.  */
-  int m_next_rid_int;
-  int m_next_sid_int;
-};
-
 } // namespace ana
 
 namespace inchash
-- 
2.21.0

