analyzer: avoid calling binding_map::const_iterator::operator* [PR123145]

PR analyzer/123145 tracks a large slowdown seen in -fanalyzer on
a particular test case in qemu.

Profiling showed a large amount of time being spent iterating through
binding maps with binding_map::const_iterator::operator*, due to the
work spent converting from bit_range to concrete_key via
store_manager::get_concrete_binding.

Many of the iterations where this done are merely looking at the bound
svalues, not the keys, so this work is wasted.

This patch updates these iterations to avoid needing to do work on the
keys.

Crude benchmarking (on a debug, not release build) showed a speedup on
the test case, from 3 hours to 2.2 hours.

No functional change intended.

gcc/analyzer/ChangeLog:
	PR analyzer/123145
	* program-state.cc (sm_state_map::impl_set_state): Update
	iteration to avoid looking up binding_key values.
	* region-model-reachability.cc (reachable_regions::handle_sval):
	Use iter.get_svalue.
	(reachable_regions::handle_parm): Likewise.
	* region-model.cc (iterable_cluster::iterable_cluster): Update
	iteration to avoid looking up binding_key values.
	(iterable_cluster::dump_to_pp): Likewise.
	(exposure_through_uninit_copy::calc_num_uninit_bits): Likewise.
	(exposure_through_uninit_copy::complain_about_uninit_ranges):
	Likewise.
	(contains_uninit_p): Likewise.
	* store.cc (binding_map::hash): Likewise.
	* store.h (bit_range::hash): New, based on...
	(concrete_binding::hash): ...this.  Reimplement using the above.
	(binding_map::const_iterator::get_svalue): New decl.
	(binding_map::get_symbolic_bindings): New accessor.
	(binding_map::for_each_value): Update iteration to avoid looking
	up binding_key values.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>

diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index 048d992e5751a068ec285a406cf4df375eb731ca..08d6b5599504f5298778ce521fbb2e2dfd4b0b8a 100644 (file)
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -562,9 +562,10 @@ sm_state_map::impl_set_state (const svalue *sval,
     {
       if (const compound_svalue *compound_sval
            = sval->dyn_cast_compound_svalue ())
-       for (auto iter : *compound_sval)
+       for (auto iter = compound_sval->begin ();
+            iter != compound_sval->end (); ++iter)
          {
-           const svalue *inner_sval = iter.m_sval;
+           const svalue *inner_sval = iter.get_svalue ();
            if (inner_sval->can_have_associated_state_p ())
              impl_set_state (inner_sval, state, origin, ext_state);
          }

diff --git a/gcc/analyzer/region-model-reachability.cc b/gcc/analyzer/region-model-reachability.cc
index fc4583406db76270868b8d4bfcaed0607ae0afe9..19b96b982809b40dbb5c29f460bf81af5cedb970 100644 (file)
--- a/gcc/analyzer/region-model-reachability.cc
+++ b/gcc/analyzer/region-model-reachability.cc
@@ -172,7 +172,7 @@ reachable_regions::handle_sval (const svalue *sval)
       for (auto iter = compound_sval->begin ();
           iter != compound_sval->end (); ++iter)
        {
-         const svalue *iter_sval = (*iter).m_sval;
+         const svalue *iter_sval = iter.get_svalue ();
          handle_sval (iter_sval);
        }
     }
@@ -238,7 +238,7 @@ reachable_regions::handle_parm (const svalue *sval, tree param_type)
       for (auto iter = compound_sval->begin ();
           iter != compound_sval->end (); ++iter)
        {
-         const svalue *iter_sval = (*iter).m_sval;
+         const svalue *iter_sval = iter.get_svalue ();
          handle_sval (iter_sval);
        }
     }

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index be7b3d7b95c0a7ee57a4e72f02d664bd02c99f89..c6b22706c7b1687df8dc148eda736579a66fc6a3 100644 (file)
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -4509,21 +4509,17 @@ public:
   {
     if (!cluster)
       return;
-    for (auto iter : *cluster)
+    for (auto iter : cluster->get_map ().get_concrete_bindings ())
       {
-       const binding_key *key = iter.m_key;
-       const svalue *sval = iter.m_sval;
+       const bit_range &bits = iter.first;
+       const svalue *sval = iter.second;
 
-       if (const concrete_binding *concrete_key
-           = key->dyn_cast_concrete_binding ())
-         {
-           byte_range fragment_bytes (0, 0);
-           if (concrete_key->get_byte_range (&fragment_bytes))
-             m_fragments.safe_push (fragment (fragment_bytes, sval));
-         }
-       else
-         m_symbolic_bindings.safe_push (key);
+       byte_range fragment_bytes (0, 0);
+       if (bits.as_byte_range (&fragment_bytes))
+         m_fragments.safe_push (fragment (fragment_bytes, sval));
       }
+    for (auto iter : cluster->get_map ().get_symbolic_bindings ())
+      m_symbolic_bindings.safe_push (iter);
     m_fragments.qsort (fragment::cmp_ptrs);
   }
 
