PATCH: MIPS GO_IF_LEGITIMATE_ADDRESS broken

Mark Mitchell mark@codesourcery.com
Sun Apr 16 14:46:00 GMT 2000


This change:

  Sat Mar 25 09:12:10 2000  Richard Kenner  <kenner@vlsi1.ultra.nyu.edu>
	* machmode.h (mode_size, mode_unit_size): Now unsigned.

broke the MIPS back-end.  An expression involving GET_MODE_SIZE became
unsigned, which meant that comparing it to a negative signed quantity
didn't do what it was supposed to.

This patch fixes that problem.  It also makes GO_IF_LEGITIMATE_ADDRESS
a thin wrapper around a function, rather than a gigantic macro, and
fixes a few other signedness issues in the MIPS back-end.

Kenner, please look at all the other ports and see if any of them are
likely to run into similar troubles; it seems to me that code like:

  if (XINTVAL (x) + GET_MODE_SIZE (mode) >= -32768)

(which was approximately what was in the MIPS back-end) might well
appear elsewhere.  In this case, we (luckily) got a compiler abort,
and not just bad code generation, but other folks might have to spend
more than a few hours tracking this down, otherwise.  I notice Joern
already tracked a similar problem in the SH back-end.  Thanks.

This patch gets us past the problem, but I don't know if an entire
MIPS bootstrap will succeed or not; I'm doing that now.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

2000-04-16  Mark Mitchell  <mark@codesourcery.com>

	* config/mips/mips-protos.h (mips_legitimate_address_p): New
	function.
	(mips_reg_mode_ok_for_base_p): Likewise.
	* config/mips/mips.h (REG_OK_STRICT_P): Don't define.
	(REG_OK_FOR_INDEX_P): Define unconditionally.
	(REG_MODE_OK_FOR_BASE_P): Use mips_reg_mode_ok_for_base_p.
	(GO_IF_LEGITIMATE_ADDRESS): Use mips_legitimate_address_p.
	* config/mips/mips.c (mips16_simple_memory_operand): Adjust now
	that GET_MODE_SIZE is unsigned.
	(mips_reg_mode_ok_for_base_p): Define.
	(mips_legitimate_address_p): Likewise.  Adjust now
	that GET_MODE_SIZE is unsigned.
	(block_move_loop): Make the number of bytes unsigned.
	(expand_block_move): Likewise.
	(function_arg): Make the loop counter unsigned to match the
	boundary condition.

Index: config/mips/mips-protos.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/mips/mips-protos.h,v
retrieving revision 1.2
diff -c -p -r1.2 mips-protos.h
*** mips-protos.h	2000/02/26 21:35:46	1.2
--- mips-protos.h	2000/04/16 21:35:57
*************** extern int		se_uns_arith_operand PARAMS 
*** 130,135 ****
--- 130,137 ----
  extern int		se_arith_operand PARAMS ((rtx, enum machine_mode));
  extern int		se_nonmemory_operand PARAMS ((rtx, enum machine_mode));
  extern int		se_nonimmediate_operand PARAMS ((rtx, enum machine_mode));
+ extern int              mips_legitimate_address_p PARAMS ((enum machine_mode, rtx, int));
+ extern int              mips_reg_mode_ok_for_base_p PARAMS ((rtx, enum machine_mode, int));
  extern int              extend_operator PARAMS ((rtx, enum machine_mode));
  extern int              highpart_shift_operator PARAMS ((rtx, enum machine_mode));
  extern int		m16_uimm3_b PARAMS ((rtx, enum machine_mode));
