Bug 51106 - [4.6 Regression] ICE in move_insn, at haifa-sched.c:2314
Summary: [4.6 Regression] ICE in move_insn, at haifa-sched.c:2314
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.5.3
: P2 normal
Target Milestone: 4.7.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-12 13:39 UTC by Roger Luethi
Modified: 2015-06-23 08:27 UTC (History)
9 users (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build:
Known to work: 4.3.6, 4.4.5, 4.7.0
Known to fail: 4.5.3, 4.6.2, 4.8.0
Last reconfirmed: 2012-01-03 00:00:00


Attachments
reduced test case by SpanKY (386 bytes, application/octet-stream)
2011-11-12 13:39 UTC, Roger Luethi
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Roger Luethi 2011-11-12 13:39:17 UTC
Created attachment 25806 [details]
reduced test case by SpanKY

I am getting the following ICE compiling Linux 3.0.4 with gcc 4.5.3 (Gentoo). It didn't happen with 4.4, but it also happens with 4.6.1 (Ubuntu).

$ gcc-4.5.3  -c -m32 -O2 aesni-intel_glue.i
aesni-intel_glue.i:32:25: warning: initialization from incompatible pointer type [enabled by default]
aesni-intel_glue.i: In function 'aes_decrypt':
aesni-intel_glue.i:30:1: internal compiler error: in move_insn, at haifa-sched.c:2314
Please submit a full bug report,
[...]

$ gcc-4.6.1  -c -m32 -O2 aesni-intel_glue.i
aesni-intel_glue.i:32:25: warning: initialization from incompatible pointer type [enabled by default]
aesni-intel_glue.i: In function 'aes_decrypt':
aesni-intel_glue.i:30:1: internal compiler error: in move_insn, at haifa-sched.c:2353
Please submit a full bug report,
[...]

Reported as https://bugs.gentoo.org/show_bug.cgi?id=388835
Comment 1 Richard Biener 2012-01-03 11:49:52 UTC
On trunk we ICE with

t.i:32:25: warning: initialization from incompatible pointer type [enabled by default]
t.i:2:5: warning: always_inline function might not be inlinable [-Wattributes]
t.i: In function 'aes_decrypt':
t.i:30:1: error: in basic block 8:
t.i:30:1: error: flow control insn inside a basic block
(jump_insn 34 33 47 8 (parallel [
            (asm_operands/v ("1: jmp %l1
2:
.section .altinstructions,"a"
.balign 4
.long 1b
.long 0
.word %P0
.byte 2b - 1b
.byte 0
.previous
") ("") 0 [
                    (const_int 24 [0x18])
                ]
                 [
                    (asm_input:HI ("i") (null):0)
                ]
                 [
                    (label_ref:SI 22)
                ] t.i:30)
            (clobber (reg:QI 18 fpsr))
            (clobber (reg:QI 17 flags))
        ]) t.i:4 -1
     (expr_list:REG_UNUSED (reg:QI 18 fpsr)
        (expr_list:REG_UNUSED (reg:QI 17 flags)
            (nil)))
 -> 22)
t.i:30:1: internal compiler error: in rtl_verify_flow_info_1, at cfgrtl.c:2002
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

at -O2 and

t.i:4:2: error: impossible constraint in 'asm'
t.i:9:1: error: too many outgoing branch edges from bb 2
t.i:9:1: internal compiler error: verify_flow_info failed
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

with -O1.
Comment 2 Andrey Belevantsev 2012-01-19 09:24:27 UTC
Well, the instantiate virtual regs pass just deletes the asm as having impossible constraints and does not clean up cfg after itself.  As the asm is actually a jump in this case, everything blows up.

The trivial patch below makes this work for -O[012].  Any other places in function.c need patching up? Jakub, what do you think?

diff --git a/gcc/function.c b/gcc/function.c
index fcb79f5..94e51f4 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -1737,7 +1737,7 @@ instantiate_virtual_regs_in_insn (rtx insn)
       if (!check_asm_operands (PATTERN (insn)))
        {
          error_for_asm (insn, "impossible constraint in %<asm%>");
-         delete_insn (insn);
+         delete_insn_and_edges (insn);
        }
     }
   else
Comment 3 Jakub Jelinek 2012-01-19 09:43:16 UTC
Shorter testcase for gcc.c-torture/compile/ :
int
foo (int x)
{
  asm goto ("" : : "i" (x) : : lab);
  return 1;
lab:
  return 0;
}

