https://gcc.gnu.org/g:c28dd8467b1e35219b8d74c5ff96c43d49eb9612
commit r16-7053-gc28dd8467b1e35219b8d74c5ff96c43d49eb9612 Author: David Malcolm <[email protected]> Date: Mon Jan 26 18:57:51 2026 -0500 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 <[email protected]> Diff: --- gcc/analyzer/program-state.cc | 5 ++- gcc/analyzer/region-model-reachability.cc | 4 +- gcc/analyzer/region-model.cc | 66 ++++++++++++------------------- gcc/analyzer/store.cc | 23 ++++++++++- gcc/analyzer/store.h | 24 ++++++----- 5 files changed, 67 insertions(+), 55 deletions(-) diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc index 048d992e5751..08d6b5599504 100644 --- 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 fc4583406db7..19b96b982809 100644 --- 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 be7b3d7b95c0..c6b22706c7b1 100644 --- 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 b11a81c39a4e..878536eb42b4 100644 --- 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 3c2be4a76b07..e562e5479ed0 100644 --- 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,
