This a build regression.  This is eh related.

SH is not a primary or secondary platform.

Here is a workaround.  Although it doesn't solve the issue
completely, it'd be better than nothing.  It prevents to
insert the mode switching code after the last insn of BB
when that insn has the REG_EH_REGION note.  It also checks
if the last insn needs a mode which doesn't match with
the mode given by the mode switching code in such case.

diff -uprN ORIG/trunk/gcc/mode-switching.c LOCAL/trunk/gcc/mode-switching.c
--- ORIG/trunk/gcc/mode-switching.c	2006-03-09 15:11:37.000000000 +0900
+++ LOCAL/trunk/gcc/mode-switching.c	2006-08-22 09:55:23.000000000 +0900
@@ -612,9 +612,11 @@ optimize_mode_switching (void)
 		 of the previous block.  */
 	      if (eg->flags & EDGE_ABNORMAL)
 		{
+		  rtx last = BB_END (src_bb);
+
 		  emited = true;
-		  if (JUMP_P (BB_END (src_bb)))
-		    emit_insn_before (mode_set, BB_END (src_bb));
+		  if (JUMP_P (last))
+		    emit_insn_before (mode_set, last);
 		  else
 		    {
 		      /* It doesn't make sense to switch to normal
@@ -629,8 +631,19 @@ optimize_mode_switching (void)
 		         sense, anyway).  In the case of EH edges, EH
 		         entry points also start in normal mode, so a
 		         similar reasoning applies.  */
-		      gcc_assert (NONJUMP_INSN_P (BB_END (src_bb)));
-		      emit_insn_after (mode_set, BB_END (src_bb));
+		      gcc_assert (NONJUMP_INSN_P (last));
+		      /* We cannot insert instructions after the insn
+			 having a REG_EH_REGION note.  */
+		      if (find_reg_note (last, REG_EH_REGION, NULL_RTX))
+			{
+			  gcc_assert ((MODE_NEEDED (entity_map[j], last)
+				       == no_mode)
+				      || (MODE_NEEDED (entity_map[j], last)
+					  == mode));
+			  emit_insn_before (mode_set, last);
+			}
+		      else
+			emit_insn_after (mode_set, last);
 		    }
 		  bb_info[j][src_bb->index].computing = mode;
 		  RESET_BIT (transp[src_bb->index], j);

As far as I can see, the only reason why we can have a REG_EH_REGION note
on a non-call instruction is when the instruction may trap, and we compile
with -fnon-call-exceptions.

(In reply to comment #3)
> Here is a workaround.  Although it doesn't solve the issue
> completely, it'd be better than nothing.  It prevents to
> insert the mode switching code after the last insn of BB
> when that insn has the REG_EH_REGION note.

I think you can get an incorect mode this way, when the
fall-thorough path is supposed to have a different mode than
the abnormal edge destination.  In fact, if the modes were the same,
and not in conflict with the last insn of the source BB, LCM
would be expected to place the mode switch into the source block
or even earlier in the flow graph.
I think the only sane way to handle this is not to emit any mode
switching code for exception edges at the potential trapping site,
and make the exception handling code assume no particular mode is
present (unless the compiler can prove that an exception handler can only
be reached from throwing/trapping sites that all have the same mode).


> It also checks
> if the last insn needs a mode which doesn't match with
> the mode given by the mode switching code in such case.

If HONOR_SNANS of flag_trapping_math is active together with
-fnon-call-exceptions, such problems can be expected for
floating point operations.

> I think the only sane way to handle this is not to emit any mode
> switching code for exception edges at the potential trapping site,
> and make the exception handling code assume no particular mode is
> present (unless the compiler can prove that an exception handler can only
> be reached from throwing/trapping sites that all have the same mode).

Although the mode switching code beyonds me, how does the exception
handling code have no particular mode at its start?
By calling make_preds_opaque for the corresponding BB and set
no_mode to seginfo->mode?

(In reply to comment #5)
> > I think the only sane way to handle this is not to emit any mode
> > switching code for exception edges at the potential trapping site,
> > and make the exception handling code assume no particular mode is
> > present (unless the compiler can prove that an exception handler can only
> > be reached from throwing/trapping sites that all have the same mode).
> 
> Although the mode switching code beyonds me,

I don't think the MODE_AFTER hack works - it fails to affect transparency
and anticipatability.

> how does the exception
> handling code have no particular mode at its start?

Actually, this code seems to attempt to do that already:

          /* Pretend the mode is clobbered across abnormal edges.  */
          {
            edge_iterator ei;
            edge e;
            FOR_EACH_EDGE (e, ei, bb->preds)
              if (e->flags & EDGE_COMPLEX)
                break;
            if (e)
              RESET_BIT (transp[bb->index], j);
          }

However, this is not safe in case BB has a locally anticipatable mode.
In addition to the REST_BIT invocation, it needs:

                      ptr = new_seginfo (no_mode, insn, bb->index, live_now);
                      add_seginfo (info + bb->index, ptr);

so that we won't show an anticipatable mode.

> By calling make_preds_opaque for the corresponding BB and set
> no_mode to seginfo->mode?

No, although calling make_preds_opaque on the successor block should work,
that would be a gross pessimization.
make_preds_opaque is called when we effetively move a mode set upwards
in the flowgraph to one or more 'earliest' locations, and thus the mode
information in the mode-switchable entity becomes live on all the paths
upwards from the use of the mode to the sets of the required mode.  Thus,
we know that we may not switch the mode-switchable entity to any other
mode on these paths.

(In reply to comment #0)

Actually, I think this code:

              /* If this is an abnormal edge, we'll insert at the end
                 of the previous block.  */
              if (eg->flags & EDGE_ABNORMAL)
...

is just nonsense.  It's not safe to insert at the end of the previous block,
since it may be more than one successor.  In fact, this is expected
both for exception handling (where there is a fall-through path) as
well as computed jumps (if there is only one successor, why compute it?)

And when the other code to handle abnormal edges is fixed, we should not
even try to insert a mode set on an abnormal edge, so the above code can
be changed into
              gcc_assert (! (eg_flags & EDGE_ABNORMAL))

On possible optimization is to identify when there is no fall-through
predecessor for the edge destination, and a means exists to redirect the abnormal edge (that should be possible in a simple exception handling
scenario); in that case, the abnormal edge can be redirected to a newly
created fall-through predecssor of the original successor block.

> And when the other code to handle abnormal edges is fixed, we should not
> even try to insert a mode set on an abnormal edge, so the above code can
> be changed into
>               gcc_assert (! (eg_flags & EDGE_ABNORMAL))

Do you mean the patch like this?

--- ORIG/trunk/gcc/mode-switching.c	2006-03-09 15:11:37.000000000 +0900
+++ LOCAL/trunk/gcc/mode-switching.c	2006-08-25 09:17:13.000000000 +0900
@@ -465,7 +465,11 @@ optimize_mode_switching (void)
 	      if (e->flags & EDGE_COMPLEX)
 		break;
 	    if (e)
-	      RESET_BIT (transp[bb->index], j);
+	      {
+		ptr = new_seginfo (no_mode, BB_HEAD (bb), bb->index, live_now);
+		add_seginfo (info + bb->index, ptr);
+		RESET_BIT (transp[bb->index], j);
+	      }
 	  }
 
 	  for (insn = BB_HEAD (bb);
@@ -608,38 +612,11 @@ optimize_mode_switching (void)
 	      if (mode_set == NULL_RTX)
 		continue;
 
-	      /* If this is an abnormal edge, we'll insert at the end
-		 of the previous block.  */
-	      if (eg->flags & EDGE_ABNORMAL)
-		{
-		  emited = true;
-		  if (JUMP_P (BB_END (src_bb)))
-		    emit_insn_before (mode_set, BB_END (src_bb));
-		  else
-		    {
-		      /* It doesn't make sense to switch to normal
-		         mode after a CALL_INSN.  The cases in which a
-		         CALL_INSN may have an abnormal edge are
-		         sibcalls and EH edges.  In the case of
-		         sibcalls, the dest basic-block is the
-		         EXIT_BLOCK, that runs in normal mode; it is
-		         assumed that a sibcall insn requires normal
-		         mode itself, so no mode switch would be
-		         required after the call (it wouldn't make
-		         sense, anyway).  In the case of EH edges, EH
-		         entry points also start in normal mode, so a
-		         similar reasoning applies.  */
-		      gcc_assert (NONJUMP_INSN_P (BB_END (src_bb)));
-		      emit_insn_after (mode_set, BB_END (src_bb));
-		    }
-		  bb_info[j][src_bb->index].computing = mode;
-		  RESET_BIT (transp[src_bb->index], j);
-		}
-	      else
-		{
-		  need_commit = 1;
-		  insert_insn_on_edge (mode_set, eg);
-		}
+	      /* We should not get an abnormal edge here.  */
+	      gcc_assert (! (eg->flags & EDGE_ABNORMAL));
+
+	      need_commit = 1;
+	      insert_insn_on_edge (mode_set, eg);
 	    }
 
 	  FOR_EACH_BB_REVERSE (bb)

(In reply to comment #8)
> Do you mean the patch like this?
Yes, indeed.

I've confirmed that it fixes the build failure on sh4-linux.  I'll
send it to gcc-patches after the test on i686-linux is completed.

This potentially affects i686-pc-linux-gnu as it also uses the mode switching code.  I think this should go in.

(In reply to comment #11)
> This potentially affects i686-pc-linux-gnu as it also uses the mode switching
> code.  I think this should go in.

As far as I can see, the i387 mode switching is already completely broken,
because it treats the different modes of a single mode-switchable entity
as separate entities.  However, unless
"TARGET_USE_FANCY_MATH_387
 && flag_unsafe_math_optimizations"
is true, the only mode used is I387_CW_TRUNC, which allows all the LCM problems
to be trivially solved by either inserting no mode switch (if no user
is present), or inserting a single mode switch in a common dominator.

(In reply to comment #12)
> As far as I can see, the i387 mode switching is already completely broken,
> because it treats the different modes of a single mode-switchable entity
> as separate entities.

On second thought, this can work with the current implementation of mode-switching using pre_edge_lcm, since it will never insert a computation
in a path if it hasn't been there before.
If there is a block with successors that have different mode requirements,
hoisting any computation to its predecessor edges or above would cause
a dead computation on at least one path through the sucessor edge which needs
the other mode.
So the calls to make_preds_opaque are not actually accomplishing anything,
and we could compute all modes with a single pre_edge_lcm call.

On the other hand, a more sophisticated code motion implementation would
recognize when inserting a computation on an infrequent path can avoid
doing it on a more frequent path.  And then, the incorrect use of the
mode switching interface in the i386 port could become a manifest bug.

For any transparent sub-graph, considering the expression anticipatable
in this graph is beneficial if the sum of the frequencies of outgoing edges
on which the expression is anticipatable is larger than the sum of the
frequencies of incoming edges on which the expression is not available.
So the main problem for a code motion pass that takes edge frequencies into account appears to be to find heuristics how to identify transparent subgraphs
which should be checked - a naiive algorithm to try all subsets of nodes would
be exponential. On the other hand, checking only subgraphs which are interconnected as a list and have no connection to other transparent nodes except in head and tail, and could be expanded at their tail (alternatively: at their head) only in the form of a longer list, can be done with a greedy
algorithm which checks only O(number of nodes) nodes. (Traverse all nodes to
find nodes which are tail / head of maximum lists, drop nodes at the bottom if it increases the tally, for a positive tally, if adding a new node would
decrease it, remember current list.  When current tally becomes negative or the list can't be extended, check for remembered list.) 

> On second thought, this can work with the current implementation of
> mode-switching using pre_edge_lcm, since it will never insert a computation
> in a path if it hasn't been there before.

Yes, and that's why I think the priority of this PR is lower than it should be.

(In reply to comment #13)
> For any transparent sub-graph, considering the expression anticipatable
> in this graph is beneficial if the sum of the frequencies of outgoing edges
> on which the expression is anticipatable is larger than the sum of the
> frequencies of incoming edges on which the expression is not available.

Actually, for SH variants with the FPCHG instruction, we could do a lower
cost mode switch if the previous mode is known.  Hence we can also add
some benefit for the other mode being anticipatable, and the cost of
the considered mode not being available is reduced when the other mode
is available.

Subject: Bug 28764

Author: kkojima
Date: Tue Sep  5 21:41:23 2006
New Revision: 116703

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116703
Log:
	PR target/28764
	* mode-switching.c (optimize_mode_switching): Make the destination
	block of an abnormal edge have no anticipatable mode.  Don't
	insert mode switching code at the end of the source block of
	an abnormal edge.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/mode-switching.c

Fixed.

(In reply to comment #12)

> As far as I can see, the i387 mode switching is already completely broken,
> because it treats the different modes of a single mode-switchable entity
> as separate entities.

NO, it is _NOT_ broken by any stretch of imagination!

Perhaps something could be writtenn in a more fancy way, but there is a reason, why we have separate entities for x87. Please note, that at mode switch point, we insert code that calculates mode word and stores the result in memory. This value is then used at the point, where mode is briefly switched for the insn to operate in desired mode.

So, consider this testcase:
--cut here--
double test(double *a, int x)
{
  int i;
  double z = 0.0;

  for (i = 0; i < x; i++)
    z += floor(a[i]) + ceil(a[i]);

  return z;
}
--cut here--

This gets compiled by current approach (-O2 -ffast-math) into:

        fnstcw  -6(%ebp)
        xorl    %edx, %edx
        fldz
        movzwl  -6(%ebp), %eax
        movb    $4, %ah
        movw    %ax, -10(%ebp)
        movzwl  -6(%ebp), %eax
        movb    $8, %ah
        movw    %ax, -8(%ebp)
        .p2align 4,,7
.L5:
        fldl    (%ebx,%edx,8)
        addl    $1, %edx
        fld     %st(0)
        cmpl    %ecx, %edx
        fldcw   -8(%ebp)
        frndint
        fldcw   -6(%ebp)
        fxch    %st(1)
        fldcw   -10(%ebp)
        frndint
        fldcw   -6(%ebp)
        faddp   %st, %st(1)
        faddp   %st, %st(1)
        jne     .L5

Note that mode word calculation is pushed out of the loop.

I actualy considered an idea that implemented proposed "several-modes-for-one-entity" approach for x87. However, modes are switched _inside_ the loop, so by this approach mode calculation code also stays inside:

.L5:
        fldl    (%ebx,%edx,8)
        addl    $1, %edx
        fnstcw  -6(%ebp)
        fld     %st(0)
        cmpl    %ecx, %edx
        movzwl  -6(%ebp), %eax
        movb    $8, %ah
        movw    %ax, -8(%ebp)
        movzwl  -6(%ebp), %eax
        fldcw   -8(%ebp)
        frndint
        fldcw   -6(%ebp)
        fxch    %st(1)
        movb    $4, %ah
        movw    %ax, -10(%ebp)
        fldcw   -10(%ebp)
        frndint
        fldcw   -6(%ebp)
        faddp   %st, %st(1)
        faddp   %st, %st(1)
        jne     .L5

Now, which code is preferred?