Re: GCC 2.7.2.3 good, EGCS 1.0.3 bad for x86 subtract then test

[Richard Henderson <rth at cygnus dot com>]

On Mon, Dec 21, 1998 at 10:52:13AM +0000, Jamie Lokier wrote:
> Here's the example program:
> 
> 	extern void a (void);
> 
> 	void f (int x, int y)
> 	{
> 	  if ((x -= y) >= 0)
> 	    a ();
> 	}

This is solved by a patch Jeff sent me just yesterday.


r~


From: Jeffrey A Law <law@cygnus.com>
To: rth@cygnus.com
Reply-To: law@cygnus.com
Subject: regmove change to improve x86 code
Date: Tue, 22 Dec 1998 13:23:55 -0700

Here's the patch I mentioned in the meeting.

Basically we had something like this before combine:

(set (reg1) (mem (blah))
(set (reg2) (mem (oof))
(set (reg3) (minus (reg1) (reg2))

Combine turned that into:

(set (reg3) (minus (mem (blah)) (reg2))

Which is allowed by the predicates for the subsi3 insn.

However, it loses because the first input does not match the output
operand.  This causes spills & reloads and bad code.

There were not any reg-reg copies so regmove didn't do anything with
this code.

regmove has code to try and copy a source operand into a destination
operand then change the source to match the destination.  However, it
only worked if both the source and destination operand were regs, so it
not apply in the case shown above.

The attached change allows regmove to optimize that code so that the
source and destination registers match.  As a result the number of reloads
and spills is reduced resulting in better code.

This would probably help all of our 2 address machines.  Though I have not
done any analysis.

Comments/opinions would be greatly appreciated:

	* regmove.c (copy_src_to_dest): Returns an integer now
	indicating a successful copy was performed.  Allow SRC to
	be any rtx instead of only REGs.
	(regmove_optimize): During backward pass, if a source operand
	is not a register and the destination is a register, try to copy
	the source into the destination to make them match.

	

Index: regmove.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/./gcc/regmove.c,v
retrieving revision 1.46
diff -c -3 -p -r1.46 regmove.c
*** regmove.c	1998/12/16 20:57:50	1.46
--- regmove.c	1998/12/22 20:16:07
*************** static int optimize_reg_copy_1	PROTO((rt
*** 42,48 ****
  static void optimize_reg_copy_2	PROTO((rtx, rtx, rtx));
  static void optimize_reg_copy_3	PROTO((rtx, rtx, rtx));
  static rtx gen_add3_insn	PROTO((rtx, rtx, rtx));
! static void copy_src_to_dest	PROTO((rtx, rtx, rtx, int));
  static int *regmove_bb_head;
  
  struct match {
--- 42,48 ----
  static void optimize_reg_copy_2	PROTO((rtx, rtx, rtx));
  static void optimize_reg_copy_3	PROTO((rtx, rtx, rtx));
  static rtx gen_add3_insn	PROTO((rtx, rtx, rtx));
! static int copy_src_to_dest	PROTO((rtx, rtx, rtx, int));
  static int *regmove_bb_head;
  
  struct match {
*************** optimize_reg_copy_3 (insn, dest, src)
*** 592,598 ****
  /* If we were not able to update the users of src to use dest directly, try
     instead moving the value to dest directly before the operation.  */
  
! static void
  copy_src_to_dest (insn, src, dest, loop_depth)
       rtx insn;
       rtx src;
--- 592,598 ----
  /* If we were not able to update the users of src to use dest directly, try
     instead moving the value to dest directly before the operation.  */
  
! static int
  copy_src_to_dest (insn, src, dest, loop_depth)
       rtx insn;
       rtx src;
*************** copy_src_to_dest (insn, src, dest, loop_
*** 617,625 ****
       parameter when there is no frame pointer that is not allocated a register.
       For now, we just reject them, rather than incrementing the live length.  */
  
!   if (GET_CODE (src) == REG
!       && REG_LIVE_LENGTH (REGNO (src)) > 0
!       && GET_CODE (dest) == REG
        && REG_LIVE_LENGTH (REGNO (dest)) > 0
        && (set = single_set (insn)) != NULL_RTX
        && !reg_mentioned_p (dest, SET_SRC (set))
--- 617,623 ----
       parameter when there is no frame pointer that is not allocated a register.
       For now, we just reject them, rather than incrementing the live length.  */
  
!   if (GET_CODE (dest) == REG
        && REG_LIVE_LENGTH (REGNO (dest)) > 0
        && (set = single_set (insn)) != NULL_RTX
        && !reg_mentioned_p (dest, SET_SRC (set))
*************** copy_src_to_dest (insn, src, dest, loop_
*** 684,702 ****
        if (REGNO_FIRST_UID (dest_regno) == insn_uid)
  	REGNO_FIRST_UID (dest_regno) = move_uid;
  
!       src_regno = REGNO (src);
!       if (! find_reg_note (move_insn, REG_DEAD, src))
! 	REG_LIVE_LENGTH (src_regno)++;
  
!       if (REGNO_FIRST_UID (src_regno) == insn_uid)
! 	REGNO_FIRST_UID (src_regno) = move_uid;
  
!       if (REGNO_LAST_UID (src_regno) == insn_uid)
! 	REGNO_LAST_UID (src_regno) = move_uid;
  
!       if (REGNO_LAST_NOTE_UID (src_regno) == insn_uid)
! 	REGNO_LAST_NOTE_UID (src_regno) = move_uid;
      }
  }
  
  
--- 682,705 ----
        if (REGNO_FIRST_UID (dest_regno) == insn_uid)
  	REGNO_FIRST_UID (dest_regno) = move_uid;
  
!       if (REG_P (src))
! 	{
! 	  src_regno = REGNO (src);
! 	  if (! find_reg_note (move_insn, REG_DEAD, src))
! 	    REG_LIVE_LENGTH (src_regno)++;
  
! 	  if (REGNO_FIRST_UID (src_regno) == insn_uid)
! 	    REGNO_FIRST_UID (src_regno) = move_uid;
  
! 	  if (REGNO_LAST_UID (src_regno) == insn_uid)
! 	    REGNO_LAST_UID (src_regno) = move_uid;
  
! 	  if (REGNO_LAST_NOTE_UID (src_regno) == insn_uid)
! 	    REGNO_LAST_NOTE_UID (src_regno) = move_uid;
! 	}
!       return 1;
      }
+   return 0;
  }
  
  
*************** regmove_optimize (f, nregs, regmove_dump
*** 1141,1153 ****
  	      dst = recog_operand[match_no];
  	      src = recog_operand[op_no];
  
- 	      if (GET_CODE (src) != REG)
- 		continue;
- 
  	      if (GET_CODE (dst) != REG
  		  || REGNO (dst) < FIRST_PSEUDO_REGISTER
  		  || REG_LIVE_LENGTH (REGNO (dst)) < 0)
  		continue;
  
  	      /* If the operands already match, then there is nothing to do.  */
  	      if (operands_match_p (src, dst)
--- 1144,1165 ----
  	      dst = recog_operand[match_no];
  	      src = recog_operand[op_no];
  
  	      if (GET_CODE (dst) != REG
  		  || REGNO (dst) < FIRST_PSEUDO_REGISTER
  		  || REG_LIVE_LENGTH (REGNO (dst)) < 0)
  		continue;
+ 
+ 	      if (GET_CODE (src) != REG)
+ 		{
+ 		  
+ 		  /* SRC may be a MEM or some other operand type.  We win
+ 		     if we can copy it to the destination and replace SRC
+ 		     with DST.  Note that DST is known to be a REG at this
+ 		     point.  If we are successful quit the loop.  */
+ 		  if (copy_src_to_dest (insn, src, dst, loop_depth))
+ 		    break;
+ 		  continue;
+ 		}
  
  	      /* If the operands already match, then there is nothing to do.  */
  	      if (operands_match_p (src, dst)