From: David Malcolm Date: Mon, 26 Jan 2026 23:57:51 +0000 (-0500) Subject: analyzer: avoid calling binding_map::const_iterator::operator* [PR123145] X-Git-Tag: basepoints/gcc-17~1751 X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=c28dd8467b1e35219b8d74c5ff96c43d49eb9612;p=thirdparty%2Fgcc.git 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 --- diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc index 048d992e575..08d6b559950 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 fc4583406db..19b96b98280 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 be7b3d7b95c..c6b22706c7b 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 m_fragments; - auto_vec m_symbolic_bindings; + auto_vec m_symbolic_bindings; }; /* Simulate reading the bytes at BYTES from BASE_REG. @@ -7122,18 +7118,15 @@ private: = as_a (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 uninit_keys; - for (auto iter : *compound_sval) + auto_vec 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 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 (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 b11a81c39a4..878536eb42b 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 3c2be4a76b0..e562e5479ed 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 *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,