Patch: clear NEXT_INSN, PREV_INSN and JUMP_LABEL on deleted insns

["Laurynas Biveinis" <laurynas dot biveinis at gmail dot com>]

Hello,

This cleanup patch changes remove_insn to clear NEXT_INSN, PREV_INSN,
and if applicable, JUMP_LABEL fields on the deleted insn. This exposed
several places where these insn fields were in fact accessed after the
insn was deleted and this patch fixes them all, in a sense completing
transition to FOR_BB_INSNS_SAFE.

The motivation for this change is general cleanliness and also the
fact, that some GC implementations (e.g. Boehm's GC) put inaccessible
objects on the free lists without cleaning them and later tries to
mark them. Also, it is a little step towards being able to do GC
collection at arbitrary points.

It is quite possible that my testing did not detect all the places
where the fields are accessed after deletion and thus breaks
something. Luckily, any such places should be easy to fix.

The patch seems larger than it is because of necessary indentation
changes. Bootstrapped/regtested on i686-pc-linux-gnu. OK for mainline?

:ADDPATCH middle-end:

2007-09-05  Laurynas Biveinis  <laurynas.biveinis@gmail.com>

        * regrename.c (copyprop_hardreg_forward_1): New variable next.
        Use FOR_BB_INSNS_SAFE instead of for loop.
        * cse.c (cse_extended_basic_block): Likewise.
        * postreload.c (reload_cse_regs_1): New variable next. Make sure
        that the for loop does not invoke NEXT_INSN on a deleted insn.
        * function.c (instantiate_virtual_regs): Likewise.
        * lower-subreg.c (remove_retval_note): Likewise.
        (decompose_multiword_subregs): Use FOR_BB_INSNS_SAFE instead of
        FOR_BB_INSNS.
        * emit-rtl.c (remove_insn): Set NEXT_INSN and PREV_INSN to NULL
        on a deleted insn.
        * cfgrtl.c (delete_insn): Set JUMP_LABEL to NULL on a deleted
        insn, if it's a jump.
        (try_redirect_by_replacing_jump): New variable jump_p. Call
        tablejump_p before delete_insn_chain.
        * reload1.c (reload): New variable next. Make sure that the for
        loop does not invoke NEXT_INSN on a deleted insn.
        (fixup_eh_region_note): Make the loop terminate if i becomes
        NULL.
        (delete_output_reload): New variable prev. Make sure the the for
        loops do not invoke PREV_INSN on a deleted insn.


Index: gcc/regrename.c
===================================================================
--- gcc/regrename.c     (revision 128065)
+++ gcc/regrename.c     (working copy)
@@ -1576,9 +1576,9 @@
 copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd)
 {
   bool changed = false;
-  rtx insn;
+  rtx insn, next;

-  for (insn = BB_HEAD (bb); ; insn = NEXT_INSN (insn))
+  FOR_BB_INSNS_SAFE (bb, insn, next)
     {
       int n_ops, i, alt, predicated;
       bool is_asm, any_replacements;
Index: gcc/postreload.c
===================================================================
--- gcc/postreload.c    (revision 128065)
+++ gcc/postreload.c    (working copy)
@@ -195,14 +195,15 @@
 static void
 reload_cse_regs_1 (rtx first)
 {
-  rtx insn;
+  rtx insn, next;
   rtx testreg = gen_rtx_REG (VOIDmode, -1);

   cselib_init (true);
   init_alias_analysis ();

-  for (insn = first; insn; insn = NEXT_INSN (insn))
+  for (insn = first; insn; insn = next)
     {
+      next = NEXT_INSN (insn);
       if (INSN_P (insn))
        reload_cse_simplify (insn, testreg);

Index: gcc/cse.c
===================================================================
--- gcc/cse.c   (revision 128065)
+++ gcc/cse.c   (working copy)
@@ -6017,12 +6017,12 @@
   for (path_entry = 0; path_entry < path_size; path_entry++)
     {
       basic_block bb;
-      rtx insn;
+      rtx insn, next;
       rtx libcall_insn = NULL_RTX;
       int no_conflict = 0;

       bb = ebb_data->path[path_entry].bb;
-      FOR_BB_INSNS (bb, insn)
+      FOR_BB_INSNS_SAFE (bb, insn, next)
        {
          /* If we have processed 1,000 insns, flush the hash table to
             avoid extreme quadratic behavior.  We must not include NOTEs
Index: gcc/function.c
===================================================================
--- gcc/function.c      (revision 128065)
+++ gcc/function.c      (working copy)
@@ -1666,7 +1666,7 @@
 static unsigned int
 instantiate_virtual_regs (void)
 {
-  rtx insn;
+  rtx insn, next;

   /* Compute the offsets to use for this function.  */
   in_arg_offset = FIRST_PARM_OFFSET (current_function_decl);
@@ -1684,30 +1684,34 @@

   /* Scan through all the insns, instantiating every virtual register still
      present.  */
-  for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
-    if (INSN_P (insn))
-      {
-       /* These patterns in the instruction stream can never be recognized.
-          Fortunately, they shouldn't contain virtual registers either.  */
-       if (GET_CODE (PATTERN (insn)) == USE
-           || GET_CODE (PATTERN (insn)) == CLOBBER
-           || GET_CODE (PATTERN (insn)) == ADDR_VEC
-           || GET_CODE (PATTERN (insn)) == ADDR_DIFF_VEC
-           || GET_CODE (PATTERN (insn)) == ASM_INPUT)
-         continue;
+  for (insn = get_insns (); insn; insn = next)
+    {
+      next = NEXT_INSN (insn);
+      if (INSN_P (insn))
+        {
+          /* These patterns in the instruction stream can never be recognized.
+             Fortunately, they shouldn't contain virtual registers either.  */
+          if (GET_CODE (PATTERN (insn)) == USE
+              || GET_CODE (PATTERN (insn)) == CLOBBER
+              || GET_CODE (PATTERN (insn)) == ADDR_VEC
+              || GET_CODE (PATTERN (insn)) == ADDR_DIFF_VEC
+              || GET_CODE (PATTERN (insn)) == ASM_INPUT)
+            continue;

-       instantiate_virtual_regs_in_insn (insn);
+          instantiate_virtual_regs_in_insn (insn);

-       if (INSN_DELETED_P (insn))
-         continue;
+          if (INSN_DELETED_P (insn))
+            continue;

-       for_each_rtx (&REG_NOTES (insn), instantiate_virtual_regs_in_rtx, NULL);
+          for_each_rtx (&REG_NOTES (insn), instantiate_virtual_regs_in_rtx,
+                        NULL);

-       /* Instantiate any virtual registers in CALL_INSN_FUNCTION_USAGE.  */
-       if (GET_CODE (insn) == CALL_INSN)
-         for_each_rtx (&CALL_INSN_FUNCTION_USAGE (insn),
-                       instantiate_virtual_regs_in_rtx, NULL);
-      }
+          /* Instantiate any virtual registers in CALL_INSN_FUNCTION_USAGE.  */
+          if (GET_CODE (insn) == CALL_INSN)
+            for_each_rtx (&CALL_INSN_FUNCTION_USAGE (insn),
+                          instantiate_virtual_regs_in_rtx, NULL);
+        }
+    }

   /* Instantiate the virtual registers in the DECLs for debugging purposes.  */
   instantiate_decls (current_function_decl);
Index: gcc/lower-subreg.c
===================================================================
--- gcc/lower-subreg.c  (revision 128065)
+++ gcc/lower-subreg.c  (working copy)
@@ -586,7 +586,7 @@
 static void
 remove_retval_note (rtx insn1)
 {
-  rtx note0, insn0, note1, insn;
+  rtx note0, insn0, note1, insn, next;

   note1 = find_reg_note (insn1, REG_RETVAL, NULL);
   if (note1 == NULL_RTX)
@@ -598,8 +598,9 @@
   remove_note (insn0, note0);
   remove_note (insn1, note1);

-  for (insn = insn0; insn != insn1; insn = NEXT_INSN (insn))
+  for (insn = insn0; (insn != insn1) && insn; insn = next)
     {
+      next = NEXT_INSN (insn);
       while (1)
        {
          rtx note;
@@ -1252,9 +1253,9 @@

       FOR_EACH_BB (bb)
        {
-         rtx insn;
+          rtx insn, next;

-         FOR_BB_INSNS (bb, insn)
+          FOR_BB_INSNS_SAFE (bb, insn, next)
            {
              rtx next, pat;

Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c      (revision 128065)
+++ gcc/emit-rtl.c      (working copy)
@@ -3688,6 +3688,8 @@
       if (BB_END (bb) == insn)
        BB_END (bb) = prev;
     }
+  NEXT_INSN (insn) = NULL;
+  PREV_INSN (insn) = NULL;
 }

 /* Append CALL_FUSAGE to the CALL_INSN_FUNCTION_USAGE for CALL_INSN.  */
Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c        (revision 128065)
+++ gcc/cfgrtl.c        (working copy)
@@ -141,7 +141,10 @@
   if (JUMP_P (insn)
       && JUMP_LABEL (insn)
       && LABEL_P (JUMP_LABEL (insn)))
-    LABEL_NUSES (JUMP_LABEL (insn))--;
+    {
+      LABEL_NUSES (JUMP_LABEL (insn))--;
+      JUMP_LABEL (insn) = NULL;
+    }

   /* Also if deleting an insn that references a label.  */
   else
@@ -790,6 +793,7 @@
     {
       rtx target_label = block_label (target);
       rtx barrier, label, table;
+      bool jump_p;

       emit_jump_insn_after_noloc (gen_jump (target_label), insn);
       JUMP_LABEL (BB_END (src)) = target_label;
@@ -799,14 +803,15 @@
                 INSN_UID (insn), INSN_UID (BB_END (src)));


-      delete_insn_chain (kill_from, insn, false);
-
       /* Recognize a tablejump that we are converting to a
         simple jump and remove its associated CODE_LABEL
         and ADDR_VEC or ADDR_DIFF_VEC.  */
-      if (tablejump_p (insn, &label, &table))
-       delete_insn_chain (label, table, false);
+      jump_p = tablejump_p (insn, &label, &table);

+      delete_insn_chain (kill_from, insn, false);
+      if (jump_p)
+        delete_insn_chain (label, table, false);
+
       barrier = next_nonnote_insn (BB_END (src));
       if (!barrier || !BARRIER_P (barrier))
        emit_barrier_after (BB_END (src));
Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c       (revision 128065)
+++ gcc/reload1.c       (working copy)
@@ -702,7 +702,7 @@
 reload (rtx first, int global)
 {
   int i;
-  rtx insn;
+  rtx insn, next;
   struct elim_table *ep;
   basic_block bb;

@@ -1225,87 +1225,91 @@
      are no longer useful or accurate.  Strip and regenerate REG_INC notes
      that may have been moved around.  */

-  for (insn = first; insn; insn = NEXT_INSN (insn))
-    if (INSN_P (insn))
-      {
-       rtx *pnote;
+  for (insn = first; insn; insn = next)
+    {
+      next = NEXT_INSN (insn);
+      if (INSN_P (insn))
+        {
+          rtx *pnote;

-       if (CALL_P (insn))
-         replace_pseudos_in (& CALL_INSN_FUNCTION_USAGE (insn),
-                             VOIDmode, CALL_INSN_FUNCTION_USAGE (insn));
+          if (CALL_P (insn))
+            replace_pseudos_in (& CALL_INSN_FUNCTION_USAGE (insn),
+                                VOIDmode, CALL_INSN_FUNCTION_USAGE (insn));

-       if ((GET_CODE (PATTERN (insn)) == USE
-            /* We mark with QImode USEs introduced by reload itself.  */
-            && (GET_MODE (insn) == QImode
-                || find_reg_note (insn, REG_EQUAL, NULL_RTX)))
-           || (GET_CODE (PATTERN (insn)) == CLOBBER
-               && (!MEM_P (XEXP (PATTERN (insn), 0))
-                   || GET_MODE (XEXP (PATTERN (insn), 0)) != BLKmode
-                   || (GET_CODE (XEXP (XEXP (PATTERN (insn), 0), 0)) != SCRATCH
-                       && XEXP (XEXP (PATTERN (insn), 0), 0)
-                               != stack_pointer_rtx))
-               && (!REG_P (XEXP (PATTERN (insn), 0))
-                   || ! REG_FUNCTION_VALUE_P (XEXP (PATTERN (insn), 0)))))
-         {
-           delete_insn (insn);
-           continue;
-         }
+          if ((GET_CODE (PATTERN (insn)) == USE
+               /* We mark with QImode USEs introduced by reload itself.  */
+               && (GET_MODE (insn) == QImode
+                   || find_reg_note (insn, REG_EQUAL, NULL_RTX)))
+              || (GET_CODE (PATTERN (insn)) == CLOBBER
+                  && (!MEM_P (XEXP (PATTERN (insn), 0))
+                      || GET_MODE (XEXP (PATTERN (insn), 0)) != BLKmode
+                      || (GET_CODE (XEXP (XEXP (PATTERN (insn), 0), 0))
+                          != SCRATCH
+                          && XEXP (XEXP (PATTERN (insn), 0), 0)
+                          != stack_pointer_rtx))
+                  && (!REG_P (XEXP (PATTERN (insn), 0))
+                      || ! REG_FUNCTION_VALUE_P (XEXP (PATTERN (insn), 0)))))
+            {
+              delete_insn (insn);
+              continue;
+            }

-       /* Some CLOBBERs may survive until here and still reference unassigned
-          pseudos with const equivalent, which may in turn cause ICE in later
-          passes if the reference remains in place.  */
-       if (GET_CODE (PATTERN (insn)) == CLOBBER)
-         replace_pseudos_in (& XEXP (PATTERN (insn), 0),
-                             VOIDmode, PATTERN (insn));
+          /* Some CLOBBERs may survive until here and still reference
+             unassigned pseudos with const equivalent, which may in turn cause
+             ICE in later passes if the reference remains in place.  */
+          if (GET_CODE (PATTERN (insn)) == CLOBBER)
+            replace_pseudos_in (& XEXP (PATTERN (insn), 0),
+                                VOIDmode, PATTERN (insn));

-       /* Discard obvious no-ops, even without -O.  This optimization
-          is fast and doesn't interfere with debugging.  */
-       if (NONJUMP_INSN_P (insn)
-           && GET_CODE (PATTERN (insn)) == SET
-           && REG_P (SET_SRC (PATTERN (insn)))
-           && REG_P (SET_DEST (PATTERN (insn)))
-           && (REGNO (SET_SRC (PATTERN (insn)))
-               == REGNO (SET_DEST (PATTERN (insn)))))
-         {
-           delete_insn (insn);
-           continue;
-         }
+          /* Discard obvious no-ops, even without -O.  This optimization
+             is fast and doesn't interfere with debugging.  */
+          if (NONJUMP_INSN_P (insn)
+              && GET_CODE (PATTERN (insn)) == SET
+              && REG_P (SET_SRC (PATTERN (insn)))
+              && REG_P (SET_DEST (PATTERN (insn)))
+              && (REGNO (SET_SRC (PATTERN (insn)))
+                  == REGNO (SET_DEST (PATTERN (insn)))))
+            {
+              delete_insn (insn);
+              continue;
+            }

-       pnote = &REG_NOTES (insn);
-       while (*pnote != 0)
-         {
-           if (REG_NOTE_KIND (*pnote) == REG_DEAD
-               || REG_NOTE_KIND (*pnote) == REG_UNUSED
-               || REG_NOTE_KIND (*pnote) == REG_INC
-               || REG_NOTE_KIND (*pnote) == REG_RETVAL
-               || REG_NOTE_KIND (*pnote) == REG_LIBCALL_ID
-               || REG_NOTE_KIND (*pnote) == REG_LIBCALL)
-             *pnote = XEXP (*pnote, 1);
-           else
-             pnote = &XEXP (*pnote, 1);
-         }
+          pnote = &REG_NOTES (insn);
+          while (*pnote != 0)
+            {
+              if (REG_NOTE_KIND (*pnote) == REG_DEAD
+                  || REG_NOTE_KIND (*pnote) == REG_UNUSED
+                  || REG_NOTE_KIND (*pnote) == REG_INC
+                  || REG_NOTE_KIND (*pnote) == REG_RETVAL
+                  || REG_NOTE_KIND (*pnote) == REG_LIBCALL_ID
+                  || REG_NOTE_KIND (*pnote) == REG_LIBCALL)
+                *pnote = XEXP (*pnote, 1);
+              else
+                pnote = &XEXP (*pnote, 1);
+            }

 #ifdef AUTO_INC_DEC
-       add_auto_inc_notes (insn, PATTERN (insn));
+          add_auto_inc_notes (insn, PATTERN (insn));
 #endif

-       /* Simplify (subreg (reg)) if it appears as an operand.  */
-       cleanup_subreg_operands (insn);
+          /* Simplify (subreg (reg)) if it appears as an operand.  */
+          cleanup_subreg_operands (insn);

-       /* Clean up invalid ASMs so that they don't confuse later passes.
-          See PR 21299.  */
-       if (asm_noperands (PATTERN (insn)) >= 0)
-         {
-           extract_insn (insn);
-           if (!constrain_operands (1))
-             {
-               error_for_asm (insn,
-                              "%<asm%> operand has impossible constraints");
-               delete_insn (insn);
-               continue;
-             }
-         }
-      }
+          /* Clean up invalid ASMs so that they don't confuse later passes.
+             See PR 21299.  */
+          if (asm_noperands (PATTERN (insn)) >= 0)
+            {
+              extract_insn (insn);
+              if (!constrain_operands (1))
+               {
+                  error_for_asm (insn,
+                                 "%<asm%> operand has impossible constraints");
+                  delete_insn (insn);
+                  continue;
+                }
+            }
+        }
+    }

   /* If we are doing stack checking, give a warning if this function's
      frame size is larger than we expect.  */
@@ -4014,7 +4018,7 @@
       trap_count = 0;
     }

-  for (i = NEXT_INSN (prev); i != next; i = NEXT_INSN (i))
+  for (i = NEXT_INSN (prev); i && (i != next); i = NEXT_INSN (i))
     if (INSN_P (i) && i != insn && may_trap_p (PATTERN (i)))
       {
        trap_count++;
@@ -8186,16 +8190,19 @@
       && REG_BASIC_BLOCK (REGNO (reg)) >= NUM_FIXED_BLOCKS
       && find_regno_note (insn, REG_DEAD, REGNO (reg)))
     {
-      rtx i2;
+      rtx i2, prev;

       /* We know that it was used only between here and the beginning of
         the current basic block.  (We also know that the last use before
         INSN was the output reload we are thinking of deleting, but never
         mind that.)  Search that range; see if any ref remains.  */
-      for (i2 = PREV_INSN (insn); i2; i2 = PREV_INSN (i2))
+      for (i2 = PREV_INSN (insn); i2; i2 = prev)
        {
-         rtx set = single_set (i2);
+          rtx set;

+          prev = PREV_INSN (i2);
+          set = single_set (i2);
+
          /* Uses which just store in the pseudo don't count,
             since if they are the only uses, they are dead.  */
          if (set != 0 && SET_DEST (set) == reg)
@@ -8216,9 +8223,11 @@

       /* Delete the now-dead stores into this pseudo.  Note that this
         loop also takes care of deleting output_reload_insn.  */
-      for (i2 = PREV_INSN (insn); i2; i2 = PREV_INSN (i2))
+      for (i2 = PREV_INSN (insn); i2; i2 = prev)
        {
-         rtx set = single_set (i2);
+          rtx set;
+          prev = PREV_INSN (i2);
+          set = single_set (i2);

          if (set != 0 && SET_DEST (set) == reg)
            {


-- 
Laurynas