From 8117528e8c561cc64f992109d792b084ac915738 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 18 Feb 2020 21:28:32 -0500
Subject: [PATCH 20/31] FIXME: more WIP on canonicalization/purging

---
 gcc/analyzer/constraint-manager.h |   5 +
 gcc/analyzer/region-model.cc      | 155 ++++++++++++++++++++++++------
 gcc/analyzer/region-model.h       |   6 +-
 3 files changed, 135 insertions(+), 31 deletions(-)

diff --git a/gcc/analyzer/constraint-manager.h b/gcc/analyzer/constraint-manager.h
index 916c45f928e..a4d179dcdcf 100644
--- a/gcc/analyzer/constraint-manager.h
+++ b/gcc/analyzer/constraint-manager.h
@@ -236,6 +236,11 @@ public:
 
   void canonicalize (unsigned num_svalue_ids);
 
+  void delete_redundant_ecs (const equiv_class_id_map &ec_id_map,
+			     equiv_class_id first_unused_ec_id,
+			     purge_stats *stats,
+			     logger *logger);
+
   static void merge (const constraint_manager &cm_a,
 		     const constraint_manager &cm_b,
 		     constraint_manager *out,
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 0916cae03f9..bbeeb54ae95 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1401,6 +1401,20 @@ region::remap_region_ids (const region_id_map &map)
   map.update (&m_active_view_rid);
 }
 
+/* Base implementation of region::walk_for_canonicalization vfunc; subclasses
+   should chain up to this.  */
+
+void
+region::walk_for_canonicalization (canonicalization *c) const
+{
+  c->walk_rid (m_parent_rid);
+
+  unsigned i;
+  region_id *view_rid;
+  FOR_EACH_VEC_ELT (m_view_rids, i, view_rid)
+    c->walk_rid (*view_rid);
+}
+
 /* Add a new region with id VIEW_RID as a view of this region.  */
 
 void
@@ -1537,9 +1551,9 @@ primitive_region::clone () const
    primitive_region.  */
 
 void
-primitive_region::walk_for_canonicalization (canonicalization *) const
+primitive_region::walk_for_canonicalization (canonicalization *c) const
 {
-  /* Empty.  */
+  region::walk_for_canonicalization (c);
 }
 
 /* class map_region : public region.  */