Yeah, I think delete_insn_and_edges is the right fix here.  You might also add
into the testsuite:
int
bar (int x)
{
  asm goto ("" : : "i" (x) : : lab);
  __builtin_unreachable ();
lab:
  return 0;
}
(fortunately at this point the bb ending with the asm goto still has a fallthru successor, otherwise purge_dead_edges wouldn't work).
Comment 4 Andrey Belevantsev 2012-01-20 06:22:31 UTC
Author: abel
Date: Fri Jan 20 06:22:24 2012
New Revision: 183325

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183325
Log:
gcc:/
2012-01-20  Andrey Belevantsev  <abel@ispras.ru>

      PR target/51106
      * function.c (instantiate_virtual_regs_in_insn): Use
      delete_insn_and_edges when removing a wrong asm insn.

testsuite:/
2012-01-20  Jakub Jelinek  <jakub@redhat.com>

      PR target/51106
      * gcc.dg/torture/pr51106-1.c: New test.
      * gcc.dg/torture/pr51106-2.c: New test.


Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr51106-1.c
    trunk/gcc/testsuite/gcc.dg/torture/pr51106-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/function.c
    trunk/gcc/testsuite/ChangeLog
Comment 5 Andrey Belevantsev 2012-01-20 06:23:41 UTC
Fixed on trunk.
Comment 6 Ryan Hill 2012-01-29 16:14:28 UTC
Can this be applied to 4.5/4.6 please?
Comment 7 Andrey Belevantsev 2012-02-06 12:10:17 UTC
(In reply to comment #6)
> Can this be applied to 4.5/4.6 please?

Well, the patch was approved for trunk only, but it is committed for two weeks now and looks safe -- Jakub?
Comment 8 Jakub Jelinek 2012-02-06 12:26:48 UTC
Yes, it is ok for the affected branches if bootstrap/regtest passes.
Comment 9 Andrey Belevantsev 2012-02-09 10:10:41 UTC
Author: abel
Date: Thu Feb  9 10:10:36 2012
New Revision: 184038

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184038
Log:
2012-02-09  Andrey Belevantsev  <abel@ispras.ru>

        Backport from mainline
        2012-01-20  Andrey Belevantsev  <abel@ispras.ru>

        PR target/51106
        * function.c (instantiate_virtual_regs_in_insn): Use
        delete_insn_and_edges when removing a wrong asm insn.

        Backport from mainline
        2012-01-20  Jakub Jelinek  <jakub@redhat.com>

        PR target/51106
        * gcc.dg/torture/pr51106-1.c: New test.
        * gcc.dg/torture/pr51106-2.c: New test.


Added:
    branches/gcc-4_6-branch/gcc/testsuite/gcc.dg/torture/pr51106-1.c
    branches/gcc-4_6-branch/gcc/testsuite/gcc.dg/torture/pr51106-2.c
Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/function.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
Comment 10 Andrey Belevantsev 2012-02-09 10:17:59 UTC
Author: abel
Date: Thu Feb  9 10:17:55 2012
New Revision: 184040

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=184040
Log:
2012-02-09  Andrey Belevantsev  <abel@ispras.ru>

        Backport from mainline
        2012-01-20  Andrey Belevantsev  <abel@ispras.ru>

        PR target/51106
        * function.c (instantiate_virtual_regs_in_insn): Use
        delete_insn_and_edges when removing a wrong asm insn.

        Backport from mainline
        2012-01-20  Jakub Jelinek  <jakub@redhat.com>

        PR target/51106
        * gcc.dg/torture/pr51106-1.c: New test.
        * gcc.dg/torture/pr51106-2.c: New test.


Added:
    branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/torture/pr51106-1.c
    branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/torture/pr51106-2.c
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/function.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
Comment 11 Andrey Belevantsev 2012-02-09 10:18:40 UTC
Tested ok, so committed to 4.5/4.6, fixed, closed.
Comment 12 Oleg Endo 2012-03-25 12:46:01 UTC
(In reply to comment #11)
> Tested ok, so committed to 4.5/4.6, fixed, closed.

I've noticed that on trunk rev 185768 the pr51106-2.c test case fails for the SH target with:

sh_tmp.cpp: In function 'bar':
sh_tmp.cpp:163:3: warning: asm operand 0 probably doesn't match constraints [enabled by default]
sh_tmp.cpp:163:3: error: impossible constraint in 'asm'
sh_tmp.cpp:167:1: internal compiler error: in purge_dead_edges, at cfgrtl.c:2462


Using built-in specs.
COLLECT_GCC=sh-elf-gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/sh-elf/4.8.0/lto-wrapper
Target: sh-elf
Configured with: ../gcc-trunk/configure --target=sh-elf --prefix=/usr/local --enable-languages=c,c++ --enable-multilib --disable-libssp --disable-nls --disable-werror --enable-lto --with-newlib --with-gnu-as --with-gnu-ld --with-system-zlib
Thread model: single
gcc version 4.8.0 20120324 (experimental) (GCC)
Comment 13 Andrey Belevantsev 2012-03-27 12:42:06 UTC
It's failing not only on sh, but on x86-64 too, started with (no surprise here) r185564:

    2012-03-20  Richard Guenther  <rguenther@suse.de>

        * tree-pass.h (pass_rtl_eh): Remove.

Richard, the issue here was that when removing the wrong asm jump, we need also to fixup the cfg.  This can be done by the purge_dead_edges call because as Jakub mentioned in comment #3 the basic block with the asm has two successor edges, one of them is fallthru.  After your patch (probably because of the new call to cleanup_cfg) there is no more fallthru edge but a single edge.  Which blows up purge_dead_edges called from delete_insn_and_edges.  So probably purge_dead_edges should be fixed to expect not having a fallthru edge -- Jakub may have more context here.
Comment 14 Dominique d'Humieres 2012-03-27 13:02:41 UTC
(In reply to comment #13)
> It's failing not only on sh, but on x86-64 too, started with (no surprise here)
> r185564: ...

Is it related to pr52650?
Comment 15 Andrey Belevantsev 2012-03-27 13:06:41 UTC
(In reply to comment #14)
> (In reply to comment #13)
> > It's failing not only on sh, but on x86-64 too, started with (no surprise here)
> > r185564: ...
> 
> Is it related to pr52650?

Yes, I would say it is the same.  It's strange that Richard didn't see this in his testing.
Comment 16 Andrey Belevantsev 2012-03-27 13:28:03 UTC
So, something like the below patch, or even better -- as we want to fold all RTL-build related pseudo passes into expand, make pass_instantiate_virtual_regs also the expand part (thus, until the into_cfglayout pass), and then the new cleanup_cfg will nicely work at the end of expand. 

diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index b86cc74..0c27d11 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -2261,7 +2261,7 @@ purge_dead_edges (basic_block bb)
   edge e;
   rtx insn = BB_END (bb), note;
   bool purged = false;
-  bool found;
+  bool found, fallthru;
   edge_iterator ei;

   if (DEBUG_INSN_P (insn) && insn != BB_HEAD (bb))
@@ -2433,16 +2433,22 @@ purge_dead_edges (basic_block bb)
      as these are only created by conditional branches.  If we find such an
      edge we know that there used to be a jump here and can then safely
      remove all non-fallthru edges.  */
-  found = false;
+  found = fallthru = false;
   FOR_EACH_EDGE (e, ei, bb->succs)
-    if (! (e->flags & (EDGE_COMPLEX | EDGE_FALLTHRU)))
-      {
+    {
+      if (! (e->flags & (EDGE_COMPLEX | EDGE_FALLTHRU)))
        found = true;
-       break;
-      }
-
+      if (e->flags & (EDGE_FALLTHRU | EDGE_FAKE))
+       fallthru = true;
+    }
   if (!found)
     return purged;
+  if (!fallthru)
+    {
+      gcc_assert (single_succ_p (bb));
+      single_succ_edge (bb)->flags |= EDGE_FALLTHRU;
+      return purged;
+    }

   /* Remove all but the fake and fallthru edges.  The fake edge may be
      the only successor for this block in the case of noreturn
Comment 17 rguenther@suse.de 2012-03-27 13:50:57 UTC
On Tue, 27 Mar 2012, abel at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51106
> 
> --- Comment #16 from Andrey Belevantsev <abel at gcc dot gnu.org> 2012-03-27 13:28:03 UTC ---
> So, something like the below patch, or even better -- as we want to fold all
> RTL-build related pseudo passes into expand, make pass_instantiate_virtual_regs
> also the expand part (thus, until the into_cfglayout pass), and then the new
> cleanup_cfg will nicely work at the end of expand. 
> 
> diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
> index b86cc74..0c27d11 100644
> --- a/gcc/cfgrtl.c
> +++ b/gcc/cfgrtl.c
> @@ -2261,7 +2261,7 @@ purge_dead_edges (basic_block bb)
>    edge e;
>    rtx insn = BB_END (bb), note;
>    bool purged = false;
> -  bool found;
> +  bool found, fallthru;
>    edge_iterator ei;
> 
>    if (DEBUG_INSN_P (insn) && insn != BB_HEAD (bb))
> @@ -2433,16 +2433,22 @@ purge_dead_edges (basic_block bb)
>       as these are only created by conditional branches.  If we find such an
>       edge we know that there used to be a jump here and can then safely
>       remove all non-fallthru edges.  */
> -  found = false;
> +  found = fallthru = false;
>    FOR_EACH_EDGE (e, ei, bb->succs)
> -    if (! (e->flags & (EDGE_COMPLEX | EDGE_FALLTHRU)))
> -      {
> +    {
> +      if (! (e->flags & (EDGE_COMPLEX | EDGE_FALLTHRU)))
>         found = true;
> -       break;
> -      }
> -
> +      if (e->flags & (EDGE_FALLTHRU | EDGE_FAKE))
> +       fallthru = true;
> +    }
>    if (!found)
>      return purged;
> +  if (!fallthru)
> +    {
> +      gcc_assert (single_succ_p (bb));
> +      single_succ_edge (bb)->flags |= EDGE_FALLTHRU;
> +      return purged;
> +    }
> 
>    /* Remove all but the fake and fallthru edges.  The fake edge may be
>       the only successor for this block in the case of noreturn
> 

Looks reasonable.  Though I think that whoever removed the fallthru
edge should have adjusted the flags on the others.
Comment 18 Andrey Belevantsev 2012-03-27 14:08:23 UTC
(In reply to comment #17)
> Looks reasonable.  Though I think that whoever removed the fallthru
> edge should have adjusted the flags on the others.
That's simply delete_basic_block at cfgcleanup.c:2612 -- we have block 2 with two successors 4 and 5, and block 4 is trivially dead (empty, no succ, etc.), so when removing block 4 we just remove the 2->4 edge which is the only fallthru one.  Which seems fine as the asm in question is seen by the rest of code as an unconditional jump then.  Only when we remove it, we get no jump and still no fallthru bit, which confuses purge_dead_edges.
Comment 19 rguenther@suse.de 2012-03-28 07:59:59 UTC
On Tue, 27 Mar 2012, abel at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51106
> 
> --- Comment #18 from Andrey Belevantsev <abel at gcc dot gnu.org> 2012-03-27 14:08:23 UTC ---
> (In reply to comment #17)
> > Looks reasonable.  Though I think that whoever removed the fallthru
> > edge should have adjusted the flags on the others.
> That's simply delete_basic_block at cfgcleanup.c:2612 -- we have block 2 with
> two successors 4 and 5, and block 4 is trivially dead (empty, no succ, etc.),
> so when removing block 4 we just remove the 2->4 edge which is the only
> fallthru one.  Which seems fine as the asm in question is seen by the rest of
> code as an unconditional jump then.  Only when we remove it, we get no jump and
> still no fallthru bit, which confuses purge_dead_edges.

I see.
Comment 20 davem 2012-10-27 23:08:50 UTC
This issue still exists in mainline, there seems to be no objection to Andrey's suggested fix, could someone please commit it?
Comment 21 Andrey Belevantsev 2012-10-28 18:17:16 UTC
(In reply to comment #20)
> This issue still exists in mainline, there seems to be no objection to Andrey's
> suggested fix, could someone please commit it?

Not quite, the last message was http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00161.html and then there was no consensus about the way to proceed.
Comment 22 Dominique d'Humieres 2012-11-27 15:46:07 UTC
Does revision 193846 fix this PR or not?
Comment 23 Jakub Jelinek 2013-03-22 14:43:34 UTC
GCC 4.8.0 is being released, adjusting target milestone.
Comment 24 Jakub Jelinek 2013-05-31 10:58:09 UTC
GCC 4.8.1 has been released.
Comment 25 Jakub Jelinek 2013-10-16 09:50:13 UTC
GCC 4.8.2 has been released.
Comment 26 Richard Biener 2014-05-22 09:02:32 UTC
GCC 4.8.3 is being released, adjusting target milestone.
Comment 27 Jakub Jelinek 2014-12-19 13:28:38 UTC
GCC 4.8.4 has been released.
Comment 28 Richard Biener 2015-06-23 08:27:07 UTC
Fixed according to dup.