On Fri, Nov 29, 2019 at 12:37 PM Martin Liška <mli...@suse.cz> wrote: > > Hello. > > The patch is about streaming of at maximum 3 tree types that are > used for memory references in IPA ICF. That helps rapidly to reduce > number of function bodies loaded in WPA phase. Based on numbers for > Firefox we get from: > > Init called for 87557 items (23.30%). > ... > Totally needed symbols: 40580, fraction of loaded symbols: 46.35% > > to: > > Init called for 55844 items (14.86%). > ... > Totally needed symbols: 40580, fraction of loaded symbols: 72.67% > > Where memory peak drops from 5.7GB to 4.8GB and WPA is faster by 5 seconds. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed?
It looks like a hack. First of all you do quite some useless work since already the caller of walk_stmt_* could check for the vector length. second you are streaming a quite random type here with not much semantic value, plus 't' is already the base of the memory reference. To me it looks like the stmt hash is incredibly weak, for example not considering a GIMPLE_COND comparison code, doing weird stuff for commutative ops (the idea is probably to hash in both orders? but the implementation is completely off), but not considering the same for communative comparison ops (or swapped ops). To me it looks like if we want to hash types we could at least hash basic type properties plus for aggregates hash the ODR name if any such exists. > Thanks, > Martin > > gcc/ChangeLog: > > 2019-11-27 Martin Liska <mli...@suse.cz> > > PR ipa/92535 > * ipa-icf.c (sem_function::sem_function): Initialize > memory_access_types. > (record_memory_op_type): New function. > (sem_function::init): Walk memory accesses for GIMPLE > statements. > (sem_item_optimizer::write_summary): Stream memory_access_types. > (sem_item_optimizer::read_section): Read memory_access_types > and hash pointer to canonical types. > (sem_item_optimizer::execute): Update hash by memory > access type. > (sem_item_optimizer::update_hash_by_memory_access_type): > New. > * ipa-icf.h (memory_access_types): New. > (m_canonical_types_hash): Likewise. > (update_hash_by_memory_access_type): Likewise. > --- > gcc/ipa-icf.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++-- > gcc/ipa-icf.h | 10 +++++++++ > 2 files changed, 68 insertions(+), 2 deletions(-) > >
