On Fri, Nov 29, 2019 at 1:17 PM Richard Biener <richard.guent...@gmail.com> wrote: > > 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?
Oh, and it doesn't consider dependences at all but seems to hash a functions stmts as independent? I'd have hashed SSA names as the hash of their def stmt to factor that in. 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(-) > > > >