[Bug rtl-optimization/56957] New: [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[Bug rtl-optimization/56957] New: [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[schwab@linux-m68k.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

             Bug #: 56957
           Summary: [4.9 regression] ICE in add_insn_after, at
                    emit-rtl.c:3783
    Classification: Unclassified
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: schwab@linux-m68k.org
                CC: steven@gcc.gnu.org
            Target: ia64-*-*


spawn /usr/local/gcc/gcc-20130414/Build/gcc/xgcc
-B/usr/local/gcc/gcc-20130414/Build/gcc/ -fno-diagnostics-show-caret -O3
-fomit-frame-pointer -funroll-loops -w -c -o 20070603-2.o
/usr/local/gcc/gcc-20130414/gcc/testsuite/gcc.c-torture/compile/20070603-2.c
/usr/local/gcc/gcc-20130414/gcc/testsuite/gcc.c-torture/compile/20070603-2.c:
In function 'f':
/usr/local/gcc/gcc-20130414/gcc/testsuite/gcc.c-torture/compile/20070603-2.c:9:1:
internal compiler error: in add_insn_after, at emit-rtl.c:3783
0x400000000046b74f add_insn_after(rtx_def*, rtx_def*, basic_block_def*)
        ../../gcc/emit-rtl.c:3783
0x40000000008c536f sel_gen_insn_from_expr_after(_expr*, vinsn_def*, int,
rtx_def*)
        ../../gcc/sel-sched-ir.c:1401
0x40000000008d3b6f emit_insn_from_expr_after
        ../../gcc/sel-sched.c:4520
0x40000000008e5a3f schedule_expr_on_boundary
        ../../gcc/sel-sched.c:5523
0x40000000008e9d9f fill_insns
        ../../gcc/sel-sched.c:5646
0x40000000008e9d9f schedule_on_fences
        ../../gcc/sel-sched.c:7411
0x40000000008e9d9f sel_sched_region_2
        ../../gcc/sel-sched.c:7545
0x40000000008f19ef sel_sched_region_1
        ../../gcc/sel-sched.c:7658
0x40000000008f19ef sel_sched_region(int)
        ../../gcc/sel-sched.c:7685
0x40000000008f34af run_selective_scheduling()
        ../../gcc/sel-sched.c:7761
0x4000000000dadaaf ia64_reorg
        ../../gcc/config/ia64/ia64.c:9728
0x4000000000878f0f rest_of_handle_machine_reorg
        ../../gcc/reorg.c:3904

Caused by r197942.

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.9.0

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2013-04-16
                 CC|steven at gcc dot gnu.org   |
         AssignedTo|unassigned at gcc dot       |steven at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

--- Comment #1 from Steven Bosscher <steven at gcc dot gnu.org> 2013-04-16 20:35:02 UTC ---
Bug in the selective scheduler, merely exposed by my patch:

Breakpoint 1, fancy_abort (file=0x11017130 "../../trunk/gcc/emit-rtl.c",
line=3840, function=0x110174f8 <add_insn_after_nobb(rtx_def*,
rtx_def*)::__FUNCTION__> "add_insn_after_nobb") at
../../trunk/gcc/diagnostic.c:1177
1177      internal_error ("in %s, at %s:%d", function, trim_filename (file),
line);
(gdb) up
#1  0x00000000103e616c in add_insn_after_nobb (insn=0x3fffb5da6150,
after=0x3fffb5da4e78) at ../../trunk/gcc/emit-rtl.c:3840
3840      gcc_assert (!optimize || !INSN_DELETED_P (after));
(gdb) p after
$1 = (rtx) 0x3fffb5da4e78
(gdb) p debug_rtx(after)
(insn/v 303 30 32 10 (set (reg:DF 165 f37 [421])
        (unspec:DF [
                (mem:DF (reg/f:DI 18 r18 [420]) [2 *a_7(D) S8 A64])
            ] UNSPEC_LDA)) 13 {movdf_advanced}
     (nil))
$2 = void
(gdb) p INSN_DELETED_P (after)
$3 = 1
(gdb)

The selective scheduler is trying to re-emit an insn it had previously
deleted permanently.

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
                 CC|                            |abel at gcc dot gnu.org
         AssignedTo|steven at gcc dot gnu.org   |unassigned at gcc dot
                   |                            |gnu.org

--- Comment #2 from Steven Bosscher <steven at gcc dot gnu.org> 2013-04-16 20:54:17 UTC ---
Breakpoint 5, sel_remove_insn (insn=0x3fffb5da4e78, only_disconnect=false,
full_tidying=false) at ../../trunk/gcc/sel-sched-ir.c:3938
3938          delete_insn (insn);
1: debug_rtx(insn) = (insn 303 190 191 10 (set (reg:DF 165 f37 [421])
        (unspec:DF [
                (mem:DF (reg/f:DI 18 r18 [420]) [2 *a_7(D) S8 A64])
            ] UNSPEC_LDA)) 13 {movdf_advanced}
     (nil))
void
(gdb) up
#1  0x0000000010843984 in remove_insn_from_stream (insn=0x3fffb5da4e78,
only_disconnect=false) at ../../trunk/gcc/sel-sched.c:6042
6042      sel_remove_insn (insn, only_disconnect, false);
(gdb) up
#2  0x0000000010843af0 in move_op_orig_expr_found (insn=0x3fffb5da4e78,
expr=0x115f9fd8, lparams=0x3fffffffda78, static_params=0x3fffffffdaa8) at
../../trunk/gcc/sel-sched.c:6066
6066      remove_insn_from_stream (insn, only_disconnect);
gdb) p only_disconnect
$19 = false
(gdb) l 6060
6055      moveop_static_params_p params = (moveop_static_params_p)
static_params;
6056
6057      copy_expr_onside (params->c_expr, INSN_EXPR (insn));
6058      track_scheduled_insns_and_blocks (insn);
6059      insn_emitted = handle_emitting_transformations (insn, expr, params);
6060      only_disconnect = (params->uid == INSN_UID (insn)
6061                         && ! insn_emitted  && ! EXPR_WAS_CHANGED (expr));
6062
6063      /* Mark that we've disconnected an insn.  */
6064      if (only_disconnect)
(gdb) p params->uid
$20 = 303
(gdb) p insn_emitted
$21 = false
(gdb) p EXPR_WAS_CHANGED(expr)
$22 = true