@@ -4562,14 +4558,14 @@ public:
       {
        if (&iter != m_symbolic_bindings.begin ())
          pp_string (pp, ", ");
-       (*iter).dump_to_pp (pp, true);
+       iter.m_region->dump_to_pp (pp, true);
       }
     pp_string (pp, "])");
   }
 
 private:
   auto_vec<fragment> m_fragments;
-  auto_vec<const binding_key *> m_symbolic_bindings;
+  auto_vec<binding_map::symbolic_binding> m_symbolic_bindings;
 };
 
 /* Simulate reading the bytes at BYTES from BASE_REG.
@@ -7122,18 +7118,15 @@ private:
            = as_a <const compound_svalue *> (m_copied_sval);
          bit_size_t result = 0;
          /* Find keys for uninit svals.  */
-         for (auto iter : *compound_sval)
+         for (auto iter : compound_sval->get_map ().get_concrete_bindings ())
            {
-             const svalue *sval = iter.m_sval;
+             const svalue *sval = iter.second;
              if (const poisoned_svalue *psval
                  = sval->dyn_cast_poisoned_svalue ())
                if (psval->get_poison_kind () == poison_kind::uninit)
                  {
-                   const binding_key *key = iter.m_key;
-                   const concrete_binding *ckey
-                     = key->dyn_cast_concrete_binding ();
-                   gcc_assert (ckey);
-                   result += ckey->get_size_in_bits ();
+                   const bit_range &bits = iter.first;
+                   result += bits.m_size_in_bits;
                  }
            }
          return result;
@@ -7173,23 +7166,15 @@ private:
        = m_copied_sval->dyn_cast_compound_svalue ())
       {
        /* Find keys for uninit svals.  */
-       auto_vec<const concrete_binding *> uninit_keys;
-       for (auto iter : *compound_sval)
+       auto_vec<bit_range> uninit_bit_ranges;
+       for (auto iter : compound_sval->get_map ().get_concrete_bindings ())
          {
-           const svalue *sval = iter.m_sval;
+           const svalue *sval = iter.second;
            if (const poisoned_svalue *psval
                = sval->dyn_cast_poisoned_svalue ())
              if (psval->get_poison_kind () == poison_kind::uninit)
-               {
-                 const binding_key *key = iter.m_key;
-                 const concrete_binding *ckey
-                   = key->dyn_cast_concrete_binding ();
-                 gcc_assert (ckey);
-                 uninit_keys.safe_push (ckey);
-               }
+               uninit_bit_ranges.safe_push (iter.first);
          }
-       /* Complain about them in sorted order.  */
-       uninit_keys.qsort (concrete_binding::cmp_ptr_ptr);
 
        std::unique_ptr<record_layout> layout;
 
@@ -7203,11 +7188,11 @@ private:
          }
 
        unsigned i;
-       const concrete_binding *ckey;
-       FOR_EACH_VEC_ELT (uninit_keys, i, ckey)
+       bit_range *bits;
+       FOR_EACH_VEC_ELT (uninit_bit_ranges, i, bits)
          {
-           bit_offset_t start_bit = ckey->get_start_bit_offset ();
-           bit_offset_t next_bit = ckey->get_next_bit_offset ();
+           bit_offset_t start_bit = bits->get_start_bit_offset ();
+           bit_offset_t next_bit = bits->get_next_bit_offset ();
            complain_about_uninit_range (loc, start_bit, next_bit,
                                         layout.get ());
          }
@@ -7389,11 +7374,12 @@ contains_uninit_p (const svalue *sval)
        const compound_svalue *compound_sval
          = as_a <const compound_svalue *> (sval);
 
