This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Exception causing insns in delay slots



[ Java folks, ignore the majority of this email, and please skip
  down to "Longer term:", item 2.  It is following up to the posting
  I made earlier this evening, specifically:

     http://gcc.gnu.org/ml/java/2002-04/msg00357.html

  that posting. ]

This patch fixes half of PR target/6422, the other change needed to
close that bug is in libjava and will go to java-patches shortly.

Currently, reorg will happily put exception causing instructions
into delay slots.

This causes major problems when convert_to_eh_region_ranges
runs and hits such a delay instruction.

The problematic case we end up with is a SEQUENCE containing a branch
and a delay slot exception causing instruction, in that order.

In convert_to_eh_region_ranges we try to emit:

1) NOTE_INSN_EH_REGION_BEG before the second insn in the
   SEQUENCE.

2) NOTE_INSN_EH_REGION_END after the second insn in the
   SEQUENCE.

#2 goes ok, the note is put after the SEQUENCE itself.

However, #1 is completely mishandled, this label is placed after the
SEQUENCE too.  The culprit is the SEQUENCE handling code in
emit-etl.c:add_insn_before().

The SEQUENCE handling of add_insn_before() has many problems, in fact
if you try to emit something before the first insn of a SEQUENCE this
code will create a loop in the insn chain!

However, after some discussion with Richard, it is just as wrong to
put exception causing instructions into delay slots by default.  I
also believe that this is the safest way to fix this bug at this time.

Thus, the fix implemented is to disallow reorg.c to consider exception
causing instructions for delay slots.

Longer term:

1) add_insn_before's SEQUENCE handling needs fixing or it needs to
   signal an assertion failure when something is attempted inside of
   a SEQUENCE.  The current behavior is just wrong. :-)

2) We might try to let reorg.c put exception causing instructions
   in delay slots for targets that can handle it properly.

   After some analysis of how unwinding and throw work, I think it
   may not even be supportable on Sparc.  The reason has to do with
   a combination of two things:

   a) How unwind-dw2.c looks for FDEs at "context->ra - 1" instead of
      just "context->ra"

   b) How on delay slot architectures "context->ra - 1" is not
      necessarily the previous instruction executed.

   Basically, for signal frame dwarf2 unwind to work in the current
   framework the signal handler advances the program counter past
   the exception causing instruction and that value minus one has to
   be within the exception causing instruction.

   I made a posting to java@gcc.gnu.org earlier this evening before
   realizing this point.  Sorry Java people for jumping the gun
   and possibly scaring everyone :-)  However my other point about
   how unwind-dwarf2 adjusts the program counter still stands.

   It appears Boehm has caught the errors in my initial analysis
   already :-)

Mark, ok for 3.1 branch?

2002-04-25  David S. Miller  <davem@redhat.com>

	PR target/6422
	* reorg.c (optimize_skip): Do not allow exception causing
	instructions to be considered for delay slots.
	(fill_simply_delay_slots, fill_slots_from_thread): Likewise.
	(relax_delay_slots): Do not try to consider exception causing
	instructions as redundant.

--- reorg.c.~1~	Thu Apr 25 21:05:14 2002
+++ reorg.c	Thu Apr 25 21:16:15 2002
@@ -750,7 +750,8 @@ optimize_skip (insn)
       || GET_CODE (PATTERN (trial)) == SEQUENCE
       || recog_memoized (trial) < 0
       || (! eligible_for_annul_false (insn, 0, trial, flags)
-	  && ! eligible_for_annul_true (insn, 0, trial, flags)))
+	  && ! eligible_for_annul_true (insn, 0, trial, flags))
+      || can_throw_internal (trial))
     return 0;
 
   /* There are two cases where we are just executing one insn (we assume
@@ -2127,7 +2128,8 @@ fill_simple_delay_slots (non_jumps_p)
 	  && GET_CODE (trial) == JUMP_INSN
 	  && simplejump_p (trial)
 	  && eligible_for_delay (insn, slots_filled, trial, flags)
-	  && no_labels_between_p (insn, trial))
+	  && no_labels_between_p (insn, trial)
+	  && ! can_throw_internal (trial))
 	{
 	  rtx *tmp;
 	  slots_filled++;
@@ -2197,7 +2199,7 @@ fill_simple_delay_slots (non_jumps_p)
 		  /* Can't separate set of cc0 from its use.  */
 		  && ! (reg_mentioned_p (cc0_rtx, pat) && ! sets_cc0_p (pat))
 #endif
