> > 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.
I agree that hash calculation needs fine tuning (and we want to be sure
we do have checking that hasher agrees with comparer).
I did not look in detail into the Martin's patch. The idea of hashing
access type at WPA time came from our discussion.
If you make statistics, the dominating reason for giving up compare
after reading body is mismatch in memory operands. This is extremely
comon for instances of templates where functions often have ideantical
body except that basetypes of all accesses are different.
Here we get same hash and eventually call operand_equal_p which walks
whole path and calls types_compatible_p that eventually finds the
basetype and returns false comparing TYPE_CANONICAL of both.
Because TYPE_CANONICAL is not known at compile time I was considering
two options: break out canonical type hashes out of lto-common.c or
simply update the hash at compile time.
Well, this all is aimed to solve copmile time/memory use problem caused
by not merging these functions. Of course, we ought to merge. I think
we should not use operand_equal_p for memory references and implement
separate comparsion that does two separate things:
1) compare semantics of the access path (i.e. how big chunk of memory
with what alignment is read)
For constant offset addresses this is basicaly
get_ref_base_and-offset
2) what additional info tree-ssa-alias can use.
(dependence clique, ref alias type, base alias type and the chain of
refs used by nonoverlapping_refs_p and friends).
I have prototype patch for that somewhere.
For ADDR_EXPR we are basically interested in base and offset only.
Honza
>
> 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(-)
> > >
> > >
