RFC: patch to fix g++.old-deja/g++.other/loop2.C

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

I tracked down the ICE when compiling loop2.C to loop.c (find_and_verify_loops).
Loop is there deciding whether to move the return; stuff out of the loop.
The interesting insns in the chain there are:
(jump_insn 120 119 124 (set (pc)		p
        (if_then_else (ne (reg:CCZ 17 flags)
                (const_int 0 [0x0]))
            (label_ref 132)
            (pc))) 278 {*jcc_1} (nil)
    (nil))
...
(note 253 329 269 21 NOTE_INSN_EH_REGION_BEG 0)
...
(jump_insn 284 283 347 (set (pc)		insn
        (label_ref/s 325)) -1 (nil)
    (nil))

(barrier 347 284 331)
...
(note 296 331 126 21 NOTE_INSN_EH_REGION_END 0)

It calls insns_safe_to_move_p from p to insn and as eh_region notes don't
pair, it goes up to 296 and returns it as last_insn_to_move.
But then it calls squeeze_notes from code_label (created after 120) to 296.
This with rth's change from may where squeeze_notes squeezes EH_REGION notes
moves the EH_REGION_BEG note out of the region to be moved (and keeps the
EH_REGION_END note beucase squeeze notes explicitely assumes the end is not
such a note).
This results in an ICE later on because the EH_REGION notes are no longer
properly nested.
Even if we corrected that, such that we called squeeze_notes with
NEXT_INSN(last_insn_to_move) if last_insn_to_move != insn, we'd result with
the EH_REGION empty and the region which should be guarded by the exception
handling would end up outside of that region.
Now, I see two solutions and would like to know which way to go:
either (as done in the following patch), in find_and_verify_loops avoid
squeezing EH_REGION notes, or just avoid that optimization and don't move
anything if the region contains EH_REGION notes.
The former might have issues if it is illegal to move the nested eh region
out of the nesting (in this case there is EHB3 EHB21 EHE21 EHE3 and we'd be
moving EHB21/EHE21 pair at the end of the function (ie. after EHE3)), so
someone who knows EH please tell me whether it is illegal or not.
The latter would in my eyes mean getting rid of insns_safe_to_move_p, or at
least greatly reducing it (it is currently only used in find_and_verify_loops
and its role would change into a plain scan of the block for EH_REGION notes
and return 0 if one is found). This would mean also that the new_to
argument would be useless.

--- gcc/jump.c.jj	Fri Aug 18 23:44:08 2000
+++ gcc/jump.c	Tue Aug 22 14:10:11 2000
@@ -1369,11 +1369,13 @@ duplicate_loop_exit_test (loop_start)
    eh-beg, eh-end notes between START and END out before START.  Assume that
    END is not such a note.  START may be such a note.  Returns the value
    of the new starting insn, which may be different if the original start
-   was such a note.  */
+   was such a note.
+   If MOVE_EH is 1, eh-beg and eh-end notes are not moved.  */
 
 rtx
-squeeze_notes (start, end)
+squeeze_notes (start, end, move_eh)
      rtx start, end;
+     int move_eh;
 {
   rtx insn;
   rtx next;
@@ -1388,8 +1390,9 @@ squeeze_notes (start, end)
 	      || NOTE_LINE_NUMBER (insn) == NOTE_INSN_LOOP_END
 	      || NOTE_LINE_NUMBER (insn) == NOTE_INSN_LOOP_CONT
 	      || NOTE_LINE_NUMBER (insn) == NOTE_INSN_LOOP_VTOP
-	      || NOTE_LINE_NUMBER (insn) == NOTE_INSN_EH_REGION_BEG
-	      || NOTE_LINE_NUMBER (insn) == NOTE_INSN_EH_REGION_END))
+	      || (move_eh
+		  && (NOTE_LINE_NUMBER (insn) == NOTE_INSN_EH_REGION_BEG
+		      || NOTE_LINE_NUMBER (insn) == NOTE_INSN_EH_REGION_END))))
 	{
 	  if (insn == start)
 	    start = next;
--- gcc/rtl.h.jj	Fri Aug 18 23:44:08 2000
+++ gcc/rtl.h	Fri Aug 18 23:44:08 2000
@@ -1255,7 +1255,7 @@ extern enum rtx_code signed_condition	PA
 extern rtx find_equiv_reg		PARAMS ((rtx, rtx, enum reg_class, int, short *, int, enum machine_mode));
 
 /* In jump.c */
-extern rtx squeeze_notes		PARAMS ((rtx, rtx));
+extern rtx squeeze_notes		PARAMS ((rtx, rtx, int));
 extern rtx delete_insn			PARAMS ((rtx));
 extern void delete_jump			PARAMS ((rtx));
 extern void delete_barrier		PARAMS ((rtx));
--- gcc/flow.c.jj	Fri Aug 18 23:44:08 2000
+++ gcc/flow.c	Tue Aug 22 14:11:43 2000
@@ -2269,7 +2269,7 @@ merge_blocks_move_predecessor_nojumps (a
      and adjust the block trees appropriately.   Even better would be to have
      a tighter connection between block trees and rtl so that this is not
      necessary.  */
-  start = squeeze_notes (start, end);
+  start = squeeze_notes (start, end, 1);
 
   /* Scramble the insn chain.  */
   if (end != PREV_INSN (b->head))
@@ -2333,7 +2333,7 @@ merge_blocks_move_successor_nojumps (a, 
      and adjust the block trees appropriately.   Even better would be to have
      a tighter connection between block trees and rtl so that this is not
      necessary.  */
-  start = squeeze_notes (start, end);
+  start = squeeze_notes (start, end, 1);
 
   /* Scramble the insn chain.  */
   reorder_insns (start, end, a->end);
--- gcc/ifcvt.c.jj	Fri Jun 16 13:10:29 2000
+++ gcc/ifcvt.c	Tue Aug 22 14:11:59 2000
@@ -2040,7 +2040,7 @@ dead_or_predicable (test_bb, merge_bb, o
       if (end == merge_bb->end)
 	merge_bb->end = PREV_INSN (head);
 
-      head = squeeze_notes (head, end);
+      head = squeeze_notes (head, end, 1);
       if (GET_CODE (end) == NOTE
 	  && (NOTE_LINE_NUMBER (end) == NOTE_INSN_BLOCK_END
               || NOTE_LINE_NUMBER (end) == NOTE_INSN_BLOCK_BEG
--- gcc/stmt.c.jj	Fri Aug 18 23:44:10 2000
+++ gcc/stmt.c	Tue Aug 22 14:12:39 2000
@@ -5556,7 +5556,7 @@ expand_end_case (orig_index)
 #endif
 	}
 
-      before_case = squeeze_notes (NEXT_INSN (before_case), get_last_insn ());
+      before_case = squeeze_notes (NEXT_INSN (before_case), get_last_insn (), 1);
       reorder_insns (before_case, get_last_insn (),
 		     thiscase->data.case_stmt.start);
     }
--- gcc/loop.c.jj	Fri Aug 18 23:44:10 2000
+++ gcc/loop.c	Tue Aug 22 14:13:02 2000
@@ -2896,7 +2896,7 @@ find_and_verify_loops (f, loops)
 			/* Include the BARRIER after INSN and copy the
 			   block after LOC.  */
 			new_label = squeeze_notes (new_label, 
-						   last_insn_to_move);
+						   last_insn_to_move, 0);
 			reorder_insns (new_label, last_insn_to_move, loc);
 
 			/* All those insns are now in TARGET_LOOP.  */

	Jakub