On Thu, Jan 05, 2017 at 02:06:09AM -0200, Alexandre Oliva wrote:
> If we include them in the ICF hash, they may cause congruence_groups to
> be processed in a different order due to different hashes, which in turn
> causes different funcdef_nos to be assigned to functions. Since these
> numbers are included in -fcompare-debug dumps, they cause failures.
>
> Since these options are not optimization options, in that they do not
> (or should not, save for bugs like this) affect the executable code
> output by the compiler, they should not be marked as such.
>
> This patch replaces the Optimization flag in the var-tracking options
> with the newly-introduced PerFunction flag, so that it can still be
> controlled on a per-function basis, but that disregards it in the hash
> computation used by ICF.
>
> This fixes -fcompare-debug failures in numerous LTO testcases.
>
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu. Ok to install?
I'm not sure how does that work though.
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.
If your patch changes the behavior of cl_optimization_hash to ignore
the PerFunction flags, then if you are unlucky that still means
-fcompare-debug IPA-ICF issues (ICF might work differently with -g
vs. -g0).
So, I think either we change cl_optimization_hash to have an extra
bool parameter whether PerFunction counts or not, use it with true
in cl_option_hasher::hash and false in IPA-ICF (haven't looked at the LTO
streaming), and add cl_optimization_eq again with such a flag, and
use it with true in cl_option_hasher::equal and false in IPA-ICF instead of
the memcmp. Or we keep doing the bitwise hash computation and memcmp in
build_optimization_node, do (what I assume you do in your patch) in
cl_optimization_hash and add cl_optimization_eq that also ignores the
PerFunction options in the comparison and use that in IPA-ICF instead of
memcmp. That will have the undesirable effect that if you are unlucky and
have functions with optimize ("fvar-tracking*") and optimize
("fno-var-tracking*") attributes that IPA-ICF will just randomly pick one of
them, but I guess that is not a big deal and is quite unlikely.
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.
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. I guess this would be harder
to implement though.
Jakub
