Re: [-fcompare-debug] var tracking options are not optimization options

2019-02-13 Thread Ian Lance Taylor 
> 2019-02-13  Ian Lance Taylor  
>
> * optc-save-gen.awk: Set var_opt_hash for initial optimizations
> and set current index for other optimizations.
>
> 2019-02-13  Ian Lance Taylor  
>
> * gcc.dg/func-attr-1.c: New test.

I went ahead and committed this patch.

Ian

Re: [-fcompare-debug] var tracking options are not optimization options

2019-02-13 Thread Ian Lance Taylor 
On Wed, Jan 4, 2017 at 8:08 PM Alexandre Oliva  wrote:
>
> On Jan  4, 2017, Alexandre Oliva  wrote:
>
> > So I guess we need some alternate PerFunction option flag that makes
> > it per-function, but not part of the ICF hash?
>
> Like this...
>
> 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?
>
> for  gcc/ChangeLog
>
> * doc/options.texi (PerFunction): New.
> * opt-functions.awk (switch_flags): Map both Optimization and
> PerFunction to CL_OPTIMIZATION.
> * opth-gen.awk: Test for PerFunction flag along with
> Optimization.
> * optc-save-gen.awk: Likewise.  Introduce var_opt_hash and set
> it only when the latter is present.  Skip those that don't in
> the hash function generator.
> * common.opt (fvar-tracking): Mark as PerFunction instead of
> Optimization.
> (fvar-tracking-assignments): Likewise.
> (fvar-tracking-assignments-toggle): Likewise.
> (fvar-tracking-uninit): Likewise.


> diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk
> index 8ce4248..0928d5d 100644
> --- a/gcc/optc-save-gen.awk
> +++ b/gcc/optc-save-gen.awk
> @@ -100,7 +100,7 @@ var_opt_range["optimize_debug"] = "0, 1";
>  # cache.
>
>  for (i = 0; i < n_opts; i++) {
> -   if (flag_set_p("Optimization", flags[i])) {
> +   if (flag_set_p("(Optimization|PerFunction)", flags[i])) {
> name = var_name(flags[i])
> if(name == "")
> continue;
> @@ -743,7 +743,7 @@ var_opt_val[2] = "x_optimize_debug"
>  var_opt_val_type[1] = "char "
>  var_opt_val_type[2] = "char "
>  for (i = 0; i < n_opts; i++) {
> -   if (flag_set_p("Optimization", flags[i])) {
> +   if (flag_set_p("(Optimization|PerFunction)", flags[i])) {
> name = var_name(flags[i])
> if(name == "")
> continue;
> @@ -756,6 +756,7 @@ for (i = 0; i < n_opts; i++) {
> otype = var_type_struct(flags[i])
> var_opt_val_type[n_opt_val] = otype;
> var_opt_val[n_opt_val++] = "x_" name;
> +   var_opt_hash[n_opt_val] = flag_set_p("Optimization", 
> flags[i]);
> }
>  }
>  print "";


This patch was committed a couple of years ago, but it's buggy in two
ways.  The var_opt_hash array is not set for the three flags that
precede this loop (x_optimize, x_optimize_size, and x_optimize_debug).
And, as you can see by noticing the position of n_opt_val++ above,
it's set for the wrong flag for the other cases--it's always set for
the next flag, not the current one.  This causes the hash and equality
comparisons of cl_optimization_eq to do the wrong thing.  It's hard to
notice, because in practice there tend to be other changes that affect
the result.  I was able to construct a (rather fragile) test case that
shows the problem, in which the compiler thinks that two sets of
function optimization attributes are the same when using the hash
table in build_optimization_node, although one is -O2 and one is -Os.

Any objections to my committing this patch?  Bootstrapped and tested
on x86_64-pc-linux-gnu.

Ian

2019-02-13  Ian Lance Taylor  

* optc-save-gen.awk: Set var_opt_hash for initial optimizations
and set current index for other optimizations.

2019-02-13  Ian Lance Taylor  

* gcc.dg/func-attr-1.c: New test.
Index: optc-save-gen.awk
===
--- optc-save-gen.awk   (revision 268833)
+++ optc-save-gen.awk   (working copy)
@@ -770,8 +770,11 @@
 n_opt_val = 3;
 var_opt_val[0] = "x_optimize"
 var_opt_val_type[0] = "char "
+var_opt_hash[0] = 1;
 var_opt_val[1] = "x_optimize_size"
+var_opt_hash[1] = 1;
 var_opt_val[2] = "x_optimize_debug"
+var_opt_hash[2] = 1;
 var_opt_val_type[1] = "char "
 var_opt_val_type[2] = "char "
 for (i = 0; i < n_opts; i++) {
@@ -787,8 +790,9 @@
 
otype = var_type_struct(flags[i])
var_opt_val_type[n_opt_val] = otype;
-   var_opt_val[n_opt_val++] = "x_" name;
+   var_opt_val[n_opt_val] = "x_" name;
var_opt_hash[n_opt_val] =

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-06 Thread Martin Liška 
On 01/06/2017 11:07 AM, Richard Biener wrote:
> On January 6, 2017 3:49:54 AM GMT+01:00, Alexandre Oliva  
> wrote:
>> On Jan  5, 2017, Jakub Jelinek  wrote:
>>
>>> You've just changed the hash function and my mail was about the fact
>> that
>>> it is not enough.
>>
>> Sorry, it wasn't clear 'enough for what'.  It's enough to fix the
>> bug/symptom I had observed and intended to fix, but yes, there is
>> another latent -fcompare-debug bug with a very different symptom that
>> the patch did not even attempt to address (because I had not even
>> realized we had that bug ;-)
>>
>>> With your patch, both functions
>>> will hash the same (that is ok), but as ICF for functions that hash
>> the same
>>> also performs memcmp on the OPTIMIZATION_NODE content, they will
>> compare
>>> the same in one case (-g0) and not in the other one (-g) and so ICF
>> will
>>> merge them in one case and not in the other one.
>>
>> Whereas without the proposed patch, ICF will also merge them in one
>> case
>> and not in the other.  The patch does not change that, it just
>> introduces a situation in which hashes that used to be different become
>> the same, but still without a match for merging.
>>
>>> BTW, seems the above exact case ICEs actually.
>>
>> Both with and without the proposed patch, I suppose.
>>
>> Anyway, would you please file a bug report about this, and copy me?  I
>> might be able to have a look into this one when I get a chance.
>>
>>> We need to deal not just with the iteration order, but also with
>> equality
>>> of functions, otherwise ICF will do something depending on -g vs.
>> -g0.
>>
>> Agreed.  But IMHO that's two different, unrelated bugs.  Maybe more ;-)
> 
> But can we perhaps solve the iteration order issue by sorting the candidates 
> after sth more stable then, like their DECL_UID?
> 
> Richard.
> 

Yes, I'm working on a patch for that.

M.

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-06 Thread Martin Liška 
On 01/06/2017 10:46 AM, Jakub Jelinek wrote:
> On Fri, Jan 06, 2017 at 12:49:54AM -0200, Alexandre Oliva wrote:
>> On Jan  5, 2017, Jakub Jelinek  wrote:
>>
>>> You've just changed the hash function and my mail was about the fact that
>>> it is not enough.
>>
>> Sorry, it wasn't clear 'enough for what'.  It's enough to fix the
>> bug/symptom I had observed and intended to fix, but yes, there is
>> another latent -fcompare-debug bug with a very different symptom that
>> the patch did not even attempt to address (because I had not even
>> realized we had that bug ;-)
> 
> Ok, let's do it incrementally then.  Your patch is ok for trunk, and we'll
> work on the extra stuff later.
> 
> BTW, the ICF hash traversal is what I've been trying to partly solve in
> PR78211, but already in the patch submission mentioned there is at least
> another hash table traversal that still would need adjustments.
> 
>   Jakub
> 

Hi.

Sorry for bigger latency. I'm going to fix the traversal and I was also 
thinking about
PerFunction and Target optimization flags. Currently, one can have a pair of 
functions
(one having for instance -fvar-tracking, second w/o) and use -fcompare-debug.
This is covered by ICF as it compares function attributes and one function will 
have
__attribute__ ((optimize(("-fno-var-tracking". Second issue can happen with 
LTO,
where one compilation unit can have -fvar-tracking and another not. To be 
honest, I'm
unable to trigger divergence in ICF by adding GCC_COMPARE_DEBUG=1 to env 
variables.

Ideal solution (next stage1) would be to not compare any 'target' and 
'optimization'
attributes of a function. But rather compare just cl_optimization_node and 
cl_target_node
with special argument (as suggested by Jakub), that some PerFunction flags 
would be ignored
for comparison (all var_tracking_* can be put to this category). Which would 
work with -fcompare-debug.

Martin

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-06 Thread Richard Biener 
On January 6, 2017 3:49:54 AM GMT+01:00, Alexandre Oliva  
wrote:
>On Jan  5, 2017, Jakub Jelinek  wrote:
>
>> You've just changed the hash function and my mail was about the fact
>that
>> it is not enough.
>
>Sorry, it wasn't clear 'enough for what'.  It's enough to fix the
>bug/symptom I had observed and intended to fix, but yes, there is
>another latent -fcompare-debug bug with a very different symptom that
>the patch did not even attempt to address (because I had not even
>realized we had that bug ;-)
>
>> With your patch, both functions
>> will hash the same (that is ok), but as ICF for functions that hash
>the same
>> also performs memcmp on the OPTIMIZATION_NODE content, they will
>compare
>> the same in one case (-g0) and not in the other one (-g) and so ICF
>will
>> merge them in one case and not in the other one.
>
>Whereas without the proposed patch, ICF will also merge them in one
>case
>and not in the other.  The patch does not change that, it just
>introduces a situation in which hashes that used to be different become
>the same, but still without a match for merging.
>
>> BTW, seems the above exact case ICEs actually.
>
>Both with and without the proposed patch, I suppose.
>
>Anyway, would you please file a bug report about this, and copy me?  I
>might be able to have a look into this one when I get a chance.
>
>> We need to deal not just with the iteration order, but also with
>equality
>> of functions, otherwise ICF will do something depending on -g vs.
>-g0.
>
>Agreed.  But IMHO that's two different, unrelated bugs.  Maybe more ;-)

But can we perhaps solve the iteration order issue by sorting the candidates 
after sth more stable then, like their DECL_UID?

Richard.

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-06 Thread Jakub Jelinek 
On Fri, Jan 06, 2017 at 12:49:54AM -0200, Alexandre Oliva wrote:
> On Jan  5, 2017, Jakub Jelinek  wrote:
> 
> > You've just changed the hash function and my mail was about the fact that
> > it is not enough.
> 
> Sorry, it wasn't clear 'enough for what'.  It's enough to fix the
> bug/symptom I had observed and intended to fix, but yes, there is
> another latent -fcompare-debug bug with a very different symptom that
> the patch did not even attempt to address (because I had not even
> realized we had that bug ;-)

Ok, let's do it incrementally then.  Your patch is ok for trunk, and we'll
work on the extra stuff later.

BTW, the ICF hash traversal is what I've been trying to partly solve in
PR78211, but already in the patch submission mentioned there is at least
another hash table traversal that still would need adjustments.

Jakub

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-05 Thread Alexandre Oliva 
On Jan  5, 2017, Jakub Jelinek  wrote:

> You've just changed the hash function and my mail was about the fact that
> it is not enough.

Sorry, it wasn't clear 'enough for what'.  It's enough to fix the
bug/symptom I had observed and intended to fix, but yes, there is
another latent -fcompare-debug bug with a very different symptom that
the patch did not even attempt to address (because I had not even
realized we had that bug ;-)

> With your patch, both functions
> will hash the same (that is ok), but as ICF for functions that hash the same
> also performs memcmp on the OPTIMIZATION_NODE content, they will compare
> the same in one case (-g0) and not in the other one (-g) and so ICF will
> merge them in one case and not in the other one.

Whereas without the proposed patch, ICF will also merge them in one case
and not in the other.  The patch does not change that, it just
introduces a situation in which hashes that used to be different become
the same, but still without a match for merging.

> BTW, seems the above exact case ICEs actually.

Both with and without the proposed patch, I suppose.

Anyway, would you please file a bug report about this, and copy me?  I
might be able to have a look into this one when I get a chance.

> We need to deal not just with the iteration order, but also with equality
> of functions, otherwise ICF will do something depending on -g vs. -g0.

Agreed.  But IMHO that's two different, unrelated bugs.  Maybe more ;-)

-- 
Alexandre Oliva, freedom fighterhttp://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

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-05 Thread Jakub Jelinek 
On Thu, Jan 05, 2017 at 07:13:50PM -0200, Alexandre Oliva 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).

You've just changed the hash function and my mail was about the fact that
it is not enough.  Yes, it is needed, but not sufficient.
Consider:

int
foo (int x, int y)
{
  int z = x + y;
  return x * y;
}

__attribute__((optimize ("no-var-tracking"))) int
bar (int x, int y)
{
  int z = x + y;
  return x * y;
}

or something similar (dunno if you don't also need to do
"no-var-tracking-assignments" too).  With your patch, both functions
will hash the same (that is ok), but as ICF for functions that hash the same
also performs memcmp on the OPTIMIZATION_NODE content, they will compare
the same in one case (-g0) and not in the other one (-g) and so ICF will
merge them in one case and not in the other one.
BTW, seems the above exact case ICEs actually.

> 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.

We need to deal not just with the iteration order, but also with equality
of functions, otherwise ICF will do something depending on -g vs. -g0.

Jakub

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-05 Thread Alexandre Oliva 
On Jan  5, 2017, Jakub Jelinek  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= 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 fighterhttp://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

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-05 Thread Jakub Jelinek 
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

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-04 Thread Alexandre Oliva 
On Jan  4, 2017, Alexandre Oliva  wrote:

> So I guess we need some alternate PerFunction option flag that makes
> it per-function, but not part of the ICF hash?

Like this...

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?

for  gcc/ChangeLog

* doc/options.texi (PerFunction): New.
* opt-functions.awk (switch_flags): Map both Optimization and
PerFunction to CL_OPTIMIZATION.
* opth-gen.awk: Test for PerFunction flag along with
Optimization.
* optc-save-gen.awk: Likewise.  Introduce var_opt_hash and set
it only when the latter is present.  Skip those that don't in
the hash function generator.
* common.opt (fvar-tracking): Mark as PerFunction instead of
Optimization.
(fvar-tracking-assignments): Likewise.
(fvar-tracking-assignments-toggle): Likewise.
(fvar-tracking-uninit): Likewise.
---
 gcc/common.opt|8 
 gcc/doc/options.texi  |7 +++
 gcc/opt-functions.awk |2 +-
 gcc/optc-save-gen.awk |7 +--
 gcc/opth-gen.awk  |2 +-
 5 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index 1872d51..44f7557 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2601,7 +2601,7 @@ Common Undocumented Var(flag_use_linker_plugin)
 ; will be set according to optimize, debug_info_level and debug_hooks
 ; in process_options ().
 fvar-tracking
-Common Report Var(flag_var_tracking) Init(2) Optimization
+Common Report Var(flag_var_tracking) Init(2) PerFunction
 Perform variable tracking.
 
 ; Positive if we should track variables at assignments, negative if
@@ -2609,13 +2609,13 @@ Perform variable tracking.
 ; annotations.  When flag_var_tracking_assignments ==
 ; AUTODETECT_VALUE it will be set according to flag_var_tracking.
 fvar-tracking-assignments
-Common Report Var(flag_var_tracking_assignments) Init(2) Optimization
+Common Report Var(flag_var_tracking_assignments) Init(2) PerFunction
 Perform variable tracking by annotating assignments.
 
 ; Nonzero if we should toggle flag_var_tracking_assignments after
 ; processing options and computing its default.  */
 fvar-tracking-assignments-toggle
-Common Report Var(flag_var_tracking_assignments_toggle) Optimization
+Common Report Var(flag_var_tracking_assignments_toggle) PerFunction
 Toggle -fvar-tracking-assignments.
 
 ; Positive if we should track uninitialized variables, negative if
@@ -2623,7 +2623,7 @@ Toggle -fvar-tracking-assignments.
 ; annotations.  When flag_var_tracking_uninit == AUTODETECT_VALUE it
 ; will be set according to flag_var_tracking.
 fvar-tracking-uninit
-Common Report Var(flag_var_tracking_uninit) Optimization
+Common Report Var(flag_var_tracking_uninit) PerFunction
 Perform variable tracking and also tag variables that are uninitialized.
 
 ftree-vectorize
diff --git a/gcc/doc/options.texi b/gcc/doc/options.texi
index 87cf688..14c149e 100644
--- a/gcc/doc/options.texi
+++ b/gcc/doc/options.texi
@@ -430,6 +430,13 @@ This is an optimization option.  It should be shown as 
such in
 @code{Var} should be saved and restored when the optimization level is
 changed with @code{optimize} attributes.
 
+@item PerFunction
+This is an option that can be overridden on a per-function basis.
+@code{Optimization} implies @code{PerFunction}, but options that do not
+affect executable code generation may use this flag instead, so that the
+option is not taken into account in ways that might affect executable
+code generation.
+
 @item Undocumented
 The option is deliberately missing documentation and should not
 be included in the @option{--help} output.
diff --git a/gcc/opt-functions.awk b/gcc/opt-functions.awk
index 7cf5025..a4aeeda 100644
--- a/gcc/opt-functions.awk
+++ b/gcc/opt-functions.awk
@@ -105,7 +105,7 @@ function switch_flags (flags)
  test_flag("Undocumented", flags,  " | CL_UNDOCUMENTED") \
  test_flag("NoDWARFRecord", flags,  " | CL_NO_DWARF_RECORD") \
  test_flag("Warning", flags,  " | CL_WARNING") \
- test_flag("Optimization", flags,  " | CL_OPTIMIZATION")
+ test_flag("(Optimization|PerFunction)", flags,  " |

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-04 Thread Richard Biener 
On Wed, Jan 4, 2017 at 2:08 PM, Alexandre Oliva  wrote:
> On Jan  4, 2017, Richard Biener  wrote:
>
>> On Tue, Jan 3, 2017 at 6:29 AM, 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.
>
>> Is this because ICF simply compares all optimize/target options?
>
>> So it boils down to -g implying -fvar-tracking but -g0 not?
>
> Not quite.  It would be the same if -g or any other option that
> shouldn't affect the executable code output were used to compute that
> hash.  -g is not among those flags, why should flags that adds detail to
> it be?  (Really, -fvar-tracking should have been a -g subflag rather
> than an -f one)
>
>> I don't think this is ok -- 'Optimization' doesn't really mean optimization
>> but sth we can control per-function.  You essentially remove the ability
>> to disable var-tracking just for a single (bad) function.
>
> I see the proposed patch does that indeed.  There are other ways to
> accomplish that (disabling var-tracking for a single function), but I
> suppose a direct way is important.  So I guess we need some alternate
> PerFunction option flag that makes it per-function, but not part of the
> ICF hash?

Yeah, I suppose ICF cares about those flags for the same reason
the inliner does -- just the inliner does it more fine-grained.

I suppose "[un]setting" opt[12]->x_flag_var_tracking before/after
hash compute and comparison does the trick (with a big fat comment).

That said, the inliner has all the explicit check_maybe_up/down checks
looking for options that make semantic differences and honors
explicit attribute "optimized" 1:1.  But I guess that's nothing we can
change ICF to for GCC 7.

Martin?

Thanks,
Richard.

>
> --
> Alexandre Oliva, freedom fighterhttp://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

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-04 Thread Alexandre Oliva 
On Jan  4, 2017, Richard Biener  wrote:

> On Tue, Jan 3, 2017 at 6:29 AM, 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.

> Is this because ICF simply compares all optimize/target options?

> So it boils down to -g implying -fvar-tracking but -g0 not?

Not quite.  It would be the same if -g or any other option that
shouldn't affect the executable code output were used to compute that
hash.  -g is not among those flags, why should flags that adds detail to
it be?  (Really, -fvar-tracking should have been a -g subflag rather
than an -f one)

> I don't think this is ok -- 'Optimization' doesn't really mean optimization
> but sth we can control per-function.  You essentially remove the ability
> to disable var-tracking just for a single (bad) function.

I see the proposed patch does that indeed.  There are other ways to
accomplish that (disabling var-tracking for a single function), but I
suppose a direct way is important.  So I guess we need some alternate
PerFunction option flag that makes it per-function, but not part of the
ICF hash?

-- 
Alexandre Oliva, freedom fighterhttp://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

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-04 Thread Richard Biener 
On Tue, Jan 3, 2017 at 6:29 AM, 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.

Is this because ICF simply compares all optimize/target options?

So it boils down to -g implying -fvar-tracking but -g0 not?  Maybe we need a
third state then ("default")?

> 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 removes the Optimization marker from the var-tracking
> options, and adjusts the code that uses these flags to match.
>
> This fixes -fcompare-debug failures in numerous LTO testcases.
>
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  OK to install?

I don't think this is ok -- 'Optimization' doesn't really mean optimization
but sth we can control per-function.  You essentially remove the ability
to disable var-tracking just for a single (bad) function.

Richard.

> for  gcc/ChangeLog
>
> * common.opt (fvar-tracking): Drop Optimization flag.
> (fvar-tracking-assignments): Likewise.
> (fvar-tracking-assignments-toggle): Likewise.
> (fvar-tracking-uninit): Likewise.
> * tree-inline.c (insert_debug_decl_map): Adjust uses of
> flag_var_tracking_assignments.
> (remap_gimple_stmt): Likewise.
> (insert_init_debug_bind): Likewise.
> (reset_debug_bindings): Likewise.
> ---
>  gcc/common.opt|8 
>  gcc/tree-inline.c |9 -
>  2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 6ebaf9c..c4aad6f 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2644,7 +2644,7 @@ Common Undocumented Var(flag_use_linker_plugin)
>  ; will be set according to optimize, debug_info_level and debug_hooks
>  ; in process_options ().
>  fvar-tracking
> -Common Report Var(flag_var_tracking) Init(2) Optimization
> +Common Report Var(flag_var_tracking) Init(2)
>  Perform variable tracking.
>
>  ; Positive if we should track variables at assignments, negative if
> @@ -2652,13 +2652,13 @@ Perform variable tracking.
>  ; annotations.  When flag_var_tracking_assignments ==
>  ; AUTODETECT_VALUE it will be set according to flag_var_tracking.
>  fvar-tracking-assignments
> -Common Report Var(flag_var_tracking_assignments) Init(2) Optimization
> +Common Report Var(flag_var_tracking_assignments) Init(2)
>  Perform variable tracking by annotating assignments.
>
>  ; Nonzero if we should toggle flag_var_tracking_assignments after
>  ; processing options and computing its default.  */
>  fvar-tracking-assignments-toggle
> -Common Report Var(flag_var_tracking_assignments_toggle) Optimization
> +Common Report Var(flag_var_tracking_assignments_toggle)
>  Toggle -fvar-tracking-assignments.
>
>  ; Positive if we should track uninitialized variables, negative if
> @@ -2666,7 +2666,7 @@ Toggle -fvar-tracking-assignments.
>  ; annotations.  When flag_var_tracking_uninit == AUTODETECT_VALUE it
>  ; will be set according to flag_var_tracking.
>  fvar-tracking-uninit
> -Common Report Var(flag_var_tracking_uninit) Optimization
> +Common Report Var(flag_var_tracking_uninit)
>  Perform variable tracking and also tag variables that are uninitialized.
>
>  ftree-vectorize
> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index 0de0b89..19ef5ee 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -156,7 +156,7 @@ insert_debug_decl_map (copy_body_data *id, tree key, tree 
> value)
>if (!gimple_in_ssa_p (id->src_cfun))
>  return;
>
> -  if (!opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
> +  if (!flag_var_tracking_assignments)
>  return;
>
>if (!target_for_debug_bind (key))
> @@ -1376,8 +1376,7 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id)
>bool skip_first = false;
>gimple_seq stmts = NULL;
>
> -  if (is_gimple_debug (stmt)
> -  && !opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
> +  if (is_gimple_debug (stmt) && !flag_var_tracking_assignments)
>  return stmts;
>
>/* Begin by recognizing trees that we'll completely rewrite for the
> @@ -3046,7 +3045,7 @@ insert_init_debug_bind (copy_body_data *id,
>if (!gimple_in_ssa_p (id->src_cfun))
>  return NULL;
>
> -  if (!opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
> +  if (!flag_var_tracking_assignments)
>  return NULL;
>
>tracked_var = target_for_debug_bind (var);
> @@ -4379,7 +4378,7 @@ reset_debug_bindings (copy_body_data *id, 
> gimple_stmt_iterator gsi)
>if (!gimple_in_ssa_p (id->src_cfun))
>  return;
>
> -  if (!opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
> +  if

[-fcompare-debug] var tracking options are not optimization options

2017-01-02 Thread Alexandre Oliva 
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 removes the Optimization marker from the var-tracking
options, and adjusts the code that uses these flags to match.

This fixes -fcompare-debug failures in numerous LTO testcases.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  OK to install?

for  gcc/ChangeLog

* common.opt (fvar-tracking): Drop Optimization flag.
(fvar-tracking-assignments): Likewise.
(fvar-tracking-assignments-toggle): Likewise.
(fvar-tracking-uninit): Likewise.
* tree-inline.c (insert_debug_decl_map): Adjust uses of
flag_var_tracking_assignments.
(remap_gimple_stmt): Likewise.
(insert_init_debug_bind): Likewise.
(reset_debug_bindings): Likewise.
---
 gcc/common.opt|8 
 gcc/tree-inline.c |9 -
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index 6ebaf9c..c4aad6f 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2644,7 +2644,7 @@ Common Undocumented Var(flag_use_linker_plugin)
 ; will be set according to optimize, debug_info_level and debug_hooks
 ; in process_options ().
 fvar-tracking
-Common Report Var(flag_var_tracking) Init(2) Optimization
+Common Report Var(flag_var_tracking) Init(2)
 Perform variable tracking.
 
 ; Positive if we should track variables at assignments, negative if
@@ -2652,13 +2652,13 @@ Perform variable tracking.
 ; annotations.  When flag_var_tracking_assignments ==
 ; AUTODETECT_VALUE it will be set according to flag_var_tracking.
 fvar-tracking-assignments
-Common Report Var(flag_var_tracking_assignments) Init(2) Optimization
+Common Report Var(flag_var_tracking_assignments) Init(2)
 Perform variable tracking by annotating assignments.
 
 ; Nonzero if we should toggle flag_var_tracking_assignments after
 ; processing options and computing its default.  */
 fvar-tracking-assignments-toggle
-Common Report Var(flag_var_tracking_assignments_toggle) Optimization
+Common Report Var(flag_var_tracking_assignments_toggle)
 Toggle -fvar-tracking-assignments.
 
 ; Positive if we should track uninitialized variables, negative if
@@ -2666,7 +2666,7 @@ Toggle -fvar-tracking-assignments.
 ; annotations.  When flag_var_tracking_uninit == AUTODETECT_VALUE it
 ; will be set according to flag_var_tracking.
 fvar-tracking-uninit
-Common Report Var(flag_var_tracking_uninit) Optimization
+Common Report Var(flag_var_tracking_uninit)
 Perform variable tracking and also tag variables that are uninitialized.
 
 ftree-vectorize
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 0de0b89..19ef5ee 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -156,7 +156,7 @@ insert_debug_decl_map (copy_body_data *id, tree key, tree 
value)
   if (!gimple_in_ssa_p (id->src_cfun))
 return;
 
-  if (!opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
+  if (!flag_var_tracking_assignments)
 return;
 
   if (!target_for_debug_bind (key))
@@ -1376,8 +1376,7 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id)
   bool skip_first = false;
   gimple_seq stmts = NULL;
 
-  if (is_gimple_debug (stmt)
-  && !opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
+  if (is_gimple_debug (stmt) && !flag_var_tracking_assignments)
 return stmts;
 
   /* Begin by recognizing trees that we'll completely rewrite for the
@@ -3046,7 +3045,7 @@ insert_init_debug_bind (copy_body_data *id,
   if (!gimple_in_ssa_p (id->src_cfun))
 return NULL;
 
-  if (!opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
+  if (!flag_var_tracking_assignments)
 return NULL;
 
   tracked_var = target_for_debug_bind (var);
@@ -4379,7 +4378,7 @@ reset_debug_bindings (copy_body_data *id, 
gimple_stmt_iterator gsi)
   if (!gimple_in_ssa_p (id->src_cfun))
 return;
 
-  if (!opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
+  if (!flag_var_tracking_assignments)
 return;
 
   for (var = DECL_ARGUMENTS (id->src_fn);

-- 
Alexandre Oliva, freedom fighterhttp://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