On Jan  5, 2017, Jakub Jelinek <ja...@redhat.com> wrote:

> OPTIMIZATION_NODE is created by saving options, computing hash
> (cl_option_hasher::hash apparently for OPTIMIZATION_NODE does not
> use cl_optimization_hash, why?), comparing (no cl_optimization_eq,
> just memcmp for equality) and if there is a match, use it instead.
> And on the IPA-ICF side, it uses cl_optimization_hash for hash
> computation and again memcmp for equality.

You seem to be looking at it from the equality/unequality perspective.
As far as that is concerned, the hash changes make little difference
indeed: what was different remains different, and what's the same
remains the same, maybe with a few extra collisions because certain
features are no longer regarded as part of the hash.  But remember that,
even if two different hashed elements map to the same hash number, they
remain different, and things work just as before (as long as there
aren't too many collisions, in which case performance may suffer, but
that's not the case at hand).

What needed changing as far as I was concerned, however, was not
equality of code units (for want of a better name for the combination of
code and compile options) within one translation group (possibly
multiple units), but rather iteration order on the hash table when
comparing two compilations of the same translation group with slightly
different options that should still generate the same executable code.

So, even if the option sets would still compare different in the end, by
disregarding some of them in computing the hashes, we get the same
hashes in the hash table, and thus the same order of iteration in the
hash table.

As for potentially harmful collisions, I pose we won't have any.  We
already had to have something to deal with two incoming functions that
were identical except for compile options, and to resolve them so that
we retained only one of them since compile options don't get to their
name mangling.  It just so happens that now some of these pairs that
should be collapsed into a single function will have the same hash.
That shouldn't make any difference for the code that deals with this
problem.

> Or tweak the PerFunction opts more and have some extra flag somewhere
> whether the value of the option is default (coming from command line
> options) or not (and use that default flag in cl_optimization_{hash,eq}
> such that options with default flag hash the same no matter what their
> value is and compare unequal to all the non-default options.

That wouldn't help with the problem I'm trying to solve.  If say
explicit -g vs -g -gtoggle, or -g -fvar-tracking-assignments-toggle were
to produce different code, be they "explicit" flags passed through
-fcompare-debug=<option> or given in the command line to debug one such
codegen difference, it would make it more difficult to debug such
issues, but not help avoid the codegen divergences that we don't want
debug-only options, implicit or explicit, to cause.

> This would mean IPA-ICF would not be able to merge a function without
> explicit optimize ("fvar-tracking*") or optimize ("fno-var-tracking*")
> attribute with one that has such an attribute.

If it is to merge them, it must be able to do so already, before and
after the hash change, because they have different hashes *and* their
option sets compare different.  If it's not to merge them, their having
the same hash shouldn't make a difference because the options compare
different in spite of the same hash.  Makes sense?

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Reply via email to