[Bug rtl-optimization/31848] New: [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[Bug rtl-optimization/31848] New: [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[rearnsha at gcc dot gnu dot org]

Compiling genautomata with -O2 on ARM results in a program that segfaults when
run.  The problem is in  output_min_issue_delay_table.  What appears to be
happening is that the RTL loop optimizer is identifying a loop invariant and
hoisting it.  However, the particular instance of the invariant that is being
hoisted has a REG_EQUAL (const_int 0) note attached, so when the invariant gets
spilled, the value of 0 is substituted instead of the address of the array. 
Essentially, the loop seems to consist of 

x = malloc(...)

for (i=...)
{
  if (x != 0)
    t1 = x;
  else
    t1 = 0;

  t1[i] = yyy;
}

which gets optimized to 

 x = malloc(...)

for (i=...)
{
  if (x != 0)
    t1 = x;
  else
    t1 = x; // NOTE REG_EQUAL (0);

  t1[i] = yyy;
}

which is further transformed to 

x = malloc
t1 = x;  //Note REG_EQUAL (0);
for (i=...)
{
  t1[i] = yyy;
}

I've attached the following files.
foo.c: Source code that shows the problem -- look for
compressed_min_issue_delay_vect.
foo.s: Generated assembly, with annotated instruction that is invalid
foo.c.139r.loop2_invariant: which shows the hoisting of the invariant.

This regression is caused by the fix to PR rtl-optimization/31360


-- 
           Summary: [4.3 regression] Invalid loop optimization causes
                    bootstrap failure in genautomata
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rearnsha at gcc dot gnu dot org
GCC target triplet: arm-netbsdelf2


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[rearnsha at gcc dot gnu dot org]

------- Comment #1 from rearnsha at gcc dot gnu dot org  2007-05-07 00:12 -------
Created an attachment (id=13517)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13517&action=view)
compressed source


-- 


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[rearnsha at gcc dot gnu dot org]

------- Comment #2 from rearnsha at gcc dot gnu dot org  2007-05-07 00:14 -------
Created an attachment (id=13518)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13518&action=view)
assembly output (annotated)


-- 


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[rearnsha at gcc dot gnu dot org]

------- Comment #3 from rearnsha at gcc dot gnu dot org  2007-05-07 00:15 -------
Created an attachment (id=13519)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13519&action=view)
RTL dump showing invariant hoisting


-- 


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |31360
              nThis|                            |
   Target Milestone|---                         |4.3.0


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[steven at gcc dot gnu dot org]

------- Comment #4 from steven at gcc dot gnu dot org  2007-05-09 07:06 -------
Investigating...


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |steven at gcc dot gnu dot
                   |dot org                     |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2007-05-09 07:06:10
               date|                            |


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[steven at gcc dot gnu dot org]

------- Comment #5 from steven at gcc dot gnu dot org  2007-05-09 18:45 -------
The cause of this bug is obvious.  We move insns out of the loop either by
emitting the SET_SRC of an insn before the loop, or by reordering the insns
such that the invariant SET is moved into the pre-header.  The code to do this
is in loop-invariant.c:move_invariant_reg() :

      /* If the SET_DEST of the invariant insn is a pseudo, we can just move
         the insn out of the loop.  Otherwise, we have to use gen_move_insn
         to let emit_move_insn produce a valid instruction stream.  */
      if (REG_P (dest) && !HARD_REGISTER_P (dest))
        {
          emit_insn_after (gen_move_insn (dest, reg), inv->insn);
          SET_DEST (set) = reg;
          reorder_insns (inv->insn, inv->insn, BB_END (preheader));
        }
      else
        {
           ... /* Brute-force re-emit insns into the pre-header.  */
        }

Now, imagine the representative of a class of invariant insns is this one:

(gdb) next
1161              reorder_insns (inv->insn, inv->insn, BB_END (preheader));
(gdb) p debug_rtx(inv->insn)
(insn 552 551 644 57 foo.c:11152 (set (reg:SI 323)
        (reg/v/f:SI 170 [ compressed_min_issue_delay_vect ])) 161
{*arm_movsi_insn} (nil)
    (expr_list:REG_EQUAL (const_int 0 [0x0])
        (nil)))
$7 = void
(gdb) p debug_rtx(set)
(set (reg/f:SI 172 [ iftmp.331 ])
    (reg/v/f:SI 170 [ compressed_min_issue_delay_vect ]))
$6 = void
(gdb) p preheader->index
$10 = 37
(gdb) next
1195      inv->reg = reg;
(gdb) p debug_rtx(inv->insn)
(insn 552 386 423 37 foo.c:11152 (set (reg:SI 323)
        (reg/v/f:SI 170 [ compressed_min_issue_delay_vect ])) 161
{*arm_movsi_insn} (nil)
    (expr_list:REG_EQUAL (const_int 0 [0x0])
        (nil)))
$8 = void

...and we've moved the insn with note and all out of the loop, even though the
note is not valid anymore.

