[Bug lto/42088] New: flag_gtoggle in free_lang_data hides -fcompare-debug errors

[Bug lto/42088] New: flag_gtoggle in free_lang_data hides -fcompare-debug errors

[aoliva at gcc dot gnu dot org]

The codegen differences introduced by free_alng_data can be promptly exposed
using -fcompare-debug=-g0.

Two such errors are known to me, on x86_64-linux-gnu native bootstrap:

ada/einfo.adb fails -fcompare-debug=-g0 because boolean_type_node is modified.
http://gcc.gnu.org/ml/gcc-patches/2009-11/msg00414.html has details.

tree.c fails -fcompare-debug=-g0 because declaration UIDs go out of sync when
local declarations of types are removed in free_lang_data_in_block().
http://gcc.gnu.org/ml/gcc-patches/2009-11/msg00457.html has details.


-- 
           Summary: flag_gtoggle in free_lang_data hides -fcompare-debug
                    errors
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: lto
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: aoliva at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2009-11-18 10:28 -------
Hmpf, no testcases ...

./xgcc -B. -c tree.i -O -g -fcompare-debug=-g0

has a compare-debug failure (-O0 does not).  Fixed by

Index: gcc/tree.c
===================================================================
--- gcc/tree.c  (revision 154283)
+++ gcc/tree.c  (working copy)
@@ -4313,7 +4313,8 @@ free_lang_data_in_block (tree fn, tree b
   tp = &BLOCK_VARS (block);
   while (*tp)
     {
-      if (!pointer_set_contains (locals, *tp))
+      if (TREE_CODE (*tp) == VAR_DECL
+         && !pointer_set_contains (locals, *tp))
        *tp = TREE_CHAIN (*tp);
       else
        tp = &TREE_CHAIN (*tp);

Note that UID differences that do not change UID order are a red herring
IMHO and should be tracked down and fixed.  Any idea where this particular
one comes from?  Most of the time it's walking hashtables in hash order,
like FOR_EACH_REFERENCED_VAR does.  Relevant such walkings might be
in insert_phi_nodes, analyze_all_variable_accesses (both easy to fix,
but fixing them doesn't fix the debug miscompare).


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |rguenth at gcc dot gnu dot
                   |dot org                     |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2009-11-18 10:28:24
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2009-11-18 10:32 -------
Oh, the UIDs appear in the dumps you compare.  That's a bug in your comparer,
we never said different UIDs are not ok, only codegen differences are not
(and yes, UID _ordering_ differences have a great chance of causing them, but
mere differences are not).

Please fix your comparer.  The tree.c issue didn't show up with a regular
bootstrap-debug?  Or is that because of the flag_gtoggle check?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[aoliva at gcc dot gnu dot org]

------- Comment #3 from aoliva at gcc dot gnu dot org  2009-11-18 16:30 -------
Different UIDs affect hash table walks, which in turn may affect codegen. 
We've had several -g/-g0 codegen differences ultimately caused by decl UID
differences, and they were detected and fixed long before -fcompare-debug was
even introduced, i.e., they were detected with the compare-debug script that
compares actual generated code.

That's why -fcompare-debug must report such differences.  Even if the code ends
up the same in one particular testcase, supersets of that testcase would likely
diverge.  Disregarding the UIDs in -fcompare-debug would hide errors.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenther at suse dot de]

------- Comment #4 from rguenther at suse dot de  2009-11-18 16:51 -------
Subject: Re:  flag_gtoggle in free_lang_data hides -fcompare-debug
 errors

On Wed, 18 Nov 2009, aoliva at gcc dot gnu dot org wrote:

> ------- Comment #3 from aoliva at gcc dot gnu dot org  2009-11-18 16:30 -------
> Different UIDs affect hash table walks, which in turn may affect codegen. 
> We've had several -g/-g0 codegen differences ultimately caused by decl UID
> differences, and they were detected and fixed long before -fcompare-debug was
> even introduced, i.e., they were detected with the compare-debug script that
> compares actual generated code.
> 
> That's why -fcompare-debug must report such differences.  Even if the code ends
> up the same in one particular testcase, supersets of that testcase would likely
> diverge.  Disregarding the UIDs in -fcompare-debug would hide errors.

Sorry, but hashtable walks are always broken if codegen depends on them
and you do not ensure proper ordering.  Enforcing same UIDs for -g/-g0
just forces us to use more memory than necessary.

Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2009-11-18 16:53 -------
Btw, when I remove the flag_gtoggle check from free-lang-data we should get
bootstrap-debug comparison fails, no?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[aoliva at gcc dot gnu dot org]

------- Comment #6 from aoliva at gcc dot gnu dot org  2009-11-18 17:58 -------
decl UIDs are used in various expression hashes.  Enforcing strict ordering
among them without relying on UIDs would be an interesting problem, but if you
have a solution for that, go for it.  Disabling the printing of UIDs in
-fcompare-debug dumps will be the least of your problems.

The only bootstrap-debug failures I remember on x86_64-linux-gnu were in ada,
because of the boolean type.  Others required the stricter -fcompare-debug.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenther at suse dot de]

------- Comment #7 from rguenther at suse dot de  2009-11-18 19:50 -------
Subject: Re:  flag_gtoggle in free_lang_data hides -fcompare-debug
 errors

On Wed, 18 Nov 2009, aoliva at gcc dot gnu dot org wrote:

> ------- Comment #6 from aoliva at gcc dot gnu dot org  2009-11-18 17:58 -------
> decl UIDs are used in various expression hashes.  Enforcing strict ordering
> among them without relying on UIDs would be an interesting problem, but if you
> have a solution for that, go for it.  Disabling the printing of UIDs in
> -fcompare-debug dumps will be the least of your problems.

Expression hashes do not have to be ordered.  Nobody walks them and
generates code from them.  Instead they hash expressions and for
different UIDs the expressions better are different.  And we'd better
have only one same expression in a hash.

So I fail to see how expression hashes matter here at all.

Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[aoliva at gcc dot gnu dot org]

------- Comment #8 from aoliva at gcc dot gnu dot org  2009-11-18 20:05 -------
We used to do such things, using pointers rather than UIDs even, and I fixed a
number of such issues to avoid codegen differences.  But hey, if you could
prove that a DECL uid would never affect decisions taken by the compiler,
including decisions that might depend on the order in which you perform similar
tests over difference pieces of code, more power to you, don't let me stand on
your way.  Bonus points if you can devise a way to avoid such dependences on
UIDs from being accidentally introduced at a later time.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenther at suse dot de]