-       for (auto iter : *compound_sval)
+       for (auto iter = compound_sval->begin ();
+            iter != compound_sval->end (); ++iter)
          {
-           const svalue *sval = iter.m_sval;
+           const svalue *inner_sval = iter.get_svalue ();
            if (const poisoned_svalue *psval
-               = sval->dyn_cast_poisoned_svalue ())
+               = inner_sval->dyn_cast_poisoned_svalue ())
              if (psval->get_poison_kind () == poison_kind::uninit)
                return true;
          }

diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
index b11a81c39a4ee153326e792b6c8ea9dc439d926d..878536eb42b4df3bbb2ee01d052ad7d6b9a10f69 100644 (file)
--- a/gcc/analyzer/store.cc
+++ b/gcc/analyzer/store.cc
@@ -658,6 +658,18 @@ binding_map::const_iterator::operator* ()
     }
 }
 
+const svalue *
+binding_map::const_iterator::get_svalue () const
+{
+  if (m_concrete != m_map.m_concrete.end ())
+    return m_concrete->second;
+  else
+    {
+      gcc_assert (m_symbolic != m_map.m_symbolic.end ());
+      return m_symbolic->m_sval;
+    }
+}
+
 /* class binding_map::iterator.  */
 
 bool
@@ -754,12 +766,19 @@ hashval_t
 binding_map::hash () const
 {
   hashval_t result = 0;
-  for (auto iter : *this)
+  for (auto iter : m_concrete)
+    {
+      result ^= iter.first.hash ();
+      inchash::hash hstate;
+      hstate.add_ptr (iter.second);
+      result ^= hstate.end ();
+    }
+  for (auto iter : m_symbolic)
     {
       /* Use a new hasher for each key to avoid depending on the ordering
         of keys when accumulating the result.  */
       inchash::hash hstate;
-      hstate.add_ptr (iter.m_key);
+      hstate.add_ptr (iter.m_region);
       hstate.add_ptr (iter.m_sval);
       result ^= hstate.end ();
     }

diff --git a/gcc/analyzer/store.h b/gcc/analyzer/store.h
index 3c2be4a76b07c612121ca381059f0982cc49f382..e562e5479ed03a00141fdfe19d3883c3674123c4 100644 (file)
--- a/gcc/analyzer/store.h
+++ b/gcc/analyzer/store.h
@@ -309,6 +309,14 @@ struct bit_range
     return (m_size_in_bits < other.m_size_in_bits);
   }
 
+  hashval_t hash () const
+  {
+    inchash::hash hstate;
+    hstate.add_wide_int (m_start_bit_offset);
+    hstate.add_wide_int (m_size_in_bits);
+    return hstate.end ();
+  }
+
   bit_offset_t m_start_bit_offset;
   bit_size_t m_size_in_bits;
 };
@@ -396,13 +404,7 @@ public:
   }
   bool concrete_p () const final override { return true; }
 
-  hashval_t hash () const
-  {
-    inchash::hash hstate;
-    hstate.add_wide_int (m_bit_range.m_start_bit_offset);
-    hstate.add_wide_int (m_bit_range.m_size_in_bits);
-    return hstate.end ();
-  }
+  hashval_t hash () const { return m_bit_range.hash (); }
   bool operator== (const concrete_binding &other) const
   {
     return m_bit_range == other.m_bit_range;
@@ -567,6 +569,7 @@ public:
     const_iterator &operator++ ();
 
     binding_pair operator* ();
+    const svalue *get_svalue () const;
 
   private:
     const binding_map &m_map;
@@ -662,6 +665,9 @@ public:
   const concrete_bindings_t &
   get_concrete_bindings () const { return m_concrete; }
 
+  const symbolic_bindings_t &
+  get_symbolic_bindings () const { return m_symbolic; }
+
 private:
   void get_overlapping_bindings (const binding_key *key,
                                 auto_vec<const binding_key *> *out);
@@ -746,8 +752,8 @@ public:
   void for_each_value (void (*cb) (const svalue *sval, T user_data),
                       T user_data) const
   {
-    for (auto iter : m_map)
-      cb (iter.m_sval, user_data);
+    for (auto iter = m_map.begin (); iter != m_map.end (); ++iter)
+      cb (iter.get_svalue (), user_data);
   }
 
   static bool can_merge_p (const binding_cluster *cluster_a,