[Bug tree-optimization/94166] New: [10 Regression] random -fcompare-debug failures since r10-1913

[Bug tree-optimization/94166] New: [10 Regression] random -fcompare-debug failures since r10-1913

[zhroma at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94166

            Bug ID: 94166
           Summary: [10 Regression] random -fcompare-debug failures since
                    r10-1913
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zhroma at gcc dot gnu.org
                CC: jakub at redhat dot com, linkw at gcc dot gnu.org
  Target Milestone: ---
            Target: powerpc64le-*-linux-gnu

I've regtested recent r10-7146 version on powerpc64le with -fcompare-debug.
pr88497-*.c and pr92618.c tests give some kind of random results - they may
fail or pass.

So, after running one compilation manually several times like this:
gcc -O2 -fcompare-debug gcc/testsuite/gcc.c-torture/execute/pr92618.c
--save-temps
I finally managed to see final-dumps with difference like "a=b+c" vs "a=c+b".

Than I've added -fdump-tree-all and than found that the difference starts from
'reassoc1' dump.  Investigation shows r10-1913 causes that.  I've checked that
reverting r10-1913 and related r10-4930 (but without removing their tests,
which were actually under investigation) solved the problem.

I think the problem is using
for(hash_map<tree, v_info_ptr>::iterator it = v_info_map.begin ....) {
   ...
   valid_vecs.quick_push (cand_vec);
}

That loop gives unpredictable order in hash_map iterator and we got random
order of elements in valid_vecs.  Later we sort it, but in pr92618.c example it
seems there are two elements are their comparison result is 'equal'.


PS.  Actually, I've checked the difference between usual regtest results and
-fcompare-debug and there are a lot of other "more stable" failures, I'll also
file another PR.

[Bug tree-optimization/94166] [10 Regression] random -fcompare-debug failures since r10-1913

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94166

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
           Priority|P3                          |P1
                 CC|                            |jakub at gcc dot gnu.org
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-03-13
   Target Milestone|---                         |10.0

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Indeed:
--- pr92618.c.123t.reassoc1_    2020-03-13 13:58:06.494566731 +0100
+++ pr92618.c.123t.reassoc1     2020-03-13 13:58:33.151172526 +0100
@@ -88,8 +88,8 @@ foo (long long unsigned int * x)
   <bb 2> [local count: 1073741824]:
   c_6 = bar ();
   d_8 = bar ();
-  _19 = VIEW_CONVERT_EXPR<vector(2) long long unsigned int>(c_6);
-  _18 = VIEW_CONVERT_EXPR<vector(2) long long unsigned int>(d_8);
+  _19 = VIEW_CONVERT_EXPR<vector(2) long long unsigned int>(d_8);
+  _18 = VIEW_CONVERT_EXPR<vector(2) long long unsigned int>(c_6);
   _17 = _18 + _19;
   _21 = BIT_FIELD_REF <_17, 64, 0>;
   _20 = BIT_FIELD_REF <_17, 64, 64>;

[Bug tree-optimization/94166] [10 Regression] random -fcompare-debug failures since r10-1913

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94166

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This fixes it, but will try to find a testcase that fails more reliably:
--- gcc/tree-ssa-reassoc.c.jj   2020-01-12 11:54:38.510381771 +0100
+++ gcc/tree-ssa-reassoc.c      2020-03-13 14:06:32.358085863 +0100
@@ -1793,8 +1793,11 @@ sort_by_mach_mode (const void *p_i, cons
     return 1;
   else if (mode1 < mode2)
     return -1;
-  else
-    return 0;
+  if (SSA_NAME_VERSION (tr1) < SSA_NAME_VERSION (tr2))
+    return -1;
+  else if (SSA_NAME_VERSION (tr1) > SSA_NAME_VERSION (tr2))
+    return 1;
+  return 0;
 }

 /* Cleanup hash map for VECTOR information.  */

[Bug tree-optimization/94166] [10 Regression] random -fcompare-debug failures since r10-1913

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94166

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48028
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48028&action=edit
gcc10-pr94166.patch

Untested fix.

[Bug tree-optimization/94166] [10 Regression] random -fcompare-debug failures since r10-1913

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94166

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:5ba25b2ef179aec8ba4c47612fbc5c388f41cb36

commit r10-7185-g5ba25b2ef179aec8ba4c47612fbc5c388f41cb36
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Mar 16 09:02:21 2020 +0100

    tree-inline: Fix a -fcompare-debug issue in the inliner [PR94167]

    The following testcase fails with -fcompare-debug.  The problem is that
    bar is marked as address_taken only with -g and not without.
    I've tracked it down to insert_init_stmt calling gimple_regimplify_operands
    even on DEBUG_STMTs.  That function will just insert normal stmts before
    the DEBUG_STMT if the DEBUG_STMT operand isn't gimple val or invariant.
    While DCE will turn those statements into debug temporaries, it can cause
    differences in SSA_NAMEs and more importantly, the ipa references are
    generated from those before the DCE happens.
    On the testcase, the DEBUG_STMT value is (int)bar.

    We could generate DEBUG_STMTs with debug temporaries instead, but I fail to
    see the reason to do that, DEBUG_STMTs allow other expressions and all we
    want to ensure is that the expressions aren't too large (arbitrarily
    complex), but during inlining/function versioning I don't see why something
    would queue a DEBUG_STMT with arbitrarily complex expressions in there.

    2020-03-16  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/94166
            * tree-ssa-reassoc.c (sort_by_mach_mode): Use SSA_NAME_VERSION
            as secondary comparison key.

            * gcc.dg/pr94166.c: New test.

[Bug tree-optimization/94166] [10 Regression] random -fcompare-debug failures since r10-1913

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94166

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Oops, I've screwed up, the commit log should have been:
reassoc: Avoid code generation to depend on hash_map traversal [PR94166]

On the following testcase, if there is ASLR, the compiler generates             
different code each time (out of 1000 invocations 994 unique assembler          
contents).  The problem is that undistribute_bitref_for_vector uses             
a hash_map from a tree (SSA_NAME) to a vector and such a hash_map is            
by default doing pointer hashing on the SSA_NAME rather than using              
something stable (SSA_NAME_VERSION).                                            
One possible way would be to use SSA_NAME_VERSION as hash function,             
but given that we from the hash_map traversal just populate a vector            
which is then sorted, I think it is easier to make the sort callback            
use SSA_NAME_VERSION as secondary sorting key and thus ensure stable            
sort (that is generally desirable anyway).                                      

instead.  Sorry.