@@ -1955,6 +1969,8 @@ map_region::can_merge_p (const map_region *map_region_a,
 void
 map_region::walk_for_canonicalization (canonicalization *c) const
 {
+  region::walk_for_canonicalization (c);
+
   auto_vec<tree> keys (m_map.elements ());
   for (map_t::iterator iter = m_map.begin ();
        iter != m_map.end ();
@@ -2418,6 +2434,8 @@ array_region::key_cmp (const void *p1, const void *p2)
 void
 array_region::walk_for_canonicalization (canonicalization *c) const
 {
+  region::walk_for_canonicalization (c);
+
   auto_vec<int> keys (m_map.elements ());
   for (map_t::iterator iter = m_map.begin ();
        iter != m_map.end ();
@@ -2701,6 +2719,8 @@ stack_region::can_merge_p (const stack_region *stack_region_a,
 void
 stack_region::walk_for_canonicalization (canonicalization *c) const
 {
+  region::walk_for_canonicalization (c);
+
   int i;
   region_id *frame_rid;
   FOR_EACH_VEC_ELT (m_frame_rids, i, frame_rid)
@@ -2766,9 +2786,9 @@ heap_region::clone () const
    heap_region.  */
 
 void
-heap_region::walk_for_canonicalization (canonicalization *) const
+heap_region::walk_for_canonicalization (canonicalization *c) const
 {
-  /* Empty.  */
+  region::walk_for_canonicalization (c);
 }
 
 /* class root_region : public region.  */
@@ -3158,6 +3178,7 @@ root_region::add_to_hash (inchash::hash &hstate) const
 void
 root_region::walk_for_canonicalization (canonicalization *c) const
 {
+  region::walk_for_canonicalization (c);
   c->walk_rid (m_stack_rid);
   c->walk_rid (m_globals_rid);
   c->walk_rid (m_code_rid);
@@ -3219,9 +3240,9 @@ symbolic_region::clone () const
    symbolic_region.  */
 
 void
-symbolic_region::walk_for_canonicalization (canonicalization *) const
+symbolic_region::walk_for_canonicalization (canonicalization *c) const
 {
-  /* Empty.  */
+  region::walk_for_canonicalization (c);
 }
 
 /* class region_model.  */
@@ -6154,10 +6175,9 @@ region_model::canonicalize_and_purge (region_model_context *ctxt,
 	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
+  /* Drop constraints that refer to any ECs being purged, or that
+     purely compare constants.  */
   // FIXME: move to constraint_manager?
-  // FIXME: should we reorder them?
   for (unsigned con_idx = 0;
        con_idx < get_constraints ()->m_constraints.length (); )
     {
@@ -6176,22 +6196,8 @@ region_model::canonicalize_and_purge (region_model_context *ctxt,
   /* 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++;
-    }
+  get_constraints ()->delete_redundant_ecs (c.m_ec_id_map, first_unused_ec_id,
+					    stats, logger);
 
   /* Drop the redundant svalues from the end of the vector.  */
   while ((signed)m_svalues.length () > first_unused_sid.as_int ())
@@ -6223,9 +6229,70 @@ region_model::canonicalize_and_purge (region_model_context *ctxt,
 	stats->m_num_regions++;
     }
 
+  /* Some ECs could now be newly redundant; find them and purge them.  */
+  {
+    /* Find the ECs that are referenced by constraints.  */
+    equiv_class_id_map ec_id_map (get_constraints ()->get_num_equiv_classes ());
+    int next_ec_id_int = 0;
+    int i;
+    constraint *con;
+    FOR_EACH_VEC_ELT (get_constraints ()->m_constraints, i, con)
+      {
+	if (ec_id_map.get_dst_for_src (con->m_lhs).null_p ())
+	  ec_id_map.put (con->m_lhs, next_ec_id_int++);
+	if (ec_id_map.get_dst_for_src (con->m_rhs).null_p ())
+	  ec_id_map.put (con->m_rhs, next_ec_id_int++);
+      }
+    // FIXME: also, any that have more than one element!!
+
+    equiv_class_id first_unused_ec_id
+      = equiv_class_id::from_int (next_ec_id_int);
+
+    /* Move the rest.  */
+    equiv_class *ec;
+    FOR_EACH_VEC_ELT (get_constraints ()->m_equiv_classes, i, ec)
+      {
+	equiv_class_id ec_id (i);
+	if (ec_id_map.get_dst_for_src (ec_id).null_p ())
+	  ec_id_map.put (ec_id, next_ec_id_int++);
+      }
+
+    get_constraints ()->remap_equiv_class_ids (ec_id_map);
+
+    get_constraints ()->delete_redundant_ecs (ec_id_map, first_unused_ec_id,
+					      stats, logger);
+  }
+
   validate ();
 }
 
+/* Subroutine of region_model::canonicalize_and_purge.
+   Delete all ECs after FIRST_UNUSED_EC_ID.  */
+// FIXME: move this
+void
+constraint_manager::delete_redundant_ecs (const equiv_class_id_map &ec_id_map,
+				    equiv_class_id first_unused_ec_id,
+				    purge_stats *stats,
+				    logger *logger)
+{
+  /* Drop the redundant equiv_classes from the end of the vector.  */
+  while ((signed)m_equiv_classes.length ()
+	 > first_unused_ec_id.as_int ())
+    {
+      if (logger)
+	{
+	  equiv_class_id victim = equiv_class_id::from_int
+	    (m_equiv_classes.length () - 1);
+	  logger->log ("deleting ec%i (was ec%i)",
+		       victim.as_int (),
+		       ec_id_map.get_src_for_dst (victim).as_int ());
+	}
+      delete m_equiv_classes.pop ();
+      if (stats)
+	stats->m_num_equiv_classes++;
+    }
+}
+
 /* Renumber the svalues within this model according to MAP.  */
 
 void
@@ -7482,9 +7549,11 @@ test_purge_unused ()
   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
-     have been purged.  */
-  ASSERT_EQ (purged.m_num_equiv_classes, 1);
+  /* The redundant ECs for the old unknown value for x
+     and for constant 42 should have been purged.  */
+  ASSERT_EQ (purged.m_num_equiv_classes, 2);
+/* The redundant constraint that the old value of x != 42
+   and the constraint 0 < 42 should have been purged.  */
   ASSERT_EQ (purged.m_num_constraints, 2);
   ASSERT_EQ (model.get_constraints ()->m_constraints.length (), 1);
 
@@ -7827,6 +7896,35 @@ test_canonicalization_4 ()
   model.canonicalize_and_purge (NULL);
 }
 
+/* Verify that redundant ECs get purged during canonicalization.  */
+
+static void
+test_canonicalization_5 ()
+{
+  tree x = build_global_decl ("x", integer_type_node);
+
+  region_model model0;
+  model0.set_to_new_unknown_value (model0.get_lvalue (x, NULL),
+				   integer_type_node, NULL);
+
+  region_model model1 (model0);
+  /* Add redundant EC.  */
+  equiv_class &ec
+    = model1.get_constraints ()->get_equiv_class (model1.get_rvalue (x, NULL));
+  ASSERT_EQ (ec.get_representative (), model1.get_rvalue (x, NULL));
+
+  purge_stats stats0;
+  model0.canonicalize_and_purge (NULL, &stats0);
+
+  purge_stats stats1;
+  model1.canonicalize_and_purge (NULL, &stats1);
+  /* The redundant EC should have been purged.  */
+  ASSERT_EQ (stats1.m_num_equiv_classes, 1);
+
+  /* The models should be equal.  */
+  ASSERT_EQ (model0, model1);
+}
+
 /* Assert that if we have two region_model instances
    with values VAL_A and VAL_B for EXPR that they are
    mergable.  Write the merged model to *OUT_MERGED_MODEL,
@@ -8280,6 +8378,7 @@ analyzer_region_model_cc_tests ()
   test_canonicalization_2 ();
   test_canonicalization_3 ();
   test_canonicalization_4 ();
+  test_canonicalization_5 ();
   test_state_merging ();
   test_constraint_merging ();
 }
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 130561a5c61..5193ca73bbd 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -144,8 +144,8 @@ private:
 
 /* A class for renumbering IDs within a region_model, mapping old IDs
    to new IDs (e.g. when removing one or more elements, thus needing to
-   renumber).  */
-// TODO: could this be useful for equiv_class_ids?
+   renumber).
+   Also used for equiv_class_ids.  */
 
 template <typename T>
 class id_map
@@ -885,7 +885,7 @@ public:
   void remap_svalue_ids (const svalue_id_map &map);
   virtual void remap_region_ids (const region_id_map &map);
 
-  virtual void walk_for_canonicalization (canonicalization *c) const = 0;
+  virtual void walk_for_canonicalization (canonicalization *c) const;
 
   void add_view (region_id view_rid, region_model *model);
   region_id get_view (tree type, region_model *model) const;
-- 
2.21.0

