Bug 109410 - [13/14/15 Regression] ICE: verify_flow_info failed
Summary: [13/14/15 Regression] ICE: verify_flow_info failed
Status: REOPENED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 13.0
: P2 normal
Target Milestone: 13.4
Assignee: Not yet assigned to anyone
URL:
Keywords: compare-debug-failure, ice-checking, ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2023-04-04 16:10 UTC by G. Steinmetz
Modified: 2024-05-21 09:14 UTC (History)
6 users (show)

See Also:
Host:
Target: x86_64-pc-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed: 2023-04-11 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description G. Steinmetz 2023-04-04 16:10:58 UTC
Started between 20220717 and 20220724, at -O1+ :
(testcase derived from pr108783.c)
(gcc configured with --enable-checking=yes)


$ cat z1.c
__attribute__((returns_twice)) int baz (int, int);
int
bar (int x)
{
  return x;
}
int
foo (int x, int y)
{
  int a;
  a = bar (x);
  baz (x, y);
  return y && a && a;
}
int
qux (int x, int y)
{
  int a;
  a = bar (x);
  baz (x, y);
  return y && a != 42 && a >= 42;
}
int
corge (int x, int y)
{
  int a;
  baz (x, y);
  a = bar (x);
  return y || a == 42 || a > 42;
}


$ gcc-13-20230402 -c z1.c -O2
z1.c: In function 'corge':
z1.c:24:1: error: returns_twice call is not first in basic block 2
   24 | corge (int x, int y)
      | ^~~~~
baz (x_7(D), y_8(D));
during GIMPLE pass: reassoc
z1.c:24:1: internal compiler error: verify_flow_info failed
0x8eeb6f verify_flow_info()
        ../../gcc/cfghooks.cc:285
0xd58ce6 execute_function_todo
        ../../gcc/passes.cc:2110
0xd59742 execute_todo
        ../../gcc/passes.cc:2152
Comment 1 Andrew Pinski 2023-04-04 20:53:29 UTC
Most likely r13-1754-g7a158a5776f5ca which introduced the checking in the first place.
Comment 2 Andrew Pinski 2023-04-04 20:58:46 UTC
The broken IR has been there since at least 4.9.0:
  <bb 2>:
  _1 = x_4(D) > 41;
  baz (x_4(D), y_5(D));
  goto <bb 4>;

  <bb 3>:
  ABNORMAL_DISPATCHER (0);

  <bb 4>:
  _8 = y_5(D) != 0;
  _9 = x_4(D) == 42;
  _11 = x_4(D) > 42;
  _12 = _1 | _8;
  _13 = (int) _12;


4.7.x didn't have the ABNORMAL_DISPATCHER part of the IR either.
Comment 3 Richard Biener 2023-04-11 12:31:24 UTC
Confirmed.  ISTR we did have another issue like this Jakub fixed recently?

   <bb 2> [local count: 1073741824]:
+  _4 = x_7(D) > 41;
   baz (x_7(D), y_8(D));
   goto <bb 4>; [99.96%]
 
@@ -101,9 +141,8 @@
   <bb 4> [local count: 1073312329]:
   _1 = y_8(D) != 0;
   _2 = x_7(D) == 42;
-  _3 = _1 | _2;
   _12 = x_7(D) > 42;
-  _10 = _3 | _12;
+  _10 = _4 | _1;
   _13 = (int) _10;
   return _13;
