[Bug regression/56131] New: [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[Bug regression/56131] New: [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[mikpe at it dot uu.se]

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

             Bug #: 56131
           Summary: [4.8 regression] gcc.dg/pr56035.c ICEs gcc on
                    sparc-linux
    Classification: Unclassified
           Product: gcc
           Version: 4.7.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: regression
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: mikpe@it.uu.se


The recently added gcc.dg/pr56035.c test case fails on sparc-linux:

+FAIL: gcc.dg/pr56035.c (internal compiler error)
+FAIL: gcc.dg/pr56035.c (test for excess errors)

Compiling it with an x86_64-linux to sparc-linux cross-compiler shows:

> /tmp/objdir/gcc/xgcc -B/tmp/objdir/gcc/ -O1 -ftree-vectorize -fcse-follow-jumps -fstrict-overflow -S pr56035.c 
pr56035.c: In function 'f':
pr56035.c:35:1: internal compiler error: Segmentation fault
 }
 ^
0x70fa9f crash_signal
        /tmp/gcc-4.8-20130127/gcc/toplev.c:332
0x4cdc96 delete_insn(rtx_def*)
        /tmp/gcc-4.8-20130127/gcc/cfgrtl.c:151
0x6210a8 delete_related_insns(rtx_def*)
        /tmp/gcc-4.8-20130127/gcc/jump.c:1243
0x6215e7 redirect_jump_2(rtx_def*, rtx_def*, rtx_def*, int, int)
        /tmp/gcc-4.8-20130127/gcc/jump.c:1574
0x6216b2 redirect_jump(rtx_def*, rtx_def*, int)
        /tmp/gcc-4.8-20130127/gcc/jump.c:1533
0x6c4464 dbr_schedule(rtx_def*)
        /tmp/gcc-4.8-20130127/gcc/reorg.c:3722
0x6c52bf rest_of_handle_delay_slots
        /tmp/gcc-4.8-20130127/gcc/reorg.c:3891
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.

This ICE is different from the one in PR56035.

Reverting the PR56035 fix in r195462 makes no difference.

gcc-4.7 does not ICE on this test case.

[Bug regression/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[mikpe at it dot uu.se]

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

Mikael Pettersson <mikpe at it dot uu.se> changed:

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

--- Comment #1 from Mikael Pettersson <mikpe at it dot uu.se> 2013-01-28 21:58:49 UTC ---
Started with Tom de Vries' "Remove dead labels to increase superblock scope"
patch in r186451:
http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00875.html
http://gcc.gnu.org/ml/gcc-cvs/2012-04/msg00402.html

[Bug regression/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

vries at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-01-28
     Ever Confirmed|0                           |1

--- Comment #2 from vries at gcc dot gnu.org 2013-01-28 23:15:32 UTC ---
I can reproduce this on a x86_64-linux to sparc-linux cross-compiler.

The segv is due to bb being null and dereferenced:
...
Program received signal SIGSEGV, Segmentation fault.
0x000000000066145b in delete_insn (insn=0x7ffff6b1a480) at gcc/cfgrtl.c:150
150              BB_HEAD (bb) = bb_note;
(gdb) p bb
$4 = (basic_block) 0x0
...

the bb var is set here:
...
138          basic_block bb = BLOCK_FOR_INSN (insn);
...

with insn being:
...
(gdb) call debug_rtx (insn)
(code_label 22 21 23 3 ("lbl1") [0 uses])
...

[Bug regression/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

--- Comment #3 from vries at gcc dot gnu.org 2013-01-29 00:28:38 UTC ---
Using NOTE_BASIC_BLOCK instead of BLOCK_FOR_INSN on bb_note allows us to get
the bb.

This tentative patch:
...
Index: cfgrtl.c
===================================================================
--- cfgrtl.c (revision 195240)
+++ cfgrtl.c (working copy)
@@ -144,9 +144,11 @@ delete_insn (rtx insn)
       NOTE_DELETED_LABEL_NAME (insn) = name;

       if (bb_note != NULL_RTX && NOTE_INSN_BASIC_BLOCK_P (bb_note)
-          && BLOCK_FOR_INSN (bb_note) == bb)
+          && (bb == NULL
+          || NOTE_BASIC_BLOCK (bb_note) == bb))
         {
           reorder_insns_nobb (insn, insn, bb_note);
+          bb = NOTE_BASIC_BLOCK (bb_note);
           BB_HEAD (bb) = bb_note;
           if (BB_END (bb) == bb_note)
         BB_END (bb) = insn;
...

gives the following result:
...
(note 23 221 22 [bb 3] NOTE_INSN_BASIC_BLOCK)
(note 22 23 67 ("lbl1") NOTE_INSN_DELETED_LABEL 3)
...

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|regression                  |rtl-optimization
   Target Milestone|---                         |4.8.0

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[danglin at gcc dot gnu.org]

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

John David Anglin <danglin at gcc dot gnu.org> changed:

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

--- Comment #4 from John David Anglin <danglin at gcc dot gnu.org> 2013-01-29 17:01:20 UTC ---
Also seen on hppa64-hp-hpux11.11.

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

--- Comment #5 from vries at gcc dot gnu.org 2013-01-29 19:34:08 UTC ---
A more structured version of the patch:
...
Index: cfgrtl.c
===================================================================
--- cfgrtl.c (revision 195240)
+++ cfgrtl.c (working copy)
@@ -135,7 +135,7 @@ delete_insn (rtx insn)
       if (! can_delete_label_p (insn))
     {
       const char *name = LABEL_NAME (insn);
-      basic_block bb = BLOCK_FOR_INSN (insn);
+      basic_block bb = NULL, label_bb = BLOCK_FOR_INSN (insn);
       rtx bb_note = NEXT_INSN (insn);

       really_delete = false;
@@ -143,8 +143,17 @@ delete_insn (rtx insn)
       NOTE_KIND (insn) = NOTE_INSN_DELETED_LABEL;
       NOTE_DELETED_LABEL_NAME (insn) = name;

-      if (bb_note != NULL_RTX && NOTE_INSN_BASIC_BLOCK_P (bb_note)
-          && BLOCK_FOR_INSN (bb_note) == bb)
+      if (bb_note != NULL_RTX
+          && NOTE_INSN_BASIC_BLOCK_P (bb_note))
+        bb = NOTE_BASIC_BLOCK (bb_note);
+
+      /* If the note following the label starts a basic block, and the
+         label is a member of the same basic block, interchange the two.
+         If the label is not marked with a bb, assume it's the same bb.  */
+      */
+      if (bb != NULL
+          && (bb == label_bb
+          || label_bb == NULL))
         {
           reorder_insns_nobb (insn, insn, bb_note);
           BB_HEAD (bb) = bb_note;
...

I'll try to bootstrap this patch on x86_64 coming weekend when I'll be back
home. I've also asked for access to the gcc compile farm to be able to test
this on a sparc machine.

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[mikpe at it dot uu.se]

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

--- Comment #6 from Mikael Pettersson <mikpe at it dot uu.se> 2013-01-30 22:20:19 UTC ---
(In reply to comment #5)
> A more structured version of the patch:

After fixing the obvious syntax error

> +         If the label is not marked with a bb, assume it's the same bb.  */
> +      */

I tested this on x86_64-linux and sparc64-linux.  On x86_64 there were no test
suite changes, on sparc64 I got

-FAIL: gcc.dg/pr56035.c (internal compiler error)
-FAIL: gcc.dg/pr56035.c (test for excess errors)

which is good, but I also got

+FAIL: g++.old-deja/g++.pt/memtemp52.C -std=c++11 (test for excess errors)

g++.log shows it failed due to an exit 1 from xg++, without diagnostic. 
Re-running that one test manually doesn't fail so I guess it was just a fluke.

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[ro at gcc dot gnu.org]

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

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|sparc-linux                 |sparc-*-*
                 CC|                            |ro at gcc dot gnu.org

--- Comment #7 from Rainer Orth <ro at gcc dot gnu.org> 2013-02-04 13:08:35 UTC ---
Same on 32-bit Solaris/SPARC only, 64-bit is fine.

  Rainer

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

vries at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #8 from vries at gcc dot gnu.org 2013-02-04 15:18:50 UTC ---
Mikael,

> I tested this on x86_64-linux and sparc64-linux.  On x86_64 there were no test
> suite changes,

Thanks for testing this patch.

Was the run on x86_64 a bootstrap-and-test? If not, I'll start one now.

Thanks,
- Tom

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[mikpe at it dot uu.se]

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

--- Comment #9 from Mikael Pettersson <mikpe at it dot uu.se> 2013-02-04 15:39:09 UTC ---
(In reply to comment #8)
> Mikael,
> 
> > I tested this on x86_64-linux and sparc64-linux.  On x86_64 there were no test
> > suite changes,
> 
> Thanks for testing this patch.
> 
> Was the run on x86_64 a bootstrap-and-test? If not, I'll start one now.

Both of them were full bootstrap-and-test runs.

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

vries at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ice-on-valid-code, patch

--- Comment #10 from vries at gcc dot gnu.org 2013-02-04 17:35:13 UTC ---
submitted patch: http://gcc.gnu.org/ml/gcc-patches/2013-02/msg00122.html

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

--- Comment #11 from vries at gcc dot gnu.org 2013-02-06 08:53:41 UTC ---
Author: vries
Date: Wed Feb  6 08:53:32 2013
New Revision: 195784

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195784
Log:
2013-02-06  Tom de Vries  <tom@codesourcery.com>

    PR rtl-optimization/56131
    * cfgrtl.c (delete_insn): Use NOTE_BASIC_BLOCK instead of BLOCK_FOR_INSN
    to get the bb of a NOTE_INSN_BASIC_BLOCK.  Handle the case that the bb
    of the label is NULL.  Add comment.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cfgrtl.c

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

vries at gcc dot gnu.org changed:

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

--- Comment #12 from vries at gcc dot gnu.org 2013-02-06 12:24:05 UTC ---
patch checked in, ICE triggered on existing test-case to no need for extra
test-case.

Marking resolved-fixed.

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[steven at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
                 CC|                            |steven at gcc dot gnu.org
         Resolution|FIXED                       |

--- Comment #13 from Steven Bosscher <steven at gcc dot gnu.org> 2013-02-21 10:29:25 UTC ---
The fix for this PR is wrong.

Nothing even trying to look at the CFG after freeing it, so the looks at
BLOCK_FOR_INSN in delete_insn are non-sense. Looking for the basic block
anywhere at all at this point makes no sense, basic block contents and
boundaries are not maintained and may be scrambled enough to make even
the basic block notes unreliable.

Also, "If the label is not marked with a bb, assume it's the same bb"
is wrong if the label is a marker for a constant pool or a jump table.

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

--- Comment #14 from vries at gcc dot gnu.org 2013-02-22 10:30:06 UTC ---
Steven,

thanks for the comments.

> Nothing even trying to look at the CFG after freeing it, so the looks at
> BLOCK_FOR_INSN in delete_insn are non-sense.

AFAIU now from
http://gcc.gnu.org/onlinedocs/gccint/Maintaining-the-CFG.html#Maintaining-the-CFG,
BLOCK_FOR_INSN == NULL shows whether the CFG has been freed or not, so I'd say
it makes sense to test for equality of BLOCK_FOR_INSN when non-NULL.

> Looking for the basic block
> anywhere at all at this point makes no sense,

If I understand you correctly with 'this point' you mean 'after the CFG has
been freed'?

> basic block contents and
> boundaries are not maintained and may be scrambled enough to make even
> the basic block notes unreliable.

OK. I've added a comment in insn-notes.def in the patch below to make that more
explicit.

> Also, "If the label is not marked with a bb, assume it's the same bb"
> is wrong if the label is a marker for a constant pool or a jump table.

OK, fixed in patch below.

Does this patch address all your concerns?
...
Index: gcc/insn-notes.def
===================================================================
--- gcc/insn-notes.def (revision 195874)
+++ gcc/insn-notes.def (working copy)
@@ -70,7 +70,9 @@ INSN_NOTE (CALL_ARG_LOCATION)

 /* Record the struct for the following basic block.  Uses
    NOTE_BASIC_BLOCK.  FIXME: Redundant with the basic block pointer
-   now included in every insn.  */
+   now included in every insn.  NOTE: If there's no CFG anymore, in other
words,
+   if BLOCK_FOR_INSN () == NULL, NOTE_BASIC_BLOCK cannot be considered
reliable
+   anymore.  */
 INSN_NOTE (BASIC_BLOCK)

 /* Mark the inflection point in the instruction stream where we switch
Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c (revision 195874)
+++ gcc/cfgrtl.c (working copy)
@@ -135,7 +135,7 @@ delete_insn (rtx insn)
       if (! can_delete_label_p (insn))
     {
       const char *name = LABEL_NAME (insn);
-      basic_block bb, label_bb = BLOCK_FOR_INSN (insn);
+      basic_block bb = BLOCK_FOR_INSN (insn);
       rtx bb_note = NEXT_INSN (insn);

       really_delete = false;
@@ -144,15 +144,13 @@ delete_insn (rtx insn)
       NOTE_DELETED_LABEL_NAME (insn) = name;

       /* If the note following the label starts a basic block, and the
-         label is a member of the same basic block, interchange the two.
-         If the label is not marked with a bb, assume it's the same bb.  */
+         label is a member of the same basic block, interchange the two.  */
       if (bb_note != NULL_RTX
           && NOTE_INSN_BASIC_BLOCK_P (bb_note)
-          && (label_bb == NOTE_BASIC_BLOCK (bb_note)
-          || label_bb == NULL))
+          && bb != NULL
+          && bb == BLOCK_FOR_INSN (bb_note))
         {
           reorder_insns_nobb (insn, insn, bb_note);
-          bb = NOTE_BASIC_BLOCK (bb_note);
           BB_HEAD (bb) = bb_note;
           if (BB_END (bb) == bb_note)
         BB_END (bb) = insn;
...

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-02-22 13:17:48 UTC ---
Does that fix both this PR and the PR56242 regression?  It looks reasonable to
me.

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[steven at gcc dot gnu.org]

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

--- Comment #16 from Steven Bosscher <steven at gcc dot gnu.org> 2013-02-22 16:33:58 UTC ---
(In reply to comment #14)

Yes, iff the CFG hasn't been freed, looking at BLOCK_FOR_INSN is of
course OK. I was referring to the situation after freeing the CFG.

Adding that comment to insn-notes.def seems like a good idea.

The patch looks reasonable. Does it fix bug 56242?

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

--- Comment #17 from vries at gcc dot gnu.org 2013-02-22 22:37:04 UTC ---
> patch below.

Bootstrapped and reg-tested on x86_64 (ada inclusive).

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

vries at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dant at picochip dot com

--- Comment #18 from vries at gcc dot gnu.org 2013-02-25 09:30:25 UTC ---
> Does that fix both this PR and the PR56242 regression?

PR56242 happens when we're deleting an undeletable label (i.e. turning it into
an deleted label note) and reordering that note in an insn stream with
SEQUENCEs. This fix makes sure that the reordering is done only when we have a
CFG.

PR56242 has been reported for hppa. For that target, the CFG is not recomputed
in pass_machine_reorg (For all targets, the CFG is freed before
pass_machine_reorg). That means that the fix has the effect that PR56242 won't
trigger any more on hppa.

The more general question then is whether there are targets that both have
sequences and have a CFG at the same time.

I'll make an attempt at answering this question.

For all these targets, we recompute the CFG at the start of pass_machine_reorg:
...
$ egrep '(compute|free)_bb_for_insn' gcc/config/*/*
gcc/config/arm/arm.c:  compute_bb_for_insn ();
gcc/config/bfin/bfin.c:  compute_bb_for_insn ();
gcc/config/c6x/c6x.c:  compute_bb_for_insn ();
gcc/config/frv/frv.c:  compute_bb_for_insn ();
gcc/config/i386/i386.c:  compute_bb_for_insn ();
gcc/config/ia64/ia64.c:  compute_bb_for_insn ();
gcc/config/mep/mep.c:  compute_bb_for_insn ();
gcc/config/mips/mips.c:    compute_bb_for_insn ();
gcc/config/mips/mips.c:      free_bb_for_insn ();
gcc/config/mn10300/mn10300.c:  compute_bb_for_insn ();
gcc/config/picochip/picochip.c:  compute_bb_for_insn ();
gcc/config/spu/spu.c:      compute_bb_for_insn ();
gcc/config/spu/spu.c:      free_bb_for_insn ();
gcc/config/spu/spu.c:  compute_bb_for_insn ();
gcc/config/spu/spu.c:  free_bb_for_insn ();
gcc/config/tilegx/tilegx.c:  compute_bb_for_insn ();
gcc/config/tilepro/tilepro.c:  compute_bb_for_insn ();
...
For mips, that is only briefly, before any SEQUENCE is created. For spu, it's
during the whole of pass_machine_reorg, but not after.

These targets can have sequences (disregarding frv because they're all in
comments):
...
$ egrep -c 'define_delay' gcc/config/*/* | egrep -v ':0|frv'
gcc/config/cris/cris.md:3
gcc/config/fr30/fr30.md:1
gcc/config/h8300/h8300.md:1
gcc/config/iq2000/iq2000.md:3
gcc/config/microblaze/microblaze.md:1
gcc/config/mips/mips.md:4
gcc/config/pa/pa.md:7
gcc/config/picochip/picochip.md:1
gcc/config/sh/sh.md:4
gcc/config/sparc/sparc.md:5
...

Looking for targets that occur in both lists (and ignoring mips) I find only
picochip. So, AFAIU, PR56242 might still trigger for picochip.

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[jakub at gcc dot gnu.org]

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

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-02-25 10:32:36 UTC ---
Please post the patch to gcc-patches, if there are issues on picochip, they can
be resolved later, the patch will cure the issues on the primary and secondary
architectures.

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

--- Comment #20 from vries at gcc dot gnu.org 2013-02-25 11:50:30 UTC ---
Author: vries
Date: Mon Feb 25 11:50:25 2013
New Revision: 196255

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196255
Log:
2013-02-25  Tom de Vries  <tom@codesourcery.com>

    PR rtl-optimization/56131
    * insn-notes.def (INSN_NOTE_BASIC_BLOCK): Update comment.
    * cfgrtl.c (delete_insn): Don't reorder NOTE_INSN_DELETED_LABEL and
    NOTE_INSN_BASIC_BLOCK if BLOCK_FOR_INSN == NULL.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cfgrtl.c
    trunk/gcc/insn-notes.def

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[vries at gcc dot gnu.org]

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

vries at gcc dot gnu.org changed:

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

--- Comment #21 from vries at gcc dot gnu.org 2013-02-25 11:51:18 UTC ---
Fixed

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[stevenb.gcc at gmail dot com]

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

--- Comment #22 from stevenb.gcc at gmail dot com <stevenb.gcc at gmail dot com> 2013-02-25 19:54:59 UTC ---
> Looking for targets that occur in both lists (and ignoring mips) I find only
> picochip. So, AFAIU, PR56242 might still trigger for picochip.

No. It is impossible for a target to have SEQUENCEs and a CFG at the
same time. The CFG code doesn't handle SEQUENCEs. Having SEQUENCEs for
delay slots in the CFG would result in a verify_flow_info failure,
because there would be non-jump insns (i.e. the SEQUENCE) at the end
of basic blocks ending in a branch.

Any target pretends to have a valid CFG and SEQUENCEs at the same time
is broken.

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[stevenb.gcc at gmail dot com]

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

--- Comment #23 from stevenb.gcc at gmail dot com <stevenb.gcc at gmail dot com> 2013-02-25 19:59:09 UTC ---
> For all these targets, we recompute the CFG at the start of pass_machine_reorg:
> ...
> $ egrep '(compute|free)_bb_for_insn' gcc/config/*/*
> gcc/config/arm/arm.c:  compute_bb_for_insn ();
> gcc/config/bfin/bfin.c:  compute_bb_for_insn ();
> gcc/config/c6x/c6x.c:  compute_bb_for_insn ();
> gcc/config/frv/frv.c:  compute_bb_for_insn ();
> gcc/config/i386/i386.c:  compute_bb_for_insn ();
> gcc/config/ia64/ia64.c:  compute_bb_for_insn ();
> gcc/config/mep/mep.c:  compute_bb_for_insn ();
> gcc/config/mips/mips.c:    compute_bb_for_insn ();
> gcc/config/mips/mips.c:      free_bb_for_insn ();
> gcc/config/mn10300/mn10300.c:  compute_bb_for_insn ();
> gcc/config/picochip/picochip.c:  compute_bb_for_insn ();
> gcc/config/spu/spu.c:      compute_bb_for_insn ();
> gcc/config/spu/spu.c:      free_bb_for_insn ();
> gcc/config/spu/spu.c:  compute_bb_for_insn ();
> gcc/config/spu/spu.c:  free_bb_for_insn ();
> gcc/config/tilegx/tilegx.c:  compute_bb_for_insn ();
> gcc/config/tilepro/tilepro.c:  compute_bb_for_insn ();

For GCC 4.9 we should make freeing the CFG late the default.
I have patches for that in my queue already.

[Bug rtl-optimization/56131] [4.8 regression] gcc.dg/pr56035.c ICEs gcc on sparc-linux

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

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

--- Comment #24 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2013-02-25 22:29:30 UTC ---
> No. It is impossible for a target to have SEQUENCEs and a CFG at the
> same time. The CFG code doesn't handle SEQUENCEs. Having SEQUENCEs for
> delay slots in the CFG would result in a verify_flow_info failure,
> because there would be non-jump insns (i.e. the SEQUENCE) at the end
> of basic blocks ending in a branch.

FWIW that's my understanding as well.  You need to shut down the CFG machinery
once you start to introduce SEQUENCEs in the insn stream.