[Bug debug/99230] New: [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[Bug debug/99230] New: [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[hp at gcc dot gnu.org]

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

            Bug ID: 99230
           Summary: [11 Regression] gcc.dg/pr83527.c excess errors:
                    '-fcompare-debug' failure (length)
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: debug
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hp at gcc dot gnu.org
                CC: aoliva at gcc dot gnu.org, sandra at gcc dot gnu.org
  Target Milestone: ---
              Host: x86_64-linux
            Target: cris-elf

This test started to fail for cris-elf almost identically to the description in
PR83527 since 3696a50beeb73f4 / r11-3302 "Change C front end to emit structured
loop and switch tree nodes", but I don't see this failure for other targets in
the gcc-testresults@ archives (at least no search engine hits) and I can't find
an obvious target description oddity related to switches or case-tables.

Build machine is x86_64 stock Debian 9.  A brief look at the commit fixing
PR83527 (18408e962e5a) didn't help with applying that to the mentioned switch
rewrite.

No extra tools are needed to repeat the problem, just configure with  e.g.
"--target=cris-elf --enable-languages=c" and
"make all-gcc" then "make check-gcc-c RUNTESTFLAGS=dg.exp=pr83527.c"

(I use --target_board=cris-sim usually but that can be elided for this test.)

Perhaps of interest:
--- pr83527.gk.c.gkd    2021-02-23 16:02:07.985669417 +0100
+++ pr83527.c.gkd       2021-02-23 16:02:07.973669826 +0100
@@ -7,7 +7,7 @@ Declarations used by fn1, sorted by DECL
 5:   void <L0>;
 2:   void <L1>;
 3:   void <L2>;
-1:   void <L9>;
+1:   void <L10>;

 ;; Function fn1 (fn1, funcdef_no=0, cgraph_uid=1, symbol_order=2)

Testsuite run:
Running /x/gcc/gcc/testsuite/gcc.dg/dg.exp ...
... (lines elided)
FAIL: gcc.dg/pr83527.c (test for excess errors)

>From gcc.log:
xgcc: error: /x/gcc/gcc/testsuite/gcc.dg/pr83527.c: '-fcompare-debug' failure
(length)

[Bug debug/99230] [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[hp at gcc dot gnu.org]

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

--- Comment #1 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
Adding author of the switch rewrite (the trigging commit).

[Bug debug/99230] [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0

[Bug debug/99230] [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[jakub at gcc dot gnu.org]

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

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> ---
On x86_64-linux I can reproduce the *.gimple dump discrepancies with
-O2 -fcompare-debug --param logical-op-non-short-circuit=0
diff -up pr83527.c.006t.gimple pr83527.gk.c.006t.gimple 
--- pr83527.c.006t.gimple       2021-02-24 17:56:57.066963901 +0100
+++ pr83527.gk.c.006t.gimple    2021-02-24 17:56:57.536958750 +0100
@@ -3,23 +3,31 @@ void fn1 ()
   int c;
   short int d;

+  # DEBUG BEGIN_STMT
+  # DEBUG BEGIN_STMT
+  # DEBUG BEGIN_STMT
   a.0_1 = a;
   switch (a.0_1) <default: <D.1961>, case 32769: <D.1953>, case 32771:
<D.1954>, case 32772: <D.1955>, case 32782: <D.1956>, case 32800: <D.1952>>
   <D.1952>:
+  # DEBUG BEGIN_STMT
   fn2 ();
   <D.1953>:
+  # DEBUG BEGIN_STMT
   b = 0;
   <D.1954>:
   <D.1955>:
   <D.1956>:
+  # DEBUG BEGIN_STMT
   fn3 ();
   <D.1961>:
-  if (d != 0) goto <D.1962>; else goto <D.1963>;
-  <D.1963>:
-  if (c != 0) goto <D.1964>; else goto <D.1965>;
+  # DEBUG BEGIN_STMT
+  if (d != 0) goto <D.1962>; else goto <D.1964>;
   <D.1964>:
-  <D.1965>:
+  if (c != 0) goto <D.1962>; else goto <D.1963>;
   <D.1962>:
+  # DEBUG BEGIN_STMT
+  # DEBUG BEGIN_STMT
+  <D.1963>:
 }


which looks to me incorrect.
Without that param,
diff -up pr83527.c.006t.gimple pr83527.gk.c.006t.gimple 
--- pr83527.c.006t.gimple       2021-02-24 17:58:39.484841634 +0100
+++ pr83527.gk.c.006t.gimple    2021-02-24 17:58:39.967836342 +0100
@@ -3,22 +3,31 @@ void fn1 ()
   int c;
   short int d;

+  # DEBUG BEGIN_STMT
+  # DEBUG BEGIN_STMT
+  # DEBUG BEGIN_STMT
   a.0_1 = a;
   switch (a.0_1) <default: <D.1961>, case 32769: <D.1953>, case 32771:
<D.1954>, case 32772: <D.1955>, case 32782: <D.1956>, case 32800: <D.1952>>
   <D.1952>:
+  # DEBUG BEGIN_STMT
   fn2 ();
   <D.1953>:
+  # DEBUG BEGIN_STMT
   b = 0;
   <D.1954>:
   <D.1955>:
   <D.1956>:
+  # DEBUG BEGIN_STMT
   fn3 ();
   <D.1961>:
+  # DEBUG BEGIN_STMT
   _2 = d != 0;
   _3 = c != 0;
   _4 = _2 | _3;
   if (_4 != 0) goto <D.1962>; else goto <D.1963>;
   <D.1962>:
+  # DEBUG BEGIN_STMT
+  # DEBUG BEGIN_STMT
   <D.1963>:
 }

looks good.  But probably cris and x86_64 has different cswitch lowering later.

[Bug debug/99230] [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug debug/99230] [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-03-01
     Ever confirmed|0                           |1

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This can be seen also on
int a, b;
void fn1(int c, short d) {
  if (d || c) {
    do
      ;
    while (0);
  }
}
with -O2 --param logical-op-non-short-circuit=0 -fcompare-debug - differences
in *.gimple dump:
--- pr99230-2.c.006t.gimple     2021-03-01 12:46:25.715634310 +0100
+++ pr99230-2.gk.c.006t.gimple  2021-03-01 12:46:25.740634033 +0100
@@ -1,11 +1,13 @@
 void fn1 (int c, short int d)
 {
-  if (d != 0) goto <D.1955>; else goto <D.1956>;
-  <D.1956>:
-  if (c != 0) goto <D.1957>; else goto <D.1958>;
+  # DEBUG BEGIN_STMT
+  if (d != 0) goto <D.1955>; else goto <D.1957>;
   <D.1957>:
-  <D.1958>:
+  if (c != 0) goto <D.1955>; else goto <D.1956>;
   <D.1955>:
+  # DEBUG BEGIN_STMT
+  # DEBUG BEGIN_STMT
+  <D.1956>:
 }


I'm afraid it is the same issue as PR93786.
The difference is that when shortcut_cond_expr is run, in one case
then_se is false, because then_ is
 <nop_expr 0x7fffea1842e0
    type <void_type 0x7fffea05af18 void VOID
        align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea05af18
        pointer_to_this <pointer_type 0x7fffea062000>>
    constant
    arg:0 <integer_cst 0x7fffea041d08 type <integer_type 0x7fffea05a000
sizetype> constant 0>
    pr99230-2.c:11:7 start: pr99230-2.c:11:7 finish: pr99230-2.c:11:7>
but in another case is
 <statement_list 0x7fffea1862e0
    type <void_type 0x7fffea05af18 void VOID
        align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea05af18
        pointer_to_this <pointer_type 0x7fffea062000>>
    side-effects head 0x7fffea189168 tail 0x7fffea1891c8 stmts 0x7fffea189150
0x7fffea186240

    stmt <debug_begin_stmt 0x7fffea189150 type <void_type 0x7fffea05af18 void>

        pr99230-2.c:10:5 start: pr99230-2.c:10:5 finish: pr99230-2.c:10:6>
    stmt <statement_list 0x7fffea186240 type <void_type 0x7fffea05af18 void>
        head 0x7fffea189240 tail 0x7fffea189240 stmts 0x7fffea189228

        stmt <debug_begin_stmt 0x7fffea189228 type <void_type 0x7fffea05af18
void>

            pr99230-2.c:11:7 start: pr99230-2.c:11:7 finish:
pr99230-2.c:11:7>>>
and only the latter has TREE_SIDE_EFFECTS set.
In PR93786 I've tried several possibilities and had to gave up.
Alex, any thoughts?
Unless we want to remove the statement frontiers support, I'm not sure what
else we can do, we'd need some new trees to hold just the debug statements that
would preserve what exactly would be the behavior without -g.

Not sure if this needs to be P1 though, the statement frontiers issues are old,
and cris is not a primary or secondary target, so if we have a testcase that in
the end only fails there...

[Bug debug/99230] [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I can actually reproduce e.g. on x86_64-linux with:
extern void fn2(void);
extern void fn3(int);
int a, b;
void fn1() {
  int c;
  short d;
  switch (a) {
  case 22000:
    fn2();
  case 22300:
    b = 0;
  case 22600:
  case 22601:
  case 22900:
    fn3(1);
  case 20100:
    fn3(2);
  case 20200:
    fn3(3);
  case 20300:
    fn3(4);
  case 20400:
    fn3(5);
  case 20310:
    fn3(4);
  case 20410:
    fn3(5);
  }
  if (d || c) {
    do
      ;
    while (0);
  }
}

and -O2 --param logical-op-non-short-circuit=0 -fcompare-debug
--param=jump-table-max-growth-ratio-for-speed=5000
And the difference is the same:
--- pr99230-4.c.gkd     2021-03-01 13:44:32.625879468 +0100
+++ pr99230-4.gk.c.gkd  2021-03-01 13:44:32.731878292 +0100
@@ -9,7 +9,7 @@ Declarations used by fn1, sorted by DECL
 5:   void <L8>;
 4:   void <L9>;
 6:   void <L10>;
-1:   void <L16>;
+1:   void <L15>;

 ;; Function fn1 (fn1, funcdef_no=0, cgraph_uid=1, symbol_order=2)

[Bug debug/99230] [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50414
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50414&action=edit
gcc11-pr99230.patch

Untested fix.

[Bug debug/99230] [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[hp at gcc dot gnu.org]

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

--- Comment #6 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> Created attachment 50414 [details]
> gcc11-pr99230.patch
> 
> Untested fix.

Tested at r11-7589 for cris-elf, fixes the fail with no regressions.
Thanks!

[Bug debug/99230] [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 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:9f59cb7cac009f3c6eba81eb09714699b9ac9f8d

commit r11-7750-g9f59cb7cac009f3c6eba81eb09714699b9ac9f8d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Mar 20 17:02:06 2021 +0100

    c-family: Fix PR94272 -fcompare-debug issue even for C [PR99230]

    The following testcase results in -fcompare-debug failure.
    The problem is the similar like in PR94272
    https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542562.html
    When genericizing, with -g0 we have just a TREE_SIDE_EFFECTS DO_STMT
    in a branch of if, while with -g we have that wrapped into
    TREE_SIDE_EFFECTS STATEMENT_LIST containing DEBUG_BEGIN_STMT and that
    DO_STMT.
    The do loop is empty with 0 condition, so c_genericize_control_stmt
    turns it into an empty statement (without TREE_SIDE_EFFECTS).
    For -g0 that means that suddenly the if branch doesn't have side effects
    and is expanded differently.  But with -g we still have TREE_SIDE_EFFECTS
    STATEMENT_LIST containing DEBUG_BEGIN_STMT and non-TREE_SIDE_EFFECTS stmt.
    The following patch fixes that by detecting this case and removing
    TREE_SIDE_EFFECTS.

    And, so that we don't duplicate the same code, changes the C++ FE to
    just call the c_genericize_control_stmt function that can now handle it.

    2021-03-20  Jakub Jelinek  <jakub@redhat.com>

            PR debug/99230
            * c-gimplify.c (c_genericize_control_stmt): Handle STATEMENT_LIST.

            * cp-gimplify.c (cp_genericize_r) <case STATEMENT_LIST>: Remove
            special code, instead call c_genericize_control_stmt.

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

[Bug debug/99230] [11 Regression] gcc.dg/pr83527.c excess errors: '-fcompare-debug' failure (length)

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.