Comment 4 Jakub Jelinek 2023-04-11 14:33:57 UTC
PR108783?
Anyway, will have a look now.
Comment 5 Sam James 2023-04-12 05:00:12 UTC
(In reply to Jakub Jelinek from comment #4)
> PR108783?

Test case was from it. I don't mind not adding such things to See Also though, I'm still new to bug wrangling. Sorry if it's wrong!
Comment 6 GCC Commits 2023-04-12 14:55:47 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:51856718a82ce60f067910d9037ca255645b37eb

commit r13-7155-g51856718a82ce60f067910d9037ca255645b37eb
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 12 16:55:15 2023 +0200

    reassoc: Fix up another ICE with returns_twice call [PR109410]
    
    The following testcase ICEs in reassoc, unlike the last case I've fixed
    there here SSA_NAME_USED_IN_ABNORMAL_PHI is not the case anywhere.
    build_and_add_sum places new statements after the later appearing definition
    of an operand but if both operands are default defs or constants, we place
    statement at the start of the function.
    
    If the very first statement of a function is a call to returns_twice
    function, this doesn't work though, because that call has to be the first
    thing in its basic block, so the following patch splits the entry successor
    edge such that the new statements are added into a different block from the
    returns_twice call.
    
    I think we should in stage1 reconsider such placements, I think it
    unnecessarily enlarges the lifetime of the new lhs if its operand(s)
    are used more than once in the function.  Unless something sinks those
    again.  Would be nice to place it closer to the actual uses (or where
    they will be placed).
    
    2023-04-12  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/109410
            * tree-ssa-reassoc.cc (build_and_add_sum): Split edge from entry
            block if first statement of the function is a call to returns_twice
            function.
    
            * gcc.dg/pr109410.c: New test.
Comment 7 Jakub Jelinek 2023-04-12 14:58:43 UTC
Fixed (though might be worth backporting).
Comment 8 GCC Commits 2023-04-18 07:16:24 UTC
The releases/gcc-12 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:0e55b8e091a7263364cb08fd2c9172babb554ab1

commit r12-9428-g0e55b8e091a7263364cb08fd2c9172babb554ab1
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 12 16:55:15 2023 +0200

    reassoc: Fix up another ICE with returns_twice call [PR109410]
    
    The following testcase ICEs in reassoc, unlike the last case I've fixed
    there here SSA_NAME_USED_IN_ABNORMAL_PHI is not the case anywhere.
    build_and_add_sum places new statements after the later appearing definition
    of an operand but if both operands are default defs or constants, we place
    statement at the start of the function.
    
    If the very first statement of a function is a call to returns_twice
    function, this doesn't work though, because that call has to be the first
    thing in its basic block, so the following patch splits the entry successor
    edge such that the new statements are added into a different block from the
    returns_twice call.
    
    I think we should in stage1 reconsider such placements, I think it
    unnecessarily enlarges the lifetime of the new lhs if its operand(s)
    are used more than once in the function.  Unless something sinks those
    again.  Would be nice to place it closer to the actual uses (or where
    they will be placed).
    
    2023-04-12  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/109410
            * tree-ssa-reassoc.cc (build_and_add_sum): Split edge from entry
            block if first statement of the function is a call to returns_twice
            function.
    
            * gcc.dg/pr109410.c: New test.
    
    (cherry picked from commit 51856718a82ce60f067910d9037ca255645b37eb)
Comment 9 David Binderman 2023-04-22 12:41:43 UTC
I think this one might have returned. With today's gcc trunk:

$ ~/gcc/results/bin/gcc -c  ./gcc.dg/pr109410.c
$ ~/gcc/results/bin/gcc -c -g ./gcc.dg/pr109410.c
$ ~/gcc/results/bin/gcc -c -g -O1 ./gcc.dg/pr109410.c
./gcc.dg/pr109410.c: In function ‘foo’:
./gcc.dg/pr109410.c:14:1: error: returns_twice call is not first in basic block 2
   14 | foo (int x, int y)
      | ^~~
baz (x_7(D), y_8(D));
during GIMPLE pass: reassoc
./gcc.dg/pr109410.c:14:1: internal compiler error: verify_flow_info failed


$ ~/gcc/results/bin/gcc -v
Using built-in specs.
COLLECT_GCC=/home/dcb36/gcc/results/bin/gcc
COLLECT_LTO_WRAPPER=/home/dcb36/gcc/results.20230422/libexec/gcc/x86_64-pc-linux-gnu/14.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../trunk.year/configure --prefix=/home/dcb36/gcc/results.20230422 --disable-multilib --disable-bootstrap --with-pkgversion=cda246f8b421ba85 --enable-checking=yes --enable-languages=c,c++,fortran
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 14.0.0 20230422 (experimental) (cda246f8b421ba85) 
$
Comment 10 Sam James 2023-04-22 12:58:36 UTC
Reopening then.
Comment 11 Sam James 2023-04-23 02:45:41 UTC
Broken for 13 again too:

$ gcc -c -g -O1 ./gcc/testsuite/gcc.dg/pr109410.c
./gcc/testsuite/gcc.dg/pr109410.c: In function ‘foo’:
./gcc/testsuite/gcc.dg/pr109410.c:14:1: error: returns_twice call is not first in basic block 2
   14 | foo (int x, int y)
      | ^~~
baz (x_7(D), y_8(D));
during GIMPLE pass: reassoc
./gcc/testsuite/gcc.dg/pr109410.c:14:1: internal compiler error: verify_flow_info failed
0x7a85a2 verify_flow_info()
        /usr/src/debug/sys-devel/gcc-13.0.1_pre20230421/gcc-13.1.0-RC-20230421/gcc/cfghooks.cc:285
0x19c3a82 execute_function_todo
        /usr/src/debug/sys-devel/gcc-13.0.1_pre20230421/gcc-13.1.0-RC-20230421/gcc/passes.cc:2110
0x192e2e6 do_per_function
        /usr/src/debug/sys-devel/gcc-13.0.1_pre20230421/gcc-13.1.0-RC-20230421/gcc/passes.cc:1694
0x192e2e6 execute_todo
        /usr/src/debug/sys-devel/gcc-13.0.1_pre20230421/gcc-13.1.0-RC-20230421/gcc/passes.cc:2152
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://bugs.gentoo.org/> for instructions.
Comment 12 Sam James 2023-04-23 03:58:56 UTC
Bisect gives a nonsensical result of r13-7156-g31eb8f18bbe646 with initial good as r13-7155-g51856718a82ce60f067910d9037ca255645b37eb and bad as releases/gcc-13.

I've checked out r13-7155-g51856718a82ce60f067910d9037ca255645b37eb and with that commit, the test case still fails:
```
$ /tmp/bisect/bin/gcc -c -g -O1 /home/sam/git/gcc/gcc/testsuite/gcc.dg/pr109410.c
/home/sam/git/gcc/gcc/testsuite/gcc.dg/pr109410.c: In function ‘foo’:
/home/sam/git/gcc/gcc/testsuite/gcc.dg/pr109410.c:14:1: error: returns_twice call is not first in basic block 2
   14 | foo (int x, int y)
      | ^~~
baz (x_7(D), y_8(D));
during GIMPLE pass: reassoc
/home/sam/git/gcc/gcc/testsuite/gcc.dg/pr109410.c:14:1: internal compiler error: verify_flow_info failed
0x9c06ee verify_flow_info()
        ../.././gcc/cfghooks.cc:285
0xdb8077 execute_function_todo
        ../.././gcc/passes.cc:2110
0xdb85be execute_todo
        ../.././gcc/passes.cc:2152
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
```

with:
```
gcc (GCC) 13.0.1 20230412 (experimental)
Copyright (C) 2023 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
```

so I don't think the original fix is complete?
Comment 13 Andrew Pinski 2023-04-23 04:07:59 UTC
So the testcase gcc.dg/pr109410.c passes but if you add -g, the ICE will show up still.

This also means there will be some compare-debug failure too as the IR/CFG will be different with/without -g.

I suspect gsi_after_labels here should really be gsi_start_nondebug_after_labels_bb .
Comment 14 Andrew Pinski 2023-04-23 04:11:32 UTC
(In reply to Andrew Pinski from comment #13)
> So the testcase gcc.dg/pr109410.c passes but if you add -g, the ICE will
> show up still.
> 
> This also means there will be some compare-debug failure too as the IR/CFG
> will be different with/without -g.
> 
> I suspect gsi_after_labels here should really be
> gsi_start_nondebug_after_labels_bb .

That is:
diff --git a/gcc/tree-ssa-reassoc.cc b/gcc/tree-ssa-reassoc.cc
index 067a3f07f7e..c71780e6d54 100644
--- a/gcc/tree-ssa-reassoc.cc
+++ b/gcc/tree-ssa-reassoc.cc
@@ -1563,7 +1563,7 @@ build_and_add_sum (tree type, tree op1, tree op2, enum tree_code opcode)
   if ((!op1def || gimple_nop_p (op1def))
       && (!op2def || gimple_nop_p (op2def)))
     {
-      gsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
+      gsi = gsi_start_nondebug_after_labels_bb (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
       if (!gsi_end_p (gsi)
          && is_gimple_call (gsi_stmt (gsi))
          && (gimple_call_flags (gsi_stmt (gsi)) & ECF_RETURNS_TWICE))
Comment 15 Richard Biener 2023-04-26 06:58:19 UTC
GCC 13.1 is being released, retargeting bugs to GCC 13.2.
Comment 16 GCC Commits 2023-05-02 20:16:45 UTC
The releases/gcc-11 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:e61e70fbfdcc99cbd23a02a1c789d3290b58d5a8

commit r11-10735-ge61e70fbfdcc99cbd23a02a1c789d3290b58d5a8
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 12 16:55:15 2023 +0200

    reassoc: Fix up another ICE with returns_twice call [PR109410]
    
    The following testcase ICEs in reassoc, unlike the last case I've fixed
    there here SSA_NAME_USED_IN_ABNORMAL_PHI is not the case anywhere.
    build_and_add_sum places new statements after the later appearing definition
    of an operand but if both operands are default defs or constants, we place
    statement at the start of the function.
    
    If the very first statement of a function is a call to returns_twice
    function, this doesn't work though, because that call has to be the first
    thing in its basic block, so the following patch splits the entry successor
    edge such that the new statements are added into a different block from the
    returns_twice call.
    
    I think we should in stage1 reconsider such placements, I think it
    unnecessarily enlarges the lifetime of the new lhs if its operand(s)
    are used more than once in the function.  Unless something sinks those
    again.  Would be nice to place it closer to the actual uses (or where
    they will be placed).
    
    2023-04-12  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/109410
            * tree-ssa-reassoc.c (build_and_add_sum): Split edge from entry
            block if first statement of the function is a call to returns_twice
            function.
    
            * gcc.dg/pr109410.c: New test.
    
    (cherry picked from commit 51856718a82ce60f067910d9037ca255645b37eb)
Comment 17 GCC Commits 2023-05-03 15:23:14 UTC
The releases/gcc-10 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:dc457bc1c72c53ee8c4800bac84264e0c8085f24

commit r10-11385-gdc457bc1c72c53ee8c4800bac84264e0c8085f24
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 12 16:55:15 2023 +0200

    reassoc: Fix up another ICE with returns_twice call [PR109410]
    
    The following testcase ICEs in reassoc, unlike the last case I've fixed
    there here SSA_NAME_USED_IN_ABNORMAL_PHI is not the case anywhere.
    build_and_add_sum places new statements after the later appearing definition
    of an operand but if both operands are default defs or constants, we place
    statement at the start of the function.
    
    If the very first statement of a function is a call to returns_twice
    function, this doesn't work though, because that call has to be the first
    thing in its basic block, so the following patch splits the entry successor
    edge such that the new statements are added into a different block from the
    returns_twice call.
    
    I think we should in stage1 reconsider such placements, I think it
    unnecessarily enlarges the lifetime of the new lhs if its operand(s)
    are used more than once in the function.  Unless something sinks those
    again.  Would be nice to place it closer to the actual uses (or where
    they will be placed).
    
    2023-04-12  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/109410
            * tree-ssa-reassoc.c (build_and_add_sum): Split edge from entry
            block if first statement of the function is a call to returns_twice
            function.
    
            * gcc.dg/pr109410.c: New test.
    
    (cherry picked from commit 51856718a82ce60f067910d9037ca255645b37eb)
Comment 18 Richard Biener 2023-07-27 09:25:49 UTC
GCC 13.2 is being released, retargeting bugs to GCC 13.3.
Comment 19 Jakub Jelinek 2024-05-21 09:14:38 UTC
GCC 13.3 is being released, retargeting bugs to GCC 13.4.