alpha failure on 920810-1

alpha failure on 920810-1

[Richard Henderson]

Transforming 

    (insn 9 8 10 (set (reg:DF 69)
            (eq:DF (reg/v:DF 68)
                (const_double:DF (cc0) 0 0))) -1 (nil)
        (nil))
    
    (jump_insn 10 9 12 (set (pc)
            (if_then_else (eq (reg:DF 69)
                    (const_double:DF (cc0) 0 0))
                (label_ref 13)
                (pc))) 204 {minsf3+1} (nil)
        (nil))
    
    (insn 12 10 13 (set (reg/v:DF 68)
            (const_double:DF (cc0) 0 0)) -1 (nil)
        (nil))
    
    (code_label 13 12 15 2 "")
    
to 

    (insn 25 23 15 (set (reg/v:DF 68)
            (if_then_else:DF (ne (reg:DF 68)
                    (const_double:DF (cc0) 0 0))
                (reg:DF 48 $f16)
                (reg:DF 72))) -1 (nil)
        (nil))

is not valid on Alpha.  The reason is that the limited comparison
done by the conditional move does not follow all of the IEEE rules
that the comparision insn does, but rather simply test that the
register has no bits set.

The correct transformation would be to retain the compare and base
the conditional move off of that:

    (insn 9 8 10 (set (reg:DF 69)
            (eq:DF (reg/v:DF 68)
                (const_double:DF (cc0) 0 0))) -1 (nil)
        (nil))

    (insn 25 23 15 (set (reg/v:DF 68)
            (if_then_else:DF (ne (reg:DF 69)
                    (const_double:DF (cc0) 0 0))
                (reg:DF 48 $f16)
                (reg:DF 72))) -1 (nil)
        (nil))

The following patch modifies get_condition to recognize this situation,
i.e. compares in an FP mode and a branch in VOIDmode, and to prevent
the two operations from being merged.

I have no idea if this is reasonable for other CCmode-less platforms.

Comments?


r~



Mon Apr 27 01:27:53 1998  Richard Henderson  <rth@cygnus.com>

	* loop.c (get_condition): If comparision insn does the cmp in an
	FP mode, and the jump insn does the cmp in VOIDmode, don't combine.


Index: loop.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/loop.c,v
retrieving revision 1.44
diff -c -p -d -r1.44 loop.c
*** loop.c	1998/04/25 16:09:23	1.44
--- loop.c	1998/04/27 08:27:35
*************** get_condition (jump, earliest)
*** 6961,6966 ****
--- 6961,6967 ----
    rtx op0, op1;
    int reverse_code = 0;
    int did_reverse_condition = 0;