Interestingly, a similar bug in the old RTL loop optimizer was discussed just
this week (see http://gcc.gnu.org/ml/gcc-patches/2007-05/msg00535.html).  The
fix I'd propose for this bug is the same: Nuke the note.

We can do a little bit better than the old RTL loop optimizer, though.  We only
have to remove the note if the invariant insn we move is not always executed.

Index: loop-invariant.c
===================================================================
--- loop-invariant.c    (revision 124576)
+++ loop-invariant.c    (working copy)
@@ -1156,9 +1156,21 @@ move_invariant_reg (struct loop *loop, u
         to let emit_move_insn produce a valid instruction stream.  */
       if (REG_P (dest) && !HARD_REGISTER_P (dest))
        {
+         rtx note;
+
          emit_insn_after (gen_move_insn (dest, reg), inv->insn);
          SET_DEST (set) = reg;
          reorder_insns (inv->insn, inv->insn, BB_END (preheader));
+
+         /* If there is a REG_EQUAL note on the insn we just moved, and
+            insn is in a basic block that is not always executed, the note
+            may no longer be valid after we move the insn.
+            Note that uses in REG_EQUAL notes are taken into account in
+            the computation of invariants.  Hence it is safe to retain the
+            note even if the note contains register references.  */
+         if (! inv->always_executed
+             && (note = find_reg_note (inv->insn, REG_EQUAL, NULL_RTX)))
+           remove_note (inv->insn, note);
        }
       else
        {


Zdenek, what do you think about this?


-- 


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[rakdver at kam dot mff dot cuni dot cz]

------- Comment #6 from rakdver at kam dot mff dot cuni dot cz  2007-05-10 09:42 -------
Subject: Re:  [4.3 regression] Invalid loop optimization causes bootstrap
failure in genautomata

> Now, imagine the representative of a class of invariant insns is this one:
> 
> (gdb) next
> 1161              reorder_insns (inv->insn, inv->insn, BB_END (preheader));
> (gdb) p debug_rtx(inv->insn)
> (insn 552 551 644 57 foo.c:11152 (set (reg:SI 323)
>         (reg/v/f:SI 170 [ compressed_min_issue_delay_vect ])) 161
> {*arm_movsi_insn} (nil)
>     (expr_list:REG_EQUAL (const_int 0 [0x0])
>         (nil)))
> $7 = void

The heuristics may need some tweaking... moving reg-reg moves out of
loop seems unnecessary.  Anyway,

> ...and we've moved the insn with note and all out of the loop, even though the
> note is not valid anymore.
> Index: loop-invariant.c
> ===================================================================
> --- loop-invariant.c    (revision 124576)
> +++ loop-invariant.c    (working copy)
> @@ -1156,9 +1156,21 @@ move_invariant_reg (struct loop *loop, u
>          to let emit_move_insn produce a valid instruction stream.  */
>        if (REG_P (dest) && !HARD_REGISTER_P (dest))
>         {
> +         rtx note;
> +
>           emit_insn_after (gen_move_insn (dest, reg), inv->insn);
>           SET_DEST (set) = reg;
>           reorder_insns (inv->insn, inv->insn, BB_END (preheader));
> +
> +         /* If there is a REG_EQUAL note on the insn we just moved, and
> +            insn is in a basic block that is not always executed, the note
> +            may no longer be valid after we move the insn.
> +            Note that uses in REG_EQUAL notes are taken into account in
> +            the computation of invariants.  Hence it is safe to retain the
> +            note even if the note contains register references.  */
> +         if (! inv->always_executed
> +             && (note = find_reg_note (inv->insn, REG_EQUAL, NULL_RTX)))
> +           remove_note (inv->insn, note);
>         }
>        else
>         {
> 
> 
> Zdenek, what do you think about this?

the patch looks OK.


-- 


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[steven at gcc dot gnu dot org]

------- Comment #7 from steven at gcc dot gnu dot org  2007-05-11 00:14 -------
Patch posted...

Waiting for Richard Earnshaw to confirm whether this fixes his problem, and
also waiting for some performance numbers.


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://gcc.gnu.org/ml/gcc-
                   |                            |patches/2007-
                   |                            |05/msg00768.html


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[bonzini at gnu dot org]

------- Comment #8 from bonzini at gnu dot org  2007-05-11 07:19 -------
I started bootstrap and SPEC2000 on i686-pc-linux-gnu, I'll post the result
tomorrow hopefully (if the weekend swamps me it I might delay that). :-(


-- 

bonzini at gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bonzini at gnu dot org


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[steven at gcc dot gnu dot org]

------- Comment #9 from steven at gcc dot gnu dot org  2007-05-12 19:13 -------
Subject: Bug 31848

Author: steven
Date: Sat May 12 18:12:54 2007
New Revision: 124639

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124639
Log:
        PR rtl-optimization/31848
        * loop-invariant.c (move_invariant_reg): If we move an insn
        with a REG_EQUAL note, and that insn is not always executed,
        remove the REG_EQUAL note.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/loop-invariant.c


-- 


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[steven at gcc dot gnu dot org]

------- Comment #10 from steven at gcc dot gnu dot org  2007-05-13 09:13 -------
Should be fixed now, but I'll put this into the WAITING state pending
confirmation from the reporter that this patch really fixes the issue for him..


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |WAITING


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[bonzini at gnu dot org]

------- Comment #11 from bonzini at gnu dot org  2007-05-13 09:20 -------
I confirm the compile-time improvements from the complete patch (2%
enable-checking, 0.8% disable-checking), but there's no improvement in SPEC (no
regression either).


-- 


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

[Bug rtl-optimization/31848] [4.3 regression] Invalid loop optimization causes bootstrap failure in genautomata

[rearnsha at gcc dot gnu dot org]

------- Comment #12 from rearnsha at gcc dot gnu dot org  2007-05-16 00:06 -------
Confirmed fixed


-- 

rearnsha at gcc dot gnu dot org changed:

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


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