------- Comment #9 from rguenther at suse dot de  2009-11-18 20:52 -------
Subject: Re:  flag_gtoggle in free_lang_data hides -fcompare-debug
 errors

On Wed, 18 Nov 2009, aoliva at gcc dot gnu dot org wrote:

> ------- Comment #8 from aoliva at gcc dot gnu dot org  2009-11-18 20:05 -------
> We used to do such things, using pointers rather than UIDs even, and I fixed a
> number of such issues to avoid codegen differences.  But hey, if you could
> prove that a DECL uid would never affect decisions taken by the compiler,
> including decisions that might depend on the order in which you perform similar
> tests over difference pieces of code, more power to you, don't let me stand on
> your way.  Bonus points if you can devise a way to avoid such dependences on
> UIDs from being accidentally introduced at a later time.

This is what we have bootstrap-debug for.  Btw, I obviously cannot
"prove" anything.  But one could randomize the delta we add to
next_decl_uid - we should get bootstrap comparison errors very
reliably that way.  (I remember doing that once, but I don't remember
the outcome)

Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenth at gcc dot gnu dot org]

------- Comment #10 from rguenth at gcc dot gnu dot org  2009-11-19 14:40 -------
Btw I can reproduce the bootstrap comparison fail with Ada when removing the
bogus flag_gtoggle check.  Fixing the problem with canonicalize_cond_expr_cond
isn't the whole story appearantly - fixing it doesn't fix the miscompare.
Still fixing it makes sense anyway, so I'm currently testing a patch.

For not doing the boolean_type_node fixing we'd need to not pre-load the
streamer chache with (selected) builtin type nodes but instead stream them
with every unit and function and pass them through the merging machinery.
Basically make lto_get_common_nodes return an empty vector (that likely
requires us to register and fixup all builtin types in lto1 before starting
to stream in stuff - I likely have some patches doing this somewhere).

I know it's all sort-of a mess right now ... (still tracking down the
specific Ada problem is intersting).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[ebotcazou at gcc dot gnu dot org]

------- Comment #11 from ebotcazou at gcc dot gnu dot org  2009-11-19 15:11 -------
> For not doing the boolean_type_node fixing we'd need to not pre-load the
> streamer chache with (selected) builtin type nodes but instead stream them
> with every unit and function and pass them through the merging machinery.
> Basically make lto_get_common_nodes return an empty vector (that likely
> requires us to register and fixup all builtin types in lto1 before starting
> to stream in stuff - I likely have some patches doing this somewhere).

Can't boolean_type_node (and sizetypes) be special-cased somehow, since it
seems that the other builtin types aren't problematic?


-- 

ebotcazou at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenther at suse dot de]

------- Comment #12 from rguenther at suse dot de  2009-11-19 15:36 -------
Subject: Re:  flag_gtoggle in free_lang_data hides -fcompare-debug
 errors

On Thu, 19 Nov 2009, ebotcazou at gcc dot gnu dot org wrote:

> ------- Comment #11 from ebotcazou at gcc dot gnu dot org  2009-11-19 15:11 -------
> > For not doing the boolean_type_node fixing we'd need to not pre-load the
> > streamer chache with (selected) builtin type nodes but instead stream them
> > with every unit and function and pass them through the merging machinery.
> > Basically make lto_get_common_nodes return an empty vector (that likely
> > requires us to register and fixup all builtin types in lto1 before starting
> > to stream in stuff - I likely have some patches doing this somewhere).
> 
> Can't boolean_type_node (and sizetypes) be special-cased somehow, since it
> seems that the other builtin types aren't problematic?

The problem is that they are used in other builtin types (well, maybe not,
but that's a big problem with various pointer types).

I will look into getting the lto1 fixups into trunk (ISTR they fix
some corner-cases anyway), it should then be easy to experiment with
excluding single builtin types from the cache pre-loading.

Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[aoliva at gcc dot gnu dot org]

------- Comment #13 from aoliva at gcc dot gnu dot org  2009-11-19 18:54 -------
> one could randomize the delta we add to next_decl_uid

Yeah, that would be an interesting test.  In fact, we had something along these
lines before, when we used (randomized) pointers rather than uids, and we
failed.  I don't think anything was done to avoid this kind of failure, and
randomizing decl uids could get even those that passed because of uid stability
to fail compare-debug.  I guess we'll only know for sure when someone
undertakes it.

Thanks for looking into the lto/compare-debug regressions.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenth at gcc dot gnu dot org]

------- Comment #14 from rguenth at gcc dot gnu dot org  2009-11-24 14:16 -------
Created an attachment (id=19110)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19110&action=view)
candidate patch

Patch I was testing.  Unfortunately I get

Comparing stages 2 and 3
warning: gcc/cc1plus-checksum.o differs
warning: gcc/cc1obj-checksum.o differs
warning: gcc/cc1-checksum.o differs
warning: gcc/cc1objplus-checksum.o differs
Bootstrap comparison failure!
gcc/ipa-cp.o differs

I'm on and off on this bug, so if anybody wants to investigate the above ...


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenth at gcc dot gnu dot org]

------- Comment #15 from rguenth at gcc dot gnu dot org  2009-11-30 15:05 -------
The issue with the boolean_type_node is that the middle-end does not have
a type for a comparison result but implicitly assumes boolean_type_node.

So for

  D.xxxx_16 = (boolean) D.xxxx_6;
  if (D.xxxx_16 != 0)

with a re-set boolean_type_node we fold this to D.xxxx_6 != 0, with
the original Ada boolean_type_node we do not.  The C frontend for
decaying to bool always produces a comparison btw, not the truncation
we see above (and it only truncates to 8 bits).

In the end we want to unconditionally use a canonical boolean_type_node
in the middle-end, but this forces us to mangle all decls early (as
the mangler looks for boolean_type_node trees).

While the patch in comment #14 now bootstraps ok it has testsuite fails.

FAIL: g++.dg/pch/system-2.C -O2 -g assembly comparison

I think for 4.5 we should admit defeat and disable free-lang-data
unconditionally if not using LTO.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[ebotcazou at gcc dot gnu dot org]

------- Comment #16 from ebotcazou at gcc dot gnu dot org  2009-11-30 22:08 -------
> The issue with the boolean_type_node is that the middle-end does not have
> a type for a comparison result but implicitly assumes boolean_type_node.
> 
> So for
> 
>   D.xxxx_16 = (boolean) D.xxxx_6;
>   if (D.xxxx_16 != 0)
> 
> with a re-set boolean_type_node we fold this to D.xxxx_6 != 0, with
> the original Ada boolean_type_node we do not.  The C frontend for
> decaying to bool always produces a comparison btw, not the truncation
> we see above (and it only truncates to 8 bits).

There is no decaying to bool in Ada, all boolean values come from truth values
or other boolean values.  However, gigi first generates all the truth values in
integer_type_node before converting them to a boolean type.

> I think for 4.5 we should admit defeat and disable free-lang-data
> unconditionally if not using LTO.

That seems sensible to me.  I plan on revisiting this boolean stuff for 4.6 (as
well as the sizetype thing, but this looks like much trickier).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenth at gcc dot gnu dot org]

------- Comment #17 from rguenth at gcc dot gnu dot org  2009-12-02 17:14 -------
Subject: Bug 42088

Author: rguenth
Date: Wed Dec  2 17:13:51 2009
New Revision: 154918

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=154918
Log:
2009-12-02  Richard Guenther  <rguenther@suse.de>

        PR middle-end/42088
        * tree.c (free_lang_data): Disable if not using LTO.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088

[Bug lto/42088] flag_gtoggle in free_lang_data hides -fcompare-debug errors

[rguenth at gcc dot gnu dot org]

------- Comment #18 from rguenth at gcc dot gnu dot org  2009-12-02 17:16 -------
Fixed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42088