+   enum machine_mode mode;
  
    /* If this is not a standard conditional jump, we can't parse it.  */
    if (GET_CODE (jump) != JUMP_INSN
*************** get_condition (jump, earliest)
*** 6968,6973 ****
--- 6969,6975 ----
      return 0;
  
    code = GET_CODE (XEXP (SET_SRC (PATTERN (jump)), 0));
+   mode = GET_MODE (XEXP (SET_SRC (PATTERN (jump)), 0));
    op0 = XEXP (XEXP (SET_SRC (PATTERN (jump)), 0), 0);
    op1 = XEXP (XEXP (SET_SRC (PATTERN (jump)), 0), 1);
  
*************** get_condition (jump, earliest)
*** 7084,7089 ****
--- 7086,7103 ----
  
        if (x)
  	{
+ 	  /* A separate fp comparision instruction may follow IEEE rules that
+ 	     the jump comparision does not.  Define this case to be when the
+ 	     jump is in VOIDmode and the comparison isn't.  */
+ 	  if (TARGET_FLOAT_FORMAT == IEEE_FLOAT_FORMAT
+ 	      && ! flag_fast_math
+ 	      && mode == VOIDmode && FLOAT_MODE_P (GET_MODE (x)))
+ 	    {
+ 	      did_reverse_condition ^= reverse_code;
+ 	      reverse_code = 0;
+ 	      break;
+ 	    }
+ 
  	  if (GET_RTX_CLASS (GET_CODE (x)) == '<')
  	    code = GET_CODE (x);
  	  if (reverse_code)

Re: alpha failure on 920810-1

[Joern Rennecke]

>     (insn 9 8 10 (set (reg:DF 69)
>             (eq:DF (reg/v:DF 68)
>                 (const_double:DF (cc0) 0 0))) -1 (nil)
>         (nil))

Does this RTL actually describe correctly what is going on on the alpha?
Does in generate floating-point truth values, i.e. 1.0 for true?

Re: alpha failure on 920810-1

[Richard Henderson]

On Mon, Apr 27, 1998 at 04:46:27PM +0100, Joern Rennecke wrote:
> >     (insn 9 8 10 (set (reg:DF 69)
> >             (eq:DF (reg/v:DF 68)
> >                 (const_double:DF (cc0) 0 0))) -1 (nil)
> >         (nil))
> 
> Does this RTL actually describe correctly what is going on on the alpha?
> Does in generate floating-point truth values, i.e. 1.0 for true?

Yes.  See FLOAT_STORE_FLAG_VALUE.


r~

Re: alpha failure on 920810-1

[Jim Wilson]

	The reason is that the limited comparison
	done by the conditional move does not follow all of the IEEE rules
	that the comparision insn does, but rather simply test that the
	register has no bits set.

I suspect that there are no currently supported machines for which your
get_condition change is wrong, but there may be future architectures for
which is it wrong.  In any case, it doesn't seem to really fix the problem.
For instance, you will probably see the same invalid transformations from
combine.  Fixing loop doesn't help with that.

To fix this elegantly, we need to make the RTL operations distinct if
they are performing different operations.  Since we can't do this with
CCmodes, it seems to imply that we need new comparison operators.  For
instance, perhaps we need eqieee separate from eq, similar to how gtu is
separate from gt.  This would require changing a lot of code though to
handle the new operators though.  That would be a major task.

Adding a strategic unspec operation could fix the problem, at the expense
of uglifying the alpha.md file a bit, and perhaps losing some optimizations.
It is likely a simpler solution though.

I don't see any clean and simple way to fix the problem.

Jim

Re: alpha failure on 920810-1

[Richard Henderson]

On Tue, Apr 28, 1998 at 09:30:43PM -0700, Jim Wilson wrote:
> I suspect that there are no currently supported machines for which your
> get_condition change is wrong, but there may be future architectures for
> which is it wrong.

Actually, part of the comment was _defining_ FPmode+VOIDmode to act a 
that way.  That future architecture might use FPmode+FPmode for that.

Sigh.  It wasn't such a good idea, I guess.

> Adding a strategic unspec operation could fix the problem, at the expense
> of uglifying the alpha.md file a bit, and perhaps losing some optimizations.
> It is likely a simpler solution though.

Hum.  I suppose 

	(set (reg:DF 1)
	  (eq:DF (reg:DF 2) (reg:DF 3)))
	(set (reg:DF 4)
	  (if_then_else:DF (eq (unspec [(reg:DF 1)] 9)
			       (const_double 0))
			   (reg:DF 5)
			   (reg:DF 4)))

would work.  It would still allow the sense of the comparisons to be
switched while disallowing their combination.  It doesn't strike me
as particularly nice, though.

As for combine, I think it needs fixing for other reasons -- Sparc v9
is failing the same test because of the same type of problem, that is,
the entire function being optimized away.  But it is doing it in a way
different from what was happening on Alpha -- I havn't examined it in
detail yet.

So you think changing get_condition and possibly combine is the wrong
approach, eh?  Well, I guess I'll figure something out...


r~

Re: alpha failure on 920810-1

[Jim Wilson]

	Actually, part of the comment was _defining_ FPmode+VOIDmode to act a 
	that way.  That future architecture might use FPmode+FPmode for that.

Since VOIDmode in a pattern really means accept any mode, I don't think this
works as well as one might hope.

	So you think changing get_condition and possibly combine is the wrong
	approach, eh?  Well, I guess I'll figure something out...

Well, I wouldn't say that this was the `wrong' approach, better would be to say
that it made me uneasy.

Jim

Re: alpha failure on 920810-1

[Joern Rennecke]

> 	The reason is that the limited comparison
> 	done by the conditional move does not follow all of the IEEE rules
> 	that the comparision insn does, but rather simply test that the
> 	register has no bits set.
...
> To fix this elegantly, we need to make the RTL operations distinct if
> they are performing different operations.  Since we can't do this with
> CCmodes, it seems to imply that we need new comparison operators.  For
> instance, perhaps we need eqieee separate from eq, similar to how gtu is
> separate from gt.  This would require changing a lot of code though to
> handle the new operators though.  That would be a major task.
> 
> Adding a strategic unspec operation could fix the problem, at the expense
> of uglifying the alpha.md file a bit, and perhaps losing some optimizations.
> It is likely a simpler solution though.

If it tests only if all bits are zero, I think we should express this
as a DImode equality test to zero:

        (set (reg:DF 1)
          (eq:DF (reg:DF 2) (reg:DF 3)))
        (set (reg:DF 4)
          (if_then_else:DF (eq (subreg:DI (reg:DF 1) 0)
                               (const_int 0))
                           (reg:DF 5)
                           (reg:DF 4)))

Re: alpha failure on 920810-1

[Richard Henderson]

On Wed, Apr 29, 1998 at 10:13:05PM +0100, Joern Rennecke wrote:
> If it tests only if all bits are zero, I think we should express this
> as a DImode equality test to zero:
> 
>         (set (reg:DF 1)
>           (eq:DF (reg:DF 2) (reg:DF 3)))
>         (set (reg:DF 4)
>           (if_then_else:DF (eq (subreg:DI (reg:DF 1) 0)
>                                (const_int 0))
>                            (reg:DF 5)
>                            (reg:DF 4)))

Unfortunately that is not the whole truth.  I read the fine print
last night and found that it handles -0.0 specially, so it is not
really a DImode compare.  Which is unfortunate, since I'd like to 
be able to do real DImode comparisons with them...

In conversation with Jim today, we decided that 

         (set (reg:DF 1) (eq:DF (reg:DF 2) (reg:DF 3)))
         (set (reg:DF 4)
           (if_then_else:DF (eq:CC (reg:DF 1) (const_int 0))
             (reg:DF 5) (reg:DF 4)))

is a reasonable compromise.  It turns out that combine already knows
that it can't do anything with CCmode compares, and with my previous
patch modified just a tad, that takes care of loop, jump, and cse.

I've bootstrapped succesfully with these changes, and am currently
running Spec95.  Patches will follow once no regressions are shown.


r~

Re: alpha failure on 920810-1

[Joern Rennecke]

> On Wed, Apr 29, 1998 at 10:13:05PM +0100, Joern Rennecke wrote:
> > If it tests only if all bits are zero, I think we should express this
> > as a DImode equality test to zero:
> > 
> >         (set (reg:DF 1)
> >           (eq:DF (reg:DF 2) (reg:DF 3)))
> >         (set (reg:DF 4)
> >           (if_then_else:DF (eq (subreg:DI (reg:DF 1) 0)
> >                                (const_int 0))
> >                            (reg:DF 5)
> >                            (reg:DF 4)))
> 
> Unfortunately that is not the whole truth.  I read the fine print
> last night and found that it handles -0.0 specially, so it is not
> really a DImode compare.  Which is unfortunate, since I'd like to 
> be able to do real DImode comparisons with them...

If that is the case, what is the problem?  After all, this is an
equality comparison, and if a NaN is found unequal because it is
unequal to everything, or becuase it is unequal to 0, doesn't really matter,
does it?

Or are you missing signals when you are comparing signaling NaNs?

Re: alpha failure on 920810-1

[Richard Henderson]

On Thu, Apr 30, 1998 at 05:51:56PM +0100, Joern Rennecke wrote:
> Or are you missing signals when you are comparing signaling NaNs?

Exactly.


r~

Re: alpha failure on 920810-1

[Richard Henderson]

On Wed, Apr 29, 1998 at 10:57:53PM -0700, I wrote:
> In conversation with Jim today, we decided that 
> 
>          (set (reg:DF 1) (eq:DF (reg:DF 2) (reg:DF 3)))
>          (set (reg:DF 4)
>            (if_then_else:DF (eq:CC (reg:DF 1) (const_int 0))
>              (reg:DF 5) (reg:DF 4)))
> 
> is a reasonable compromise.  It turns out that combine already knows
> that it can't do anything with CCmode compares, and with my previous
> patch modified just a tad, that takes care of loop, jump, and cse.
> 
> I've bootstrapped succesfully with these changes, and am currently
> running Spec95.  Patches will follow once no regressions are shown.

Here they are, as promised.  Ok to apply?


r~



Thu Apr 30 17:27:47 1998  Richard Henderson  <rth@cygnus.com>

	* loop.c (get_condition): Don't combine when either compare is MODE_CC.
	* machmode.h (COMPLEX_MODE_P): New macro.
	* alpha.c (alpha_emit_conditional_branch): New function.  Taken from
	the body of beq, additionally set the mode of the branch to CCmode for
	FP compares and not fast_math.  
	(alpha_emit_conditional_move): Always use a compare insn for FP
	when not fast_math, as well as setting CCmode on the cmov.
	* alpha.md (beq, bne, blt, et al): Call alpha_emit_conditional_branch.


Index: gcc/loop.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/loop.c,v
retrieving revision 1.45
diff -c -p -d -r1.45 loop.c
*** loop.c	1998/04/27 18:39:07	1.45
--- loop.c	1998/05/01 00:01:27
*************** get_condition (jump, earliest)
*** 6975,6980 ****
--- 6975,6981 ----
    rtx op0, op1;
    int reverse_code = 0;
    int did_reverse_condition = 0;
+   enum machine_mode mode;
  
    /* If this is not a standard conditional jump, we can't parse it.  */
    if (GET_CODE (jump) != JUMP_INSN
*************** get_condition (jump, earliest)
*** 6982,6987 ****
--- 6983,6989 ----
      return 0;
  
    code = GET_CODE (XEXP (SET_SRC (PATTERN (jump)), 0));
+   mode = GET_MODE (XEXP (SET_SRC (PATTERN (jump)), 0));
    op0 = XEXP (XEXP (SET_SRC (PATTERN (jump)), 0), 0);
    op1 = XEXP (XEXP (SET_SRC (PATTERN (jump)), 0), 1);
  
*************** get_condition (jump, earliest)
*** 7048,7053 ****
--- 7050,7062 ----
  	{
  	  enum machine_mode inner_mode = GET_MODE (SET_SRC (set));
  
+ 	  /* ??? We may not combine comparisons done in a CCmode with
+ 	     comparisons not done in a CCmode.  This is to aid targets
+ 	     like Alpha that have an IEEE compliant EQ instruction, and
+ 	     a non-IEEE compliant BEQ instruction.  The use of CCmode is
+ 	     actually artificial, simply to prevent the combination, but
+ 	     should not affect other platforms.  */
+ 
  	  if ((GET_CODE (SET_SRC (set)) == COMPARE
  	       || (((code == NE
  		     || (code == LT
*************** get_condition (jump, earliest)
*** 7063,7069 ****
  			 && FLOAT_STORE_FLAG_VALUE < 0)
  #endif
  		     ))
! 		   && GET_RTX_CLASS (GET_CODE (SET_SRC (set))) == '<')))
  	    x = SET_SRC (set);
  	  else if (((code == EQ
  		     || (code == GE
--- 7072,7080 ----
  			 && FLOAT_STORE_FLAG_VALUE < 0)
  #endif
  		     ))
! 		   && GET_RTX_CLASS (GET_CODE (SET_SRC (set))) == '<'))
! 	      && ((GET_MODE_CLASS (mode) == MODE_CC)
! 		  != (GET_MODE_CLASS (inner_mode) == MODE_CC)))
  	    x = SET_SRC (set);
  	  else if (((code == EQ
  		     || (code == GE
*************** get_condition (jump, earliest)
*** 7079,7085 ****
  			 && FLOAT_STORE_FLAG_VALUE < 0)
  #endif
  		     ))
! 		   && GET_RTX_CLASS (GET_CODE (SET_SRC (set))) == '<')
  	    {
  	      /* We might have reversed a LT to get a GE here.  But this wasn't
  		 actually the comparison of data, so we don't flag that we
--- 7090,7098 ----
  			 && FLOAT_STORE_FLAG_VALUE < 0)
  #endif
  		     ))
! 		   && GET_RTX_CLASS (GET_CODE (SET_SRC (set))) == '<'
! 	           && ((GET_MODE_CLASS (mode) == MODE_CC)
! 		       != (GET_MODE_CLASS (inner_mode) == MODE_CC)))
  	    {
  	      /* We might have reversed a LT to get a GE here.  But this wasn't
  		 actually the comparison of data, so we don't flag that we
Index: gcc/machmode.h
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/machmode.h,v
retrieving revision 1.2
diff -c -p -d -r1.2 machmode.h
*** machmode.h	1998/04/03 16:36:11	1.2
--- machmode.h	1998/05/01 00:01:27
*************** extern enum mode_class mode_class[];
*** 164,169 ****
--- 164,174 ----
    (GET_MODE_CLASS (MODE) == MODE_FLOAT	\
     || GET_MODE_CLASS (MODE) == MODE_COMPLEX_FLOAT)
  
+ /* Nonzero if MODE is a complex mode.  */
+ #define COMPLEX_MODE_P(MODE)			\
+   (GET_MODE_CLASS (MODE) == MODE_COMPLEX_INT	\
+    || GET_MODE_CLASS (MODE) == MODE_COMPLEX_FLOAT)
+ 
  /* Get the size in bytes of an object of mode MODE.  */
  
  extern int mode_size[];
Index: gcc/config/alpha/alpha.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/config/alpha/alpha.c,v
retrieving revision 1.38
diff -c -p -d -r1.38 alpha.c
*** alpha.c	1998/04/26 14:00:10	1.38
--- alpha.c	1998/05/01 00:01:28
*************** alpha_emit_set_long_const (target, c)
*** 1223,1228 ****
--- 1223,1342 ----
  }
  #endif /* HOST_BITS_PER_WIDE_INT == 64 */
  
+ /* Generate the comparison for a conditional branch.  */
+ 
+ rtx
+ alpha_emit_conditional_branch (code)
+      enum rtx_code code;
+ {
+   enum rtx_code cmp_code, branch_code;
+   enum machine_mode cmp_mode, branch_mode = VOIDmode;
+   rtx op0 = alpha_compare_op0, op1 = alpha_compare_op1;
+   rtx tem;
+ 
+   /* The general case: fold the comparison code to the types of compares
+      that we have, choosing the branch as necessary.  */
+   switch (code)
+     {
+     case EQ:  case LE:  case LT:  case LEU:  case LTU:
+       /* We have these compares: */
+       cmp_code = code, branch_code = NE;
+       break;
+ 
+     case NE:
+       /* This must be reversed. */
+       cmp_code = EQ, branch_code = EQ;
+       break;
+ 
+     case GE:  case GT: case GEU:  case GTU:
+       /* For FP, we swap them, for INT, we reverse them.  */
+       if (alpha_compare_fp_p)
+ 	{
+ 	  cmp_code = swap_condition (code);
+ 	  branch_code = NE;
+ 	  tem = op0, op0 = op1, op1 = tem;
+ 	}
+       else
+ 	{
+ 	  cmp_code = reverse_condition (code);
+ 	  branch_code = EQ;
+ 	}
+       break;
+ 
+     default:
+       abort ();
+     }
+ 
+   if (alpha_compare_fp_p)
+     {
+       cmp_mode = DFmode;
+       if (flag_fast_math)
+ 	{
+ 	  /* When we are not as concerned about non-finite values, and we
+ 	     are comparing against zero, we can branch directly.  */
+ 	  if (op1 == CONST0_RTX (DFmode))
+ 	    cmp_code = NIL, branch_code = code;
+ 	  else if (op0 == CONST0_RTX (DFmode))
+ 	    {
+ 	      /* Undo the swap we probably did just above.  */
+ 	      tem = op0, op0 = op1, op1 = tem;
+ 	      cmp_code = NIL, branch_code = swap_condition (cmp_code);
+ 	    }
+ 	}
+       else
+ 	{
+ 	  /* ??? We mark the the branch mode to be CCmode to prevent the
+ 	     compare and branch from being combined, since the compare 
+ 	     insn follows IEEE rules that the branch does not.  */
+ 	  branch_mode = CCmode;
+ 	}
+     }
+   else
+     {
+       cmp_mode = DImode;
+ 
+       /* The following optimizations are only for signed compares.  */
+       if (code != LEU && code != LTU && code != GEU && code != GTU)
+ 	{
+ 	  /* Whee.  Compare and branch against 0 directly.  */
+ 	  if (op1 == const0_rtx)
+ 	    cmp_code = NIL, branch_code = code;
+ 
+ 	  /* We want to use cmpcc/bcc when we can, since there is a zero delay
+ 	     bypass between logicals and br/cmov on EV5.  But we don't want to
+ 	     force valid immediate constants into registers needlessly.  */
+ 	  else if (GET_CODE (op1) == CONST_INT)
+ 	    {
+ 	      HOST_WIDE_INT v = INTVAL (op1), n = -v;
+ 
+ 	      if (! CONST_OK_FOR_LETTER_P (v, 'I')
+ 		  && (CONST_OK_FOR_LETTER_P (n, 'K')
+ 		      || CONST_OK_FOR_LETTER_P (n, 'L')))
+ 		{
+ 		  cmp_code = PLUS, branch_code = code;
+ 		  op1 = GEN_INT (n);
+ 		}
+ 	    }
+ 	}
+     }
+ 
+   /* Force op0 into a register.  */
+   if (GET_CODE (op0) != REG)
+     op0 = force_reg (cmp_mode, op0);
+ 
+   /* Emit an initial compare instruction, if necessary.  */
+   tem = op0;
+   if (cmp_code != NIL)
+     {
+       tem = gen_reg_rtx (cmp_mode);
+       emit_move_insn (tem, gen_rtx_fmt_ee (cmp_code, cmp_mode, op0, op1));
+     }
+ 
+   /* Return the branch comparison.  */
+   return gen_rtx_fmt_ee (branch_code, branch_mode, tem, CONST0_RTX (cmp_mode));
+ }
+ 
+ 
  /* Rewrite a comparison against zero CMP of the form
     (CODE (cc0) (const_int 0)) so it can be written validly in
     a conditional move (if_then_else CMP ...).
*************** alpha_emit_conditional_move (cmp, mode)
*** 1241,1246 ****
--- 1355,1361 ----
    enum machine_mode cmp_mode
      = (GET_MODE (op0) == VOIDmode ? DImode : GET_MODE (op0));
    enum machine_mode cmp_op_mode = alpha_compare_fp_p ? DFmode : DImode;
+   enum machine_mode cmov_mode = VOIDmode;
    rtx tem;
  
    if (alpha_compare_fp_p != FLOAT_MODE_P (mode))
*************** alpha_emit_conditional_move (cmp, mode)
*** 1249,1254 ****
--- 1364,1370 ----
    /* We may be able to use a conditional move directly.
       This avoids emitting spurious compares. */
    if (signed_comparison_operator (cmp, cmp_op_mode)
+       && (!alpha_compare_fp_p || flag_fast_math)
        && (op0 == CONST0_RTX (cmp_mode) || op1 == CONST0_RTX (cmp_mode)))
      return gen_rtx_fmt_ee (code, VOIDmode, op0, op1);
  
*************** alpha_emit_conditional_move (cmp, mode)
*** 1281,1289 ****
        abort ();
      }
  
    tem = gen_reg_rtx (cmp_op_mode);
    emit_move_insn (tem, gen_rtx_fmt_ee (code, cmp_op_mode, op0, op1));
!   return gen_rtx_fmt_ee (cmov_code, VOIDmode, tem, CONST0_RTX (cmp_op_mode));
  }
  \f
  /* Use ext[wlq][lh] as the Architecture Handbook describes for extracting
--- 1397,1411 ----
        abort ();
      }
  
+   /* ??? We mark the the branch mode to be CCmode to prevent the compare
+      and cmov from being combined, since the compare insn follows IEEE
+      rules that the cmov does not.  */
+   if (alpha_compare_fp_p && !flag_fast_math)
+     cmov_mode = CCmode;
+ 
    tem = gen_reg_rtx (cmp_op_mode);
    emit_move_insn (tem, gen_rtx_fmt_ee (code, cmp_op_mode, op0, op1));