-		  )
+		  && ! can_throw_internal (trial))
 		{
 		  trial = try_split (pat, trial, 1);
 		  next_trial = prev_nonnote_insn (trial);
@@ -2273,7 +2275,7 @@ fill_simple_delay_slots (non_jumps_p)
 
 	     Presumably, we should also check to see if we could get
 	     back to this function via `setjmp'.  */
-	  && !can_throw_internal (insn)
+	  && ! can_throw_internal (insn)
 	  && (GET_CODE (insn) != JUMP_INSN
 	      || ((condjump_p (insn) || condjump_in_parallel_p (insn))
 		  && ! simplejump_p (insn)
@@ -2340,7 +2342,8 @@ fill_simple_delay_slots (non_jumps_p)
 #endif
 		    && ! (maybe_never && may_trap_p (pat))
 		    && (trial = try_split (pat, trial, 0))
-		    && eligible_for_delay (insn, slots_filled, trial, flags))
+		    && eligible_for_delay (insn, slots_filled, trial, flags)
+		    && ! can_throw_internal(trial))
 		  {
 		    next_trial = next_nonnote_insn (trial);
 		    delay_list = add_to_delay_list (trial, delay_list);
@@ -2392,7 +2395,8 @@ fill_simple_delay_slots (non_jumps_p)
 #endif
 	      && ! (maybe_never && may_trap_p (PATTERN (next_trial)))
 	      && (next_trial = try_split (PATTERN (next_trial), next_trial, 0))
-	      && eligible_for_delay (insn, slots_filled, next_trial, flags))
+	      && eligible_for_delay (insn, slots_filled, next_trial, flags)
+	      && ! can_throw_internal (trial))
 	    {
 	      rtx new_label = next_active_insn (next_trial);
 
@@ -2496,7 +2500,7 @@ fill_simple_delay_slots (non_jumps_p)
 	  /* Don't want to mess with cc0 here.  */
 	  && ! reg_mentioned_p (cc0_rtx, pat)
 #endif
-	  )
+	  && ! can_throw_internal (trial))
 	{
 	  trial = try_split (pat, trial, 1);
 	  if (ELIGIBLE_FOR_EPILOGUE_DELAY (trial, slots_filled))
@@ -2637,7 +2641,7 @@ fill_slots_from_thread (insn, condition,
 	  && ! (reg_mentioned_p (cc0_rtx, pat)
 		&& (! own_thread || ! sets_cc0_p (pat)))
 #endif
-	  )
+	  && ! can_throw_internal (trial))
 	{
 	  rtx prior_insn;
 
@@ -2874,8 +2878,10 @@ fill_slots_from_thread (insn, condition,
       trial = new_thread;
       pat = PATTERN (trial);
 
-      if (GET_CODE (trial) != INSN || GET_CODE (pat) != SET
-	  || ! eligible_for_delay (insn, 0, trial, flags))
+      if (GET_CODE (trial) != INSN
+	  || GET_CODE (pat) != SET
+	  || ! eligible_for_delay (insn, 0, trial, flags)
+	  || can_throw_internal (trial))
 	return 0;
 
       dest = SET_DEST (pat), src = SET_SRC (pat);
@@ -3286,7 +3292,8 @@ relax_delay_slots (first)
 	     insn, redirect the jump to the following insn process again.  */
 	  trial = next_active_insn (target_label);
 	  if (trial && GET_CODE (PATTERN (trial)) != SEQUENCE
-	      && redundant_insn (trial, insn, 0))
+	      && redundant_insn (trial, insn, 0)
+	      && ! can_throw_internal (trial))
 	    {
 	      rtx tmp;
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]