No idea what to make of this. sel-sched deliberately chooses to discard
the insn instead of only disconnecting it, but it re-emits the insn
later anyway.  That's wrong.

CC sel-sched guru.

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

Steven Bosscher <steven at gcc dot gnu.org> changed:

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

--- Comment #3 from Steven Bosscher <steven at gcc dot gnu.org> 2013-04-16 21:36:32 UTC ---
Work-around:

--- sel-sched-ir.c      2013-04-16 21:51:48.000000000 +0200
+++ sel-sched-ir.c      2013-04-16 23:35:30.000000000 +0200
@@ -3935,7 +3935,7 @@
     remove_insn (insn);
   else
     {
-      delete_insn (insn);
+      remove_insn (insn);
       clear_expr (INSN_EXPR (insn));
     }


But that makes sel-sched leak DF caches again...

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[abel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

--- Comment #4 from Andrey Belevantsev <abel at gcc dot gnu.org> 2013-04-17 05:14:44 UTC ---
(In reply to comment #2)
> Breakpoint 5, sel_remove_insn (insn=0x3fffb5da4e78, only_disconnect=false,
> full_tidying=false) at ../../trunk/gcc/sel-sched-ir.c:3938
> 3938          delete_insn (insn);
> 1: debug_rtx(insn) = (insn 303 190 191 10 (set (reg:DF 165 f37 [421])
>         (unspec:DF [
>                 (mem:DF (reg/f:DI 18 r18 [420]) [2 *a_7(D) S8 A64])
>             ] UNSPEC_LDA)) 13 {movdf_advanced}
>      (nil))

You are right, this is not supposed to happen.  The only_disconnect business is
for the unification and transformation support, when you have possibly several
insns as sources of the one chosen for scheduling.  Thus, you need to delete
all sources but the one you're going to move upwards, and even this chosen one
should be deleted if it was changed while moving.

The insn in question is a speculative insn possibly generated earlier, so ...

> (gdb) p EXPR_WAS_CHANGED(expr)
> $22 = true

... if this is true, we are supposed to emit the new insn obviously with the
new id.  The issue is that we then decide to emit the very same insn.  I will
take a look.

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[abel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

Andrey Belevantsev <abel at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |abel at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #5 from Andrey Belevantsev <abel at gcc dot gnu.org> 2013-04-17 06:52:47 UTC ---
Created attachment 29886
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29886
proposed patch

Easy enough, we can have a speculation transformation that does not change insn
at all (e.g. we're asked to speculate an insn already speculated, so we just
changed the speculation probability, not the pattern itself), but
EXPR_WAS_CHANGED only tests that the transformation history vector is
non-empty, so it would report changes have actually happened.  So checking
additionally that the oldest insn form (last vector element) has the same
INSN_ID as the one of the current expr fixes the test.  I will throw this to
our Itanium for the full testing.

Steven, thanks for your insn emitting patches.  It was not that easy to catch
that kind of issues earlier, AFAIR we noticed it via corruption of our own
structures and we needed to trace that back to the offending move....

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

--- Comment #6 from Steven Bosscher <steven at gcc dot gnu.org> 2013-04-18 09:08:36 UTC ---
(In reply to comment #5)
> Created attachment 29886 [details]
> proposed patch

You should probably make this a static inline function and add a comment
with a rationale ;-)

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[abel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

--- Comment #7 from Andrey Belevantsev <abel at gcc dot gnu.org> 2013-04-18 09:18:17 UTC ---
(In reply to comment #6)
> (In reply to comment #5)
> > Created attachment 29886 [details]
> > proposed patch
> 
> You should probably make this a static inline function and add a comment
> with a rationale ;-)

Actually I've made the full-blown function, the reason being that I forgot that
we keep the history vector sorted, so it's not enough to check only the last
element, you'd need to check all of them.  Took some time from digging this
from Itanium slow testing though :-) I will add a comment, sure.

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[abel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

--- Comment #8 from Andrey Belevantsev <abel at gcc dot gnu.org> 2013-04-22 09:48:37 UTC ---
I see some more ICEs of the same type while testing on ia64, so the patch might
not be enough in general, though it fixes the original test.  I will need to
spend more time for analyzing those ICEs.

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[abel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

Andrey Belevantsev <abel at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #29886|0                           |1
        is obsolete|                            |

--- Comment #9 from Andrey Belevantsev <abel at gcc dot gnu.org> 2013-04-25 10:03:38 UTC ---
Created attachment 29937
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29937
new patch

Actually, on more thought I couldn't recall why did we need all the
insn_emitted and EXPR_WAS_CHANGED stuff (that was 5 years ago).  The code
should decide whether do we need to move the scheduling insn from below and
leave the same UID or we need to emit the new insn with the new UID.  The
current code says that the insn needs to have the same UID as the above
resulting expression, we shouldn't create any extra stuff (renaming copies,
speculative checks) at the place of the original insn, and the insn itself
should not be changed while it was being propagated up to the place of
scheduling.  However, for the last two cases (new insns created, scheduling
insn changed) the UIDs will not match, as the insn pattern would be changed
while moving up, so their check doesn't add anything new to the UID check.  And
indeed, the attached patch that only checks for UIDs passes all the tests for
me.

I will discuss this more with Alexander and will commit after additional
testing on x86_64 as appropriate.

[Bug rtl-optimization/56957] [4.9 regression] ICE in add_insn_after, at emit-rtl.c:3783

[abel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56957

Andrey Belevantsev <abel at gcc dot gnu.org> changed:

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

--- Comment #10 from Andrey Belevantsev <abel at gcc dot gnu.org> 2013-04-30 10:58:02 UTC ---
Fixed with:

Author: abel
Date: Tue Apr 30 10:55:52 2013
New Revision: 198449

URL: http://gcc.gnu.org/viewcvs?rev=198449&root=gcc&view=rev
Log:
        PR rtl-optimization/56957
        PR rtl-optimization/57105

        * sel-sched.c (move_op_orig_expr_found): Remove insn_emitted
        variable.  Use just INSN_UID for determining whether an insn
        should be only disconnected from the insn stream.
        * sel-sched-ir.h (EXPR_WAS_CHANGED): Remove.

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

Added:
    trunk/gcc/testsuite/gcc.dg/pr57105.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/sel-sched-ir.h
    trunk/gcc/sel-sched.c
    trunk/gcc/testsuite/ChangeLog