[Bug debug/104459] New: [9/10/11/12 Regression] '-fcompare-debug' failure w/ -O2 -funswitch-loops -fno-tree-dce

[Bug debug/104459] New: [9/10/11/12 Regression] '-fcompare-debug' failure w/ -O2 -funswitch-loops -fno-tree-dce

[asolokha at gmx dot com]

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

            Bug ID: 104459
           Summary: [9/10/11/12 Regression] '-fcompare-debug' failure w/
                    -O2 -funswitch-loops -fno-tree-dce
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: compare-debug-failure
          Severity: normal
          Priority: P3
         Component: debug
          Assignee: unassigned at gcc dot gnu.org
          Reporter: asolokha at gmx dot com
  Target Milestone: ---

gcc 12.0.1 20220206 snapshot (g:8eb329e963593342855b6072e5692659107337b7) fails
-fcompare-debug check when compiling the following testcase w/ -O2
-funswitch-loops -fno-tree-dce:

void
foo (int x, int y)
{
  unsigned int a;

  for (;;)
    {
      short int *p = (short int *) &x;
      unsigned int q = 0;

      a /= 2;
      if (a)
        {
          q -= y;
          while (q)
            {
            }
        }

      if (x)
        {
          for (q = 0; q != 1; q += 2)
            {
              unsigned int n;

              n = *p ? 0 : q;
              y += n < 1;

              n = a || *p;
              if (n % x == 0)
                y /= x;
            }
        }

    }
}

% gcc-12.0.1 -O2 -fcompare-debug -funswitch-loops -fno-tree-dce -c rfygrmu3.c
gcc-12.0.1: error: rfygrmu3.c: '-fcompare-debug' failure

--- rfygrmu3.c.gkd      2022-02-09 18:38:41.086339468 +0700
+++ rfygrmu3.gk.c.gkd   2022-02-09 18:38:41.117339440 +0700
@@ -32,7 +32,7 @@
             (pc))) "rfygrmu3.c":20:10# {*jcc}
      (expr_list:REG_DEAD (reg:CCZ 17 flags)
         (int_list:REG_BR_PROB 118111604 (nil)))