Index: config/mips/mips.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/mips/mips.h,v
retrieving revision 1.95
diff -c -p -r1.95 mips.h
*** mips.h	2000/04/05 15:52:17	1.95
--- mips.h	2000/04/16 21:36:06
*************** typedef struct mips_args {
*** 2725,2745 ****
     need to be strict.  */
  
  #ifndef REG_OK_STRICT
- 
- #define REG_OK_STRICT_P 0
- #define REG_OK_FOR_INDEX_P(X) 0
  #define REG_MODE_OK_FOR_BASE_P(X, MODE) \
!   GP_REG_OR_PSEUDO_NONSTRICT_P (REGNO (X), (MODE))
! 
  #else
- 
- #define REG_OK_STRICT_P 1
- #define REG_OK_FOR_INDEX_P(X) 0
  #define REG_MODE_OK_FOR_BASE_P(X, MODE) \
!   REGNO_MODE_OK_FOR_BASE_P (REGNO (X), (MODE))
! 
  #endif
  
  
  /* Maximum number of registers that can appear in a valid memory address.  */
  
--- 2725,2739 ----
     need to be strict.  */
  
  #ifndef REG_OK_STRICT
  #define REG_MODE_OK_FOR_BASE_P(X, MODE) \
!   mips_reg_mode_ok_for_base_p (X, MODE, 0)
  #else
  #define REG_MODE_OK_FOR_BASE_P(X, MODE) \
!   mips_reg_mode_ok_for_base_p (X, MODE, 1)
  #endif
  
+ #define REG_OK_FOR_INDEX_P(X) 0
+ 
  
  /* Maximum number of registers that can appear in a valid memory address.  */
  
*************** typedef struct mips_args {
*** 2806,2917 ****
  #define GO_DEBUG_RTX(x)
  #endif
  
! #define GO_IF_LEGITIMATE_ADDRESS(MODE, X, ADDR)				\
! {									\
!   register rtx xinsn = (X);						\
! 									\
!   if (TARGET_DEBUG_B_MODE)						\
!     {									\
!       GO_PRINTF2 ("\n========== GO_IF_LEGITIMATE_ADDRESS, %sstrict\n",	\
! 		  (REG_OK_STRICT_P) ? "" : "not ");			\
!       GO_DEBUG_RTX (xinsn);						\
!     }									\
! 									\
!   /* Check for constant before stripping off SUBREG, so that we don't	\
!      accept (subreg (const_int)) which will fail to reload. */   	\
!   if (CONSTANT_ADDRESS_P (xinsn)					\
!       && ! (mips_split_addresses && mips_check_split (xinsn, MODE))	\
!       && (! TARGET_MIPS16 || mips16_constant (xinsn, MODE, 1, 0)))	\
!     goto ADDR;								\
! 									\
!   while (GET_CODE (xinsn) == SUBREG)					\
!     xinsn = SUBREG_REG (xinsn);						\
! 									\
!   /* The mips16 can only use the stack pointer as a base register when	\
!      loading SImode or DImode values.  */				\
!   if (GET_CODE (xinsn) == REG && REG_MODE_OK_FOR_BASE_P (xinsn, MODE))	\
!     goto ADDR;								\
! 									\
!   if (GET_CODE (xinsn) == LO_SUM && mips_split_addresses)		\
!     {									\
!       register rtx xlow0 = XEXP (xinsn, 0);				\
!       register rtx xlow1 = XEXP (xinsn, 1);				\
! 									\
!       while (GET_CODE (xlow0) == SUBREG)				\
! 	xlow0 = SUBREG_REG (xlow0);					\
!       if (GET_CODE (xlow0) == REG					\
! 	  && REG_MODE_OK_FOR_BASE_P (xlow0, MODE)			\
! 	  && mips_check_split (xlow1, MODE))				\
! 	goto ADDR;							\
!     }									\
! 									\
!   if (GET_CODE (xinsn) == PLUS)						\
!     {									\
!       register rtx xplus0 = XEXP (xinsn, 0);				\
!       register rtx xplus1 = XEXP (xinsn, 1);				\
!       register enum rtx_code code0;					\
!       register enum rtx_code code1;					\
! 									\
!       while (GET_CODE (xplus0) == SUBREG)				\
! 	xplus0 = SUBREG_REG (xplus0);					\
!       code0 = GET_CODE (xplus0);					\
! 									\
!       while (GET_CODE (xplus1) == SUBREG)				\
! 	xplus1 = SUBREG_REG (xplus1);					\
!       code1 = GET_CODE (xplus1);					\
! 									\
!       /* The mips16 can only use the stack pointer as a base register	\
!          when loading SImode or DImode values.  */			\
!       if (code0 == REG && REG_MODE_OK_FOR_BASE_P (xplus0, MODE))	\
! 	{								\
! 	  if (code1 == CONST_INT					\
! 	      && INTVAL (xplus1) >= -32768				\
! 	      && INTVAL (xplus1) + GET_MODE_SIZE (MODE) - 1 <= 32767)	\
! 	    goto ADDR;							\
! 									\
! 	  /* On the mips16, we represent GP relative offsets in RTL.	\
!              These are 16 bit signed values, and can serve as register	\
!              offsets.  */						\
! 	  if (TARGET_MIPS16						\
! 	      && mips16_gp_offset_p (xplus1))				\
! 	    goto ADDR;							\
! 									\
! 	  /* For some code sequences, you actually get better code by	\
! 	     pretending that the MIPS supports an address mode of a	\
! 	     constant address + a register, even though the real	\
! 	     machine doesn't support it.  This is because the		\
! 	     assembler can use $r1 to load just the high 16 bits, add	\
! 	     in the register, and fold the low 16 bits into the memory	\
! 	     reference, whereas the compiler generates a 4 instruction	\
! 	     sequence.  On the other hand, CSE is not as effective.	\
! 	     It would be a win to generate the lui directly, but the	\
! 	     MIPS assembler does not have syntax to generate the	\
! 	     appropriate relocation.  */				\
! 									\
! 	  /* Also accept CONST_INT addresses here, so no else.  */	\
! 	  /* Reject combining an embedded PIC text segment reference	\
! 	     with a register.  That requires an additional		\
! 	     instruction.  */						\
!           /* ??? Reject combining an address with a register for the MIPS  \
! 	     64 bit ABI, because the SGI assembler can not handle this.  */ \
! 	  if (!TARGET_DEBUG_A_MODE					\
! 	      && (mips_abi == ABI_32					\
! 		  || mips_abi == ABI_O64				\
! 		  || mips_abi == ABI_EABI)				\
! 	      && CONSTANT_ADDRESS_P (xplus1)				\
! 	      && ! mips_split_addresses					\
! 	      && (!TARGET_EMBEDDED_PIC					\
! 		  || code1 != CONST					\
! 		  || GET_CODE (XEXP (xplus1, 0)) != MINUS)		\
! 	      && !TARGET_MIPS16)					\
! 	    goto ADDR;							\
! 	}								\
!     }									\
! 									\
!   if (TARGET_DEBUG_B_MODE)						\
!     GO_PRINTF ("Not a legitimate address\n");				\
  }
! 
  
  /* A C expression that is 1 if the RTX X is a constant which is a
     valid address.  This is defined to be the same as `CONSTANT_P (X)',
--- 2800,2818 ----
  #define GO_DEBUG_RTX(x)
  #endif
  
! #ifdef REG_OK_STRICT
! #define GO_IF_LEGITIMATE_ADDRESS(MODE, X, ADDR)	\
! {						\
!   if (mips_legitimate_address_p (MODE, X, 1))	\
!     goto ADDR;					\
  }
! #else
! #define GO_IF_LEGITIMATE_ADDRESS(MODE, X, ADDR)	\
! {						\
!   if (mips_legitimate_address_p (MODE, X, 0))	\
!     goto ADDR;					\
! }
! #endif
  
  /* A C expression that is 1 if the RTX X is a constant which is a
     valid address.  This is defined to be the same as `CONSTANT_P (X)',
Index: config/mips/mips.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/mips/mips.c,v
retrieving revision 1.87
diff -c -p -r1.87 mips.c
*** mips.c	2000/04/13 21:44:48	1.87
--- mips.c	2000/04/16 21:36:02
*************** static enum internal_test map_test_to_in
*** 91,98 ****
  static int mips16_simple_memory_operand		PARAMS ((rtx, rtx,
  							enum machine_mode));
  static int m16_check_op				PARAMS ((rtx, int, int, int));
! static void block_move_loop			PARAMS ((rtx, rtx, int, int,
! 							rtx, rtx));
  static void block_move_call			PARAMS ((rtx, rtx, rtx));
  static FILE *mips_make_temp_file		PARAMS ((void));
  static void save_restore_insns			PARAMS ((int, rtx,
--- 91,100 ----
  static int mips16_simple_memory_operand		PARAMS ((rtx, rtx,
  							enum machine_mode));
  static int m16_check_op				PARAMS ((rtx, int, int, int));
! static void block_move_loop			PARAMS ((rtx, rtx,
! 							 unsigned int, 
! 							 int,
! 							 rtx, rtx));
  static void block_move_call			PARAMS ((rtx, rtx, rtx));
  static FILE *mips_make_temp_file		PARAMS ((void));
  static void save_restore_insns			PARAMS ((int, rtx,
*************** mips16_simple_memory_operand (reg, offse
*** 671,677 ****
       rtx offset;
       enum machine_mode mode;
  {
!   int size, off;
  
    if (mode == BLKmode)
      {
--- 673,680 ----
       rtx offset;
       enum machine_mode mode;
  {
!   unsigned int size;
!   int off;
  
    if (mode == BLKmode)
      {
*************** mips_check_split (address, mode)
*** 1208,1213 ****
--- 1211,1346 ----
  
    return 0;
  }
+ 
+ /* This function is used to implement REG_MODE_OK_FOR_BASE_P.  */
+ 
+ int
+ mips_reg_mode_ok_for_base_p (reg, mode, strict)
+      rtx reg;
+      enum machine_mode mode;
+      int strict;
+ {
+   return (strict 
+ 	  ? REGNO_MODE_OK_FOR_BASE_P (REGNO (reg), mode)
+ 	  : GP_REG_OR_PSEUDO_NONSTRICT_P (REGNO (reg), mode));
+ }
+ 
+ /* This function is used to implement GO_IF_LEGITIMATE_ADDRESS.  It
+    returns a nonzero value if XINSN is a legitimate address for a
+    memory operand of the indicated MODE.  STRICT is non-zero if this
+    function is called during reload.  */
+ 
+ int
+ mips_legitimate_address_p (mode, xinsn, strict)
+      enum machine_mode mode;
+      rtx xinsn;
+      int strict;
+ {
+   if (TARGET_DEBUG_B_MODE)						
+     {									
+       GO_PRINTF2 ("\n========== GO_IF_LEGITIMATE_ADDRESS, %sstrict\n",	
+ 		  strict ? "" : "not ");			
+       GO_DEBUG_RTX (xinsn);						
+     }									
+ 									
+   /* Check for constant before stripping off SUBREG, so that we don't	
+      accept (subreg (const_int)) which will fail to reload. */   	
+   if (CONSTANT_ADDRESS_P (xinsn)					
+       && ! (mips_split_addresses && mips_check_split (xinsn, mode))	
+       && (! TARGET_MIPS16 || mips16_constant (xinsn, mode, 1, 0)))	
+     return 1;								
+ 									
+   while (GET_CODE (xinsn) == SUBREG)					
+     xinsn = SUBREG_REG (xinsn);						
+ 									
+   /* The mips16 can only use the stack pointer as a base register when	
+      loading SImode or DImode values.  */				
+   if (GET_CODE (xinsn) == REG 
+       && mips_reg_mode_ok_for_base_p (xinsn, mode, strict))	
+     return 1;								
+ 									
+   if (GET_CODE (xinsn) == LO_SUM && mips_split_addresses)		
+     {									
+       register rtx xlow0 = XEXP (xinsn, 0);				
+       register rtx xlow1 = XEXP (xinsn, 1);				
+ 									
+       while (GET_CODE (xlow0) == SUBREG)				
+ 	xlow0 = SUBREG_REG (xlow0);					
+       if (GET_CODE (xlow0) == REG					
+ 	  && mips_reg_mode_ok_for_base_p (xlow0, mode, strict)
+ 	  && mips_check_split (xlow1, mode))
+ 	return 1;							
+     }									
+ 									
+   if (GET_CODE (xinsn) == PLUS)						
+     {									
+       register rtx xplus0 = XEXP (xinsn, 0);				
+       register rtx xplus1 = XEXP (xinsn, 1);				
+       register enum rtx_code code0;					
+       register enum rtx_code code1;					
+ 									
+       while (GET_CODE (xplus0) == SUBREG)				
+ 	xplus0 = SUBREG_REG (xplus0);					
+       code0 = GET_CODE (xplus0);					
+ 									
+       while (GET_CODE (xplus1) == SUBREG)				
+ 	xplus1 = SUBREG_REG (xplus1);					
+       code1 = GET_CODE (xplus1);					
+ 									
+       /* The mips16 can only use the stack pointer as a base register	
+          when loading SImode or DImode values.  */			
+       if (code0 == REG 
+ 	  && mips_reg_mode_ok_for_base_p (xplus0, mode, strict))	
+ 	{								
+ 	  if (code1 == CONST_INT && SMALL_INT (xplus1))
+ 	    return 1;
+ 									
+ 	  /* On the mips16, we represent GP relative offsets in RTL.	
+              These are 16 bit signed values, and can serve as register	
+              offsets.  */						
+ 	  if (TARGET_MIPS16						
+ 	      && mips16_gp_offset_p (xplus1))				
+ 	    return 1;							
+ 									
+ 	  /* For some code sequences, you actually get better code by	
+ 	     pretending that the MIPS supports an address mode of a	
+ 	     constant address + a register, even though the real	
+ 	     machine doesn't support it.  This is because the		
+ 	     assembler can use $r1 to load just the high 16 bits, add	
+ 	     in the register, and fold the low 16 bits into the memory	
+ 	     reference, whereas the compiler generates a 4 instruction	
+ 	     sequence.  On the other hand, CSE is not as effective.	
+ 	     It would be a win to generate the lui directly, but the	
+ 	     MIPS assembler does not have syntax to generate the	
+ 	     appropriate relocation.  */				
+ 									
+ 	  /* Also accept CONST_INT addresses here, so no else.  */	
+ 	  /* Reject combining an embedded PIC text segment reference	
+ 	     with a register.  That requires an additional		
+ 	     instruction.  */						
+           /* ??? Reject combining an address with a register for the MIPS  
+ 	     64 bit ABI, because the SGI assembler can not handle this.  */ 
+ 	  if (!TARGET_DEBUG_A_MODE					
+ 	      && (mips_abi == ABI_32					
+ 		  || mips_abi == ABI_O64				
+ 		  || mips_abi == ABI_EABI)				
+ 	      && CONSTANT_ADDRESS_P (xplus1)				
+ 	      && ! mips_split_addresses					
+ 	      && (!TARGET_EMBEDDED_PIC					
+ 		  || code1 != CONST					
+ 		  || GET_CODE (XEXP (xplus1, 0)) != MINUS)		
+ 	      && !TARGET_MIPS16)					
+ 	    return 1;							
+ 	}								
+     }									
+ 									
+   if (TARGET_DEBUG_B_MODE)						
+     GO_PRINTF ("Not a legitimate address\n");
+   
+   /* The address was not legitimate.  */
+   return 0;
+ }
+ 
  
  /* We need a lot of little routines to check constant values on the
     mips16.  These are used to figure out how long the instruction will
*************** static void
*** 3033,3039 ****
  block_move_loop (dest_reg, src_reg, bytes, align, orig_dest, orig_src)
       rtx dest_reg;		/* register holding destination address */
       rtx src_reg;		/* register holding source address */
!      int bytes;			/* # bytes to move */
       int align;			/* alignment */
       rtx orig_dest;		/* original dest for change_address */
       rtx orig_src;		/* original source for making a reg note */
--- 3166,3172 ----
  block_move_loop (dest_reg, src_reg, bytes, align, orig_dest, orig_src)
       rtx dest_reg;		/* register holding destination address */
       rtx src_reg;		/* register holding source address */
!      unsigned int bytes;	/* # bytes to move */
       int align;			/* alignment */
       rtx orig_dest;		/* original dest for change_address */
       rtx orig_src;		/* original source for making a reg note */
*************** expand_block_move (operands)
*** 3145,3158 ****
    rtx bytes_rtx	= operands[2];
    rtx align_rtx = operands[3];
    int constp = GET_CODE (bytes_rtx) == CONST_INT;
!   HOST_WIDE_INT bytes = constp ? INTVAL (bytes_rtx) : 0;
!   int align = INTVAL (align_rtx);
    rtx orig_src	= operands[1];
    rtx orig_dest	= operands[0];
    rtx src_reg;
    rtx dest_reg;
  
!   if (constp && bytes <= 0)
      return;
  
    if (align > UNITS_PER_WORD)
--- 3278,3291 ----
    rtx bytes_rtx	= operands[2];
    rtx align_rtx = operands[3];
    int constp = GET_CODE (bytes_rtx) == CONST_INT;
!   unsigned HOST_WIDE_INT bytes = constp ? INTVAL (bytes_rtx) : 0;
!   unsigned int align = INTVAL (align_rtx);
    rtx orig_src	= operands[1];
    rtx orig_dest	= operands[0];
    rtx src_reg;
    rtx dest_reg;
  
!   if (constp && bytes == 0)
      return;
  
    if (align > UNITS_PER_WORD)
*************** function_arg (cum, mode, type, named)
*** 3863,3869 ****
  	      unsigned int chunks;
  	      HOST_WIDE_INT bitpos;
  	      unsigned int regno;
! 	      int i;
  
  	      /* ??? If this is a packed structure, then the last hunk won't
  		 be 64 bits.  */
--- 3996,4002 ----
  	      unsigned int chunks;
  	      HOST_WIDE_INT bitpos;
  	      unsigned int regno;
! 	      unsigned int i;
  
  	      /* ??? If this is a packed structure, then the last hunk won't
  		 be 64 bits.  */


More information about the Gcc-patches mailing list