!   return gen_rtx_fmt_ee (cmov_code, cmov_mode, tem, CONST0_RTX (cmp_op_mode));
  }
  \f
  /* Use ext[wlq][lh] as the Architecture Handbook describes for extracting
Index: gcc/config/alpha/alpha.h
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/config/alpha/alpha.h,v
retrieving revision 1.31
diff -c -p -d -r1.31 alpha.h
*** alpha.h	1998/04/26 23:21:55	1.31
--- alpha.h	1998/05/01 00:01:28
*************** extern int alpha_memory_latency;
*** 1076,1081 ****
--- 1076,1082 ----
     insns and emitted.  */
  extern struct rtx_def *alpha_emit_set_const ();
  extern struct rtx_def *alpha_emit_set_long_const ();
+ extern struct rtx_def *alpha_emit_conditional_branch ();
  extern struct rtx_def *alpha_emit_conditional_move ();
  
  /* Generate necessary RTL for __builtin_saveregs().
*************** do {							\
*** 2417,2420 ****
  
  /* Prototypes for alpha.c functions used in the md file.  */
  extern struct rtx_def *get_unaligned_address ();
-  
--- 2418,2420 ----
Index: gcc/config/alpha/alpha.md
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/config/alpha/alpha.md,v
retrieving revision 1.35
diff -c -p -d -r1.35 alpha.md
*** alpha.md	1998/04/14 23:50:17	1.35
--- alpha.md	1998/05/01 00:01:31
***************
*** 2855,3066 ****
  }")
  
  (define_expand "beq"
!   [(set (match_dup 1) (match_dup 2))
!    (set (pc)
! 	(if_then_else (match_dup 3)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "
! {
!   enum machine_mode mode;
!   enum rtx_code compare_code = EQ, branch_code = NE;
! 
!   if (alpha_compare_fp_p)
!     mode = DFmode;
!   else
!     {
!       mode = DImode;
!       /* We want to use cmpeq/bne when we can, since there is a zero-delay
! 	 bypass between logicals and br/cmov on the 21164.  But we don't
! 	 want to force valid immediate constants into registers needlessly.  */
!       if (GET_CODE (alpha_compare_op1) == CONST_INT
! 	  && ((INTVAL (alpha_compare_op1) >= -0x8000
! 	       && INTVAL (alpha_compare_op1) < 0)
! 	      || (INTVAL (alpha_compare_op1) > 0xff
! 		  && INTVAL (alpha_compare_op1) < 0x8000)))
! 	{
! 	  compare_code = PLUS, branch_code = EQ;
! 	  alpha_compare_op1 = GEN_INT (- INTVAL (alpha_compare_op1));
! 	}
!     }
! 
!   operands[1] = gen_reg_rtx (mode);
!   operands[2] = gen_rtx_fmt_ee (compare_code, mode,
! 				alpha_compare_op0, alpha_compare_op1);
!   operands[3] = gen_rtx_fmt_ee (branch_code, VOIDmode,
! 				operands[1], CONST0_RTX (mode));
! }")
  
  (define_expand "bne"
!   [(set (match_dup 1) (match_dup 2))
!    (set (pc)
! 	(if_then_else (match_dup 3)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "
! {
!   enum machine_mode mode;
!   enum rtx_code compare_code = EQ, branch_code = EQ;
! 
!   if (alpha_compare_fp_p)
!     mode = DFmode;
!   else
!     {
!       mode = DImode;
!       /* We want to use cmpeq/bne when we can, since there is a zero-delay
! 	 bypass between logicals and br/cmov on the 21164.  But we don't
! 	 want to force valid immediate constants into registers needlessly.  */
!       if (GET_CODE (alpha_compare_op1) == CONST_INT
! 	  && ((INTVAL (alpha_compare_op1) >= -0x8000
! 	       && INTVAL (alpha_compare_op1) < 0)
! 	      || (INTVAL (alpha_compare_op1) > 0xff
! 		  && INTVAL (alpha_compare_op1) < 0x8000)))
! 	{
! 	  compare_code = PLUS, branch_code = NE;
! 	  alpha_compare_op1 = GEN_INT (- INTVAL (alpha_compare_op1));
! 	}
!     }
! 
!   operands[1] = gen_reg_rtx (mode);
!   operands[2] = gen_rtx_fmt_ee (compare_code, mode,
! 				alpha_compare_op0, alpha_compare_op1);
!   operands[3] = gen_rtx_fmt_ee (branch_code, VOIDmode,
! 				operands[1], CONST0_RTX (mode));
! }")
  
  (define_expand "blt"
!   [(set (match_dup 1) (match_dup 2))
!    (set (pc)
! 	(if_then_else (match_dup 3)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "
! {
!   enum machine_mode mode = alpha_compare_fp_p ? DFmode : DImode;
!   operands[1] = gen_reg_rtx (mode);
!   operands[2] = gen_rtx_LT (mode, alpha_compare_op0, alpha_compare_op1);
!   operands[3] = gen_rtx_NE (VOIDmode, operands[1], CONST0_RTX (mode));
! }")
  
  (define_expand "ble"
!   [(set (match_dup 1) (match_dup 2))
!    (set (pc)
! 	(if_then_else (match_dup 3)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "
! {
!   enum machine_mode mode = alpha_compare_fp_p ? DFmode : DImode;
!   operands[1] = gen_reg_rtx (mode);
!   operands[2] = gen_rtx_LE (mode, alpha_compare_op0, alpha_compare_op1);
!   operands[3] = gen_rtx_NE (VOIDmode, operands[1], CONST0_RTX (mode));
! }")
  
  (define_expand "bgt"
!   [(set (match_dup 1) (match_dup 2))
!    (set (pc)
! 	(if_then_else (match_dup 3)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "
! {
!   if (alpha_compare_fp_p)
!     {
!       operands[1] = gen_reg_rtx (DFmode);
!       operands[2] = gen_rtx_LT (DFmode, alpha_compare_op1, alpha_compare_op0);
!       operands[3] = gen_rtx_NE (VOIDmode, operands[1], CONST0_RTX (DFmode));
!     }
!   else
!     {
!       operands[1] = gen_reg_rtx (DImode);
!       operands[2] = gen_rtx_LE (DImode, alpha_compare_op0, alpha_compare_op1);
!       operands[3] = gen_rtx_EQ (VOIDmode, operands[1], const0_rtx);
!     }
! }")
  
  (define_expand "bge"
!   [(set (match_dup 1) (match_dup 2))
!    (set (pc)
! 	(if_then_else (match_dup 3)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "
! {
!   if (alpha_compare_fp_p)
!     {
!       operands[1] = gen_reg_rtx (DFmode);
!       operands[2] = gen_rtx_LE (DFmode, alpha_compare_op1, alpha_compare_op0);
!       operands[3] = gen_rtx_NE (VOIDmode, operands[1], CONST0_RTX (DFmode));
!     }
!   else
!     {
!       operands[1] = gen_reg_rtx (DImode);
!       operands[2] = gen_rtx_LT (DImode, alpha_compare_op0, alpha_compare_op1);
!       operands[3] = gen_rtx_EQ (VOIDmode, operands[1], const0_rtx);
!     }
! }")
  
  (define_expand "bltu"
!   [(set (match_dup 1) (match_dup 2))
!    (set (pc)
! 	(if_then_else (match_dup 3)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "
! {
!   operands[1] = gen_reg_rtx (DImode);
!   operands[2] = gen_rtx_LTU (DImode, alpha_compare_op0, alpha_compare_op1);
!   operands[3] = gen_rtx_NE (VOIDmode, operands[1], const0_rtx);
! }")
  
  (define_expand "bleu"
!   [(set (match_dup 1) (match_dup 2))
!    (set (pc)
! 	(if_then_else (match_dup 3)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "
! {
!   operands[1] = gen_reg_rtx (DImode);
!   operands[2] = gen_rtx_LEU (DImode, alpha_compare_op0, alpha_compare_op1);
!   operands[3] = gen_rtx_NE (VOIDmode, operands[1], const0_rtx);
! }")
  
  (define_expand "bgtu"
!   [(set (match_dup 1) (match_dup 2))
!    (set (pc)
! 	(if_then_else (match_dup 3)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "
! {
!   operands[1] = gen_reg_rtx (DImode);
!   operands[2] = gen_rtx_LEU (DImode, alpha_compare_op0, alpha_compare_op1);
!   operands[3] = gen_rtx_EQ (VOIDmode, operands[1], const0_rtx);
! }")
  
  (define_expand "bgeu"
!   [(set (match_dup 1) (match_dup 2))
!    (set (pc)
! 	(if_then_else (match_dup 3)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "
! {
!   operands[1] = gen_reg_rtx (DImode);
!   operands[2] = gen_rtx_LTU (DImode, alpha_compare_op0, alpha_compare_op1);
!   operands[3] = gen_rtx_EQ (VOIDmode, operands[1], const0_rtx);
! }")
  
  (define_expand "seq"
    [(set (match_operand:DI 0 "register_operand" "")
--- 2855,2938 ----
  }")
  
  (define_expand "beq"
!   [(set (pc)
! 	(if_then_else (match_dup 1)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "{ operands[1] = alpha_emit_conditional_branch (EQ); }")
  
  (define_expand "bne"
!   [(set (pc)
! 	(if_then_else (match_dup 1)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "{ operands[1] = alpha_emit_conditional_branch (NE); }")
  
  (define_expand "blt"
!   [(set (pc)
! 	(if_then_else (match_dup 1)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "{ operands[1] = alpha_emit_conditional_branch (LT); }")
  
  (define_expand "ble"
!   [(set (pc)
! 	(if_then_else (match_dup 1)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "{ operands[1] = alpha_emit_conditional_branch (LE); }")
  
  (define_expand "bgt"
!   [(set (pc)
! 	(if_then_else (match_dup 1)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "{ operands[1] = alpha_emit_conditional_branch (GT); }")
  
  (define_expand "bge"
!   [(set (pc)
! 	(if_then_else (match_dup 1)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "{ operands[1] = alpha_emit_conditional_branch (GE); }")
  
  (define_expand "bltu"
!   [(set (pc)
! 	(if_then_else (match_dup 1)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "{ operands[1] = alpha_emit_conditional_branch (LTU); }")
  
  (define_expand "bleu"
!   [(set (pc)
! 	(if_then_else (match_dup 1)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "{ operands[1] = alpha_emit_conditional_branch (LEU); }")
  
  (define_expand "bgtu"
!   [(set (pc)
! 	(if_then_else (match_dup 1)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "{ operands[1] = alpha_emit_conditional_branch (GTU); }")
  
  (define_expand "bgeu"
!   [(set (pc)
! 	(if_then_else (match_dup 1)
  		      (label_ref (match_operand 0 "" ""))
  		      (pc)))]
    ""
!   "{ operands[1] = alpha_emit_conditional_branch (GEU); }")
  
  (define_expand "seq"
    [(set (match_operand:DI 0 "register_operand" "")

Re: alpha failure on 920810-1

[Jeffrey A Law]

  In message <19980427014147.34497@dot.cygnus.com>you write:
  > The correct transformation would be to retain the compare and base
  > the conditional move off of that:
  > 
  >     (insn 9 8 10 (set (reg:DF 69)
  >             (eq:DF (reg/v:DF 68)
  >                 (const_double:DF (cc0) 0 0))) -1 (nil)
  >         (nil))
  > 
  >     (insn 25 23 15 (set (reg/v:DF 68)
  >             (if_then_else:DF (ne (reg:DF 69)
  >                     (const_double:DF (cc0) 0 0))
  >                 (reg:DF 48 $f16)
  >                 (reg:DF 72))) -1 (nil)
  >         (nil))
I wonder if you could effectively do this at insn output time?

The problem is you can end up with redundant compares.  You could
use the machine dependent reorg pass to try and clean up some of
the redundant comparisons.

Just thinking outloud...

jeff

Re: alpha failure on 920810-1

[Jeffrey A Law]

  In message < 19980430173828.15223@dot.cygnus.com >you write:
  > On Wed, Apr 29, 1998 at 10:57:53PM -0700, I wrote:
  > > In conversation with Jim today, we decided that 
  > > 
  > >          (set (reg:DF 1) (eq:DF (reg:DF 2) (reg:DF 3)))
  > >          (set (reg:DF 4)
  > >            (if_then_else:DF (eq:CC (reg:DF 1) (const_int 0))
  > >              (reg:DF 5) (reg:DF 4)))
  > > 
  > > is a reasonable compromise.  It turns out that combine already knows
  > > that it can't do anything with CCmode compares, and with my previous
  > > patch modified just a tad, that takes care of loop, jump, and cse.
  > > 
  > > I've bootstrapped succesfully with these changes, and am currently
  > > running Spec95.  Patches will follow once no regressions are shown.
  > 
  > Here they are, as promised.  Ok to apply?
  > 
  > 
  > r~
  > 
  > 
  > 
  > Thu Apr 30 17:27:47 1998  Richard Henderson  <rth@cygnus.com>
  > 
  > 	* loop.c (get_condition): Don't combine when either compare is MODE_CC.
  > 	* machmode.h (COMPLEX_MODE_P): New macro.
  > 	* alpha.c (alpha_emit_conditional_branch): New function.  Taken from
  > 	the body of beq, additionally set the mode of the branch to CCmode for
  > 	FP compares and not fast_math.  
  > 	(alpha_emit_conditional_move): Always use a compare insn for FP
  > 	when not fast_math, as well as setting CCmode on the cmov.
  > 	* alpha.md (beq, bne, blt, et al): Call alpha_emit_conditional_branch.
Seems pretty reasonable to me if Jim doesn't have objections.

I note that your patch doesn't actually use COMPLEX_MODE_P, but I do
think we ought to have it to be consistent with the other *_MODE_P
tests.

jeff

Re: alpha failure on 920810-1

[Richard Henderson]

On Fri, May 08, 1998 at 02:18:14PM -0600, Jeffrey A Law wrote:
>   > 	* loop.c (get_condition): Don't combine when either compare is MODE_CC.
>   > 	* machmode.h (COMPLEX_MODE_P): New macro.
>   > 	* alpha.c (alpha_emit_conditional_branch): New function.  Taken from
>   > 	the body of beq, additionally set the mode of the branch to CCmode for
>   > 	FP compares and not fast_math.  
>   > 	(alpha_emit_conditional_move): Always use a compare insn for FP
>   > 	when not fast_math, as well as setting CCmode on the cmov.
>   > 	* alpha.md (beq, bne, blt, et al): Call alpha_emit_conditional_branch.

Applied.

> I note that your patch doesn't actually use COMPLEX_MODE_P, but I do
> think we ought to have it to be consistent with the other *_MODE_P
> tests.

Heh, didn't even notice that got left in; a previous version of
the patch did use it.  I checked that in separately.


r~