- -> 18)
+ -> 19)
 (code_label # 0 0 4 4 (nil) [1 uses])
 (note # 0 0 [bb 4] NOTE_INSN_BASIC_BLOCK)
 (insn:TI # 0 0 4 (set (reg/v:SI 36 r8 [orig:89 a ] [89])
@@ -79,7 +79,7 @@
      (nil)
  -> 3)
 (barrier # 0 0)
-(code_label # 0 0 7 18 (nil) [1 uses])
+(code_label # 0 0 7 19 (nil) [1 uses])
 (note # 0 0 [bb 7] NOTE_INSN_BASIC_BLOCK)
 (insn:TI # 0 0 7 (set (reg:CCZ 17 flags)
         (compare:CCZ (reg:HI 5 di [orig:94 x ] [94])

[Bug rtl-optimization/104459] [9/10/11/12 Regression] '-fcompare-debug' failure w/ -O2 -funswitch-loops -fno-tree-dce

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |9.5
   Last reconfirmed|                            |2022-02-09
          Component|debug                       |rtl-optimization
           Priority|P3                          |P2

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  On the GIMPLE level there are no differences.  The compare fail
might be spurious though.

[Bug rtl-optimization/104459] [9/10/11/12 Regression] '-fcompare-debug' failure w/ -O2 -funswitch-loops -fno-tree-dce

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The divergence starts during jump_after_combine.
With -g0, cfg_layout_merge_blocks is called on
(note 53 51 56 13 [bb 13] NOTE_INSN_BASIC_BLOCK)
and
(code_label 56 53 57 14 8 (nil) [0 uses])
(note 57 56 79 14 [bb 14] NOTE_INSN_BASIC_BLOCK)
while with -g on:
(note 91 89 92 13 [bb 13] NOTE_INSN_BASIC_BLOCK)
(debug_insn 92 91 93 13 (var_location:SI n (clobber (const_int 0 [0])))
"pr104459.c":29:17 -1
     (nil))
(debug_insn 93 92 135 13 (debug_marker) "pr104459.c":31:17 -1
     (nil))
(debug_insn 135 93 96 13 (var_location:SI D#3 (clobber (const_int 0 [0]))) -1
     (nil))
(debug_insn 96 135 97 13 (var_location:SI y (debug_expr:SI D#3))
"pr104459.c":31:19 -1
     (nil))
and
(code_label 97 96 98 14 8 (nil) [0 uses])
(note 98 97 99 14 [bb 14] NOTE_INSN_BASIC_BLOCK)
(debug_insn 99 98 100 14 (var_location:SI y (clobber (const_int 0 [0]))) -1
     (nil))
(debug_insn 100 99 102 14 (debug_marker) "pr104459.c":22:33 -1
     (nil))
(debug_insn 102 100 133 14 (var_location:SI q (clobber (const_int 0 [0])))
"pr104459.c":22:33 -1
     (nil))

That function calls update_bb_for_insn_chain for note 98 through 102 for -g and
note 57 through note 57 for -g0.
547       end = NEXT_INSN (end);
548       for (insn = begin; insn != end; insn = NEXT_INSN (insn))
549         if (!BARRIER_P (insn))
550           df_insn_change_bb (insn, bb);
which means that for -g0 df_insn_change_bb isn't called, while for -g it is.
df_insn_change_bb in turn calls df_set_bb_dirty and that sets bb(14)->flags |=
BB_MODIFIED.
Unfortunately, cfgcleanup.cc uses the BB_MODIFIED flag to decide on some stuff:
try_forward_edges has:
      bool may_thread = first_pass || (b->flags & BB_MODIFIED) != 0;
and
          may_thread |= (target->flags & BB_MODIFIED) != 0;
and try_crossjump_bb has:
          /* If nothing changed since the last attempt, there is nothing
             we can do.  */
          if (!first_pass
              && !((e->src->flags & BB_MODIFIED)
                   || (fallthru->src->flags & BB_MODIFIED)))
            continue;
and
          /* If nothing changed since the last attempt, there is nothing
             we can do.  */
          if (!first_pass
              && !((e->src->flags & BB_MODIFIED)
                   || (e2->src->flags & BB_MODIFIED)))
            continue;
Seems this has been introduced in
https://gcc.gnu.org/legacy-ml/gcc-patches/2010-08/txt00014.txt
but before that we were using equally problematic df_get_bb_dirty.
As BB_MODIFIED is only used in those places:
cfgcleanup.cc:      bool may_thread = first_pass || (b->flags & BB_MODIFIED) !=
0;
cfgcleanup.cc:    may_thread |= (target->flags & BB_MODIFIED) != 0;
cfgcleanup.cc:        && !((e->src->flags & BB_MODIFIED)
cfgcleanup.cc:             || (fallthru->src->flags & BB_MODIFIED)))
cfgcleanup.cc:        && !((e->src->flags & BB_MODIFIED)
cfgcleanup.cc:             || (e2->src->flags & BB_MODIFIED)))
df-core.cc:  bb->flags |= BB_MODIFIED;
I think best would be to introduce df_set_bb_dirty_for_debug or similar variant
to df_set_bb_dirty that wouldn't set the BB_MODIFIED flag, and use that in
df_insn_change_bb.

[Bug rtl-optimization/104459] [9/10/11/12 Regression] '-fcompare-debug' failure w/ -O2 -funswitch-loops -fno-tree-dce

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
On the other side, df-scan.cc does in 3 other spots:
  /* By adding the ref directly, df_insn_rescan my not find any
     differences even though the block will have changed.  So we need
     to mark the block dirty ourselves.  */
  if (!DEBUG_INSN_P (DF_REF_INSN (ref)))
    df_set_bb_dirty (bb);
and
  /* The block must be marked as dirty now, rather than later as in
     df_insn_rescan and df_notes_rescan because it may not be there at
     rescanning time and the mark would blow up.
     DEBUG_INSNs do not make a block's data flow solution dirty (at
     worst the LUIDs are no longer contiguous).  */
  if (bb != NULL && NONDEBUG_INSN_P (insn))
    df_set_bb_dirty (bb);
and
  if (!DEBUG_INSN_P (insn))
    df_set_bb_dirty (bb);
so I wonder if another fix wouldn't be just not call df_set_bb_dirty on
DEBUG_INSN_P in df_insn_change_bb.  But what do I know about df :(.

[Bug rtl-optimization/104459] [9/10/11/12 Regression] '-fcompare-debug' failure w/ -O2 -funswitch-loops -fno-tree-dce

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aoliva at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
See e.g. https://gcc.gnu.org/legacy-ml/gcc-patches/2010-10/msg00059.html

[Bug rtl-optimization/104459] [9/10/11/12 Regression] '-fcompare-debug' failure w/ -O2 -funswitch-loops -fno-tree-dce

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 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:18c8086d65f3d539e065ea7c97e3de6f3bbdf684

commit r12-7194-g18c8086d65f3d539e065ea7c97e3de6f3bbdf684
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Feb 11 11:21:24 2022 +0100

    df: Don't set bbs dirty because of debug insn moves [PR104459]

    As mentioned in the PR, we get -fcompare-debug failure, which is caused by
    cfg_layout_merge_blocks successfully merging two bbs where both bbs
    contained just CODE_LABEL, NOTE_INSN_BASIC_BLOCK and in the -g case both
    some debug insns at the end.  cfg_layout_merge_blocks calls
    update_bb_for_insn_chain which for the post-label insns in the second block
    (except for BARRIERs) calls df_insn_change_bb.  This function changes
    the bb of the insns and for notes just punts, but for other insns calls
    df_set_bb_dirty.  Now the problem is that because there were only debug
    insns and notes in the second block, df_set_bb_dirty is called on both
    only in the -g case and not with -g0.  df_set_bb_dirty these days
    sets both the BB_MODIFIED flag and marks the bb as dirty, and the former
    is what 6 spots in cfgcleanup.cc use in code-generation decisions,
    in this case
              may_thread |= (target->flags & BB_MODIFIED) != 0;
    in particular.  So, with -g may_thread is true while with -g0 it is not
    and we diverge from that point onwards.
    I've thought about introducing df_set_bb_dirty_nondebug that wouldn't
    set BB_MODIFIED but would mark the bb dirty, but then I went through
    history and found changes like:
    https://gcc.gnu.org/legacy-ml/gcc-patches/2010-10/msg00059.html
    so I've also tried just not calling df_set_bb_dirty for debug insns
    at all and it passed x86_64-linux and i686-linux
    --enable-checking=yes,rtl,extra,df bootstraps/regtests, so perhaps
    that works too.
    Now that I look at it again, if we don't need those from %d to %d messages
    for debug insns in the dump files, another way to fix it would be just to
    change the very first line in the hunk from
      if (!INSN_P (insn))
    to
      if (!DEBUG_INSN_P (insn))
    Though, df_set_bb_dirty_nondebug which will do everything but
    set bb->flags |= BB_MODIFIED is yet another option I can test.
    Perhaps even that PR42889 was solely about those 6 decisions in cfgcleanup
    (at that point it used df_get_bb_dirty) and not about actually the
    recomputation of some of the problems causing different code generations.

    2022-02-11  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/104459
            * df-scan.cc (df_insn_change_bb): Don't call df_set_bb_dirty when
            moving DEBUG_INSNs between bbs.

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

[Bug rtl-optimization/104459] [9/10/11 Regression] '-fcompare-debug' failure w/ -O2 -funswitch-loops -fno-tree-dce

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[9/10/11/12 Regression]     |[9/10/11 Regression]
                   |'-fcompare-debug' failure   |'-fcompare-debug' failure
                   |w/ -O2 -funswitch-loops     |w/ -O2 -funswitch-loops
                   |-fno-tree-dce               |-fno-tree-dce

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk.  I'll prefer to wait quite a long before considering to
backport this though.

[Bug rtl-optimization/104459] [10/11 Regression] '-fcompare-debug' failure w/ -O2 -funswitch-loops -fno-tree-dce

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.5                         |10.4

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9 branch is being closed

[Bug rtl-optimization/104459] [10/11 Regression] '-fcompare-debug' failure w/ -O2 -funswitch-loops -fno-tree-dce

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.4                        |10.5

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

[Bug rtl-optimization/104459] [11 Regression] '-fcompare-debug' failure w/ -O2 -funswitch-loops -fno-tree-dce

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.5                        |11.5

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10 branch is being closed.