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]

Re: patch committed: fix sh-elf -m4 struct-by-value-[567] failures


Joern Rennecke wrote:
> 2003-06-02  J"orn Rennecke <joern.rennecke@superh.com>
> 
>         * sh.h (OLD_ARG_MODE): New macro.
>         (FUNCTION_ARG_ADVANCE, FUNCTION_ARG_PASS_BY_REFERENCE): Use it.
>         (FUNCTION_ARG_1): Break out of:
>         (FUNCTION_ARG).  Use OLD_ARG_MODE.

I thought that the accidental ABI change to pass structs with a single member
like the single member (minus type promotions) was recent, but some further
digging showed that it happended in 1997, with some jitters in 2000.  So not
changing this back is likely to cause less problems than changing it back,
and it's a bit more efficient too.
So I went on to look for other reasons for the testsuite failures, and found
problems with the passing of complex floats.  SCmode arguments are split by
the frontend into SFmode values without regard for CANNOT_CHANGE_MODE_CLASS.
So we end up with the wrong ordering on little endian SH4.  The va_arg code
was working the way it was supposed to work, hence the mismatch.
The garbled argument passing couldn't work at all when an odd number of
floating point argument passing registers was currently in use, so I changed
the behaviour to the intended on in that case, but for an even number, it mostly
worked, except for the varargs mismatch, so, to avoid unnecessary ABI breakage
at this point, I've adjusted the varargs code to cope.  This stuff is dependent
on FUNCTION_ARG_SCmode_WART, so when we have another ABI flag day, this can
hopefully go away.
	
-- 
--------------------------
SuperH (UK) Ltd.
2410 Aztec West / Almondsbury / BRISTOL / BS32 4QX
T:+44 1454 465658
2003-06-24  J"orn Rennecke <joern.rennecke@superh.com>

	Back out these patches:
	 2003-06-02  J"orn Rennecke <joern.rennecke@superh.com>
          * sh.h (OLD_ARG_MODE): New macro.
          (FUNCTION_ARG_ADVANCE, FUNCTION_ARG_PASS_BY_REFERENCE): Use it.
          (FUNCTION_ARG_1): Break out of:
          (FUNCTION_ARG).  Use OLD_ARG_MODE.
	 2003-06-06  J"orn Rennecke <joern.rennecke@superh.com>
          * sh.h (FUNCTION_ARG_1): Consistently use NEW_MODE for the mode
          of the generated register.

	* sh.h (FUNCTION_ARG_SCmode_WART): Define.
	(FUNCTION_ARG): Unless FUNCTION_ARG_SCmode_WART is defined and
	an even number of floating point regs are in use, use the same
	sequence of argument passing registers for SCmode as would be 
	used for two SFmode values.
	* sh.c (sh_va_arg): If FUNCTION_ARG_SCmode_WART is defined,
	swap real / imaginary parts in incoming SCmode values passed
	in registers.

Index: config/sh/sh.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/sh/sh.c,v
retrieving revision 1.223
diff -p -r1.223 sh.c
*** config/sh/sh.c	19 Jun 2003 21:47:23 -0000	1.223
--- config/sh/sh.c	24 Jun 2003 17:01:19 -0000
*************** sh_va_arg (valist, type)
*** 5940,5947 ****
    HOST_WIDE_INT size, rsize;
    tree tmp, pptr_type_node;
    rtx addr_rtx, r;
!   rtx result;
    int pass_by_ref = MUST_PASS_IN_STACK (TYPE_MODE (type), type);
  
    size = int_size_in_bytes (type);
    rsize = (size + UNITS_PER_WORD - 1) & -UNITS_PER_WORD;
--- 5940,5948 ----
    HOST_WIDE_INT size, rsize;
    tree tmp, pptr_type_node;
    rtx addr_rtx, r;
!   rtx result_ptr, result = NULL_RTX;
    int pass_by_ref = MUST_PASS_IN_STACK (TYPE_MODE (type), type);
+   rtx lab_over;
  
    size = int_size_in_bytes (type);
    rsize = (size + UNITS_PER_WORD - 1) & -UNITS_PER_WORD;
*************** sh_va_arg (valist, type)
*** 5955,5961 ****
        tree f_next_o, f_next_o_limit, f_next_fp, f_next_fp_limit, f_next_stack;
        tree next_o, next_o_limit, next_fp, next_fp_limit, next_stack;
        int pass_as_float;
!       rtx lab_false, lab_over;
  
        f_next_o = TYPE_FIELDS (va_list_type_node);
        f_next_o_limit = TREE_CHAIN (f_next_o);
--- 5956,5962 ----
        tree f_next_o, f_next_o_limit, f_next_fp, f_next_fp_limit, f_next_stack;
        tree next_o, next_o_limit, next_fp, next_fp_limit, next_stack;
        int pass_as_float;
!       rtx lab_false;
  
        f_next_o = TYPE_FIELDS (va_list_type_node);
        f_next_o_limit = TREE_CHAIN (f_next_o);
*************** sh_va_arg (valist, type)
*** 5973,5978 ****
--- 5974,5989 ----
        next_stack = build (COMPONENT_REF, TREE_TYPE (f_next_stack),
  			  valist, f_next_stack);
  
+       /* Structures with a single member with a distinct mode are passed
+ 	 like their member.  This is relevant if the latter has a REAL_TYPE
+ 	 or COMPLEX_TYPE type.  */
+       if (TREE_CODE (type) == RECORD_TYPE
+ 	  && TYPE_FIELDS (type)
+ 	  && TREE_CODE (TYPE_FIELDS (type)) == FIELD_DECL
+ 	  && (TREE_CODE (TREE_TYPE (TYPE_FIELDS (type))) == REAL_TYPE
+ 	      || TREE_CODE (TREE_TYPE (TYPE_FIELDS (type))) == COMPLEX_TYPE)
+           && TREE_CHAIN (TYPE_FIELDS (type)) == NULL_TREE)
+ 	type = TREE_TYPE (TYPE_FIELDS (type));
        if (TARGET_SH4)
  	{
  	  pass_as_float = ((TREE_CODE (type) == REAL_TYPE && size <= 8)
*************** sh_va_arg (valist, type)
*** 5989,5994 ****
--- 6000,6008 ----
        lab_false = gen_label_rtx ();
        lab_over = gen_label_rtx ();
  
+       tmp = make_tree (pptr_type_node, addr_rtx);
+       valist = build1 (INDIRECT_REF, ptr_type_node, tmp);
+ 
        if (pass_as_float)
  	{
  	  int first_floatreg
*************** sh_va_arg (valist, type)
*** 6018,6023 ****
--- 6032,6068 ----
  	  if (r != addr_rtx)
  	    emit_move_insn (addr_rtx, r);
  
+ #ifdef FUNCTION_ARG_SCmode_WART
+ 	  if (TYPE_MODE (type) == SCmode && TARGET_SH4 && TARGET_LITTLE_ENDIAN)
+ 	    {
+ 	      rtx addr, real, imag, result_value, slot;
+ 	      tree subtype = TREE_TYPE (type);
+ 
+ 	      addr = std_expand_builtin_va_arg (valist, subtype);
+ #ifdef POINTERS_EXTEND_UNSIGNED
+ 	      if (GET_MODE (addr) != Pmode)
+ 		addr = convert_memory_address (Pmode, addr);
+ #endif
+ 	      imag = gen_rtx_MEM (TYPE_MODE (type), addr);
+ 	      set_mem_alias_set (imag, get_varargs_alias_set ());
+ 
+ 	      addr = std_expand_builtin_va_arg (valist, subtype);
+ #ifdef POINTERS_EXTEND_UNSIGNED
+ 	      if (GET_MODE (addr) != Pmode)
+ 		addr = convert_memory_address (Pmode, addr);
+ #endif
+ 	      real = gen_rtx_MEM (TYPE_MODE (type), addr);
+ 	      set_mem_alias_set (real, get_varargs_alias_set ());
+ 
+ 	      result_value = gen_rtx_CONCAT (SCmode, real, imag);
+ 	      /* ??? this interface is stupid - why require a pointer?  */
+ 	      result = gen_reg_rtx (Pmode);
+ 	      slot = assign_stack_temp (SCmode, 8, 0);
+ 	      emit_move_insn (slot, result_value);
+ 	      emit_move_insn (result, XEXP (slot, 0));
+ 	    }
+ #endif /* FUNCTION_ARG_SCmode_WART */
+ 
  	  emit_jump_insn (gen_jump (lab_over));
  	  emit_barrier ();
  	  emit_label (lab_false);
*************** sh_va_arg (valist, type)
*** 6060,6075 ****
  	    emit_move_insn (addr_rtx, r);
  	}
  
!       emit_label (lab_over);
! 
!       tmp = make_tree (pptr_type_node, addr_rtx);
!       valist = build1 (INDIRECT_REF, ptr_type_node, tmp);
      }
  
    /* ??? In va-sh.h, there had been code to make values larger than
       size 8 indirect.  This does not match the FUNCTION_ARG macros.  */
  
!   result = std_expand_builtin_va_arg (valist, type);
    if (pass_by_ref)
      {
  #ifdef POINTERS_EXTEND_UNSIGNED
--- 6105,6126 ----
  	    emit_move_insn (addr_rtx, r);
  	}
  
!       if (! result)
!         emit_label (lab_over);
      }
  
    /* ??? In va-sh.h, there had been code to make values larger than
       size 8 indirect.  This does not match the FUNCTION_ARG macros.  */
  
!   result_ptr = std_expand_builtin_va_arg (valist, type);
!   if (result)
!     {
!       emit_move_insn (result, result_ptr);
!       emit_label (lab_over);
!     }
!   else
!     result = result_ptr;
! 
    if (pass_by_ref)
      {
  #ifdef POINTERS_EXTEND_UNSIGNED
Index: config/sh/sh.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/sh/sh.h,v
retrieving revision 1.214
diff -p -r1.214 sh.h
*** config/sh/sh.h	20 Jun 2003 19:28:13 -0000	1.214
--- config/sh/sh.h	24 Jun 2003 17:01:19 -0000
*************** struct sh_args {
*** 1867,1886 ****
      (CUM).outgoing = 0;						\
    } while (0)
   
- #define OLD_ARG_MODE(MODE, TYPE) \
-   (((TYPE) \
-     && (TREE_CODE (TYPE) == RECORD_TYPE || TREE_CODE (TYPE) == UNION_TYPE) \
-     && (MODE) != BLKmode && GET_MODE_CLASS (MODE) != MODE_INT) \
-    ? int_mode_for_mode (MODE) : (MODE))
- 
  /* Update the data in CUM to advance over an argument
     of mode MODE and data type TYPE.
     (TYPE is null for libcalls where that information may not be
     available.)  */
  
  #define FUNCTION_ARG_ADVANCE(CUM, MODE, TYPE, NAMED)	\
- do {							\
-  enum machine_mode MODE_ = OLD_ARG_MODE ((MODE), (TYPE));\
   if ((CUM).force_mem)					\
     (CUM).force_mem = 0;					\
   else if (TARGET_SH5)					\
--- 1867,1878 ----
*************** do {							\
*** 1888,1904 ****
       tree TYPE_ = ((CUM).byref && (TYPE)		\
  		   ? TREE_TYPE (TYPE)			\
   		   : (TYPE));				\
!      int dwords, numregs;				\
  							\
-      MODE_ = ((CUM).byref && (TYPE)			\
- 	      ? TYPE_MODE (TYPE_) : (MODE_));		\
-      dwords = (((CUM).byref				\
- 		? (CUM).byref				\
- 		: (MODE_) == BLKmode			\
- 		? int_size_in_bytes (TYPE_)		\
- 		: GET_MODE_SIZE (MODE_)) + 7) / 8;	\
-      numregs = MIN (dwords, NPARM_REGS (SImode)		\
- 		    - (CUM).arg_count[(int) SH_ARG_INT]); \
       if (numregs)					\
         {						\
  	 (CUM).arg_count[(int) SH_ARG_INT] += numregs;	\
--- 1880,1896 ----
       tree TYPE_ = ((CUM).byref && (TYPE)		\
  		   ? TREE_TYPE (TYPE)			\
   		   : (TYPE));				\
!      enum machine_mode MODE_ = ((CUM).byref && (TYPE)	\
! 				? TYPE_MODE (TYPE_)	\
! 				: (MODE));		\
!      int dwords = (((CUM).byref				\
! 		    ? (CUM).byref			\
! 		    : (MODE_) == BLKmode		\
! 		    ? int_size_in_bytes (TYPE_)		\
! 		    : GET_MODE_SIZE (MODE_)) + 7) / 8;	\
!      int numregs = MIN (dwords, NPARM_REGS (SImode)	\
! 			- (CUM).arg_count[(int) SH_ARG_INT]); \
  							\
       if (numregs)					\
         {						\
  	 (CUM).arg_count[(int) SH_ARG_INT] += numregs;	\
*************** do {							\
*** 1990,2002 ****
  	   }						\
         }						\
     }							\
!  else if (! TARGET_SH4 || PASS_IN_REG_P ((CUM), (MODE_), (TYPE))) \
!    ((CUM).arg_count[(int) GET_SH_ARG_CLASS (MODE_)]	\
!     = (ROUND_REG ((CUM), (MODE_))			\
!        + ((MODE_) == BLKmode				\
  	  ? ROUND_ADVANCE (int_size_in_bytes (TYPE))	\
! 	  : ROUND_ADVANCE (GET_MODE_SIZE (MODE_)))));	\
! } while (0)
  
  /* Return boolean indicating arg of mode MODE will be passed in a reg.
     This macro is only used in this file.  */
--- 1982,1993 ----
  	   }						\
         }						\
     }							\
!  else if (! TARGET_SH4 || PASS_IN_REG_P ((CUM), (MODE), (TYPE))) \
!    ((CUM).arg_count[(int) GET_SH_ARG_CLASS (MODE)]	\
!     = (ROUND_REG ((CUM), (MODE))			\
!        + ((MODE) == BLKmode				\
  	  ? ROUND_ADVANCE (int_size_in_bytes (TYPE))	\
! 	  : ROUND_ADVANCE (GET_MODE_SIZE (MODE)))))
  
  /* Return boolean indicating arg of mode MODE will be passed in a reg.
     This macro is only used in this file.  */
*************** do {							\
*** 2016,2021 ****
--- 2007,2030 ----
  	     <= NPARM_REGS (MODE))) \
         : ROUND_REG ((CUM), (MODE)) < NPARM_REGS (MODE)))
  
+ /* By accident we got stuck with passing SCmode on SH4 little endian
+    in two registers that are nominally successive - which is different from
+    two single SFmode values, where we take endianness translation into
+    account.  That does not work at all if an odd number of registers is
+    already in use, so that got fixed, but library functions are still more
+    likely to use complex numbers without mixing them with SFmode arguments
+    (which in C would have to be structures), so for the sake of ABI
+    compatibility the way SCmode values are passed when an even number of
+    FP registers is in use remains different from a pair of SFmode values for
+    now.
+    I.e.:
+    foo (double); a: fr5,fr4
+    foo (float a, float b); a: fr5 b: fr4
+    foo (__complex float a); a.real fr4 a.imag: fr5 - for consistency,
+                             this should be the other way round...
+    foo (float a, __complex float b); a: fr5 b.real: fr4 b.imag: fr7  */
+ #define FUNCTION_ARG_SCmode_WART 1
+ 
  /* Define where to put the arguments to a function.
     Value is zero to push the argument on the stack,
     or a hard register in which to store the argument.
*************** do {							\
*** 2035,2063 ****
     its data type forbids.  */
  
  #define FUNCTION_ARG(CUM, MODE, TYPE, NAMED) \
-   FUNCTION_ARG_1 ((CUM), OLD_ARG_MODE ((MODE), (TYPE)), (MODE), (TYPE), (NAMED))
- 
- #define FUNCTION_ARG_1(CUM, MODE, NEW_MODE, TYPE, NAMED) \
    ((! TARGET_SH5 \
      && PASS_IN_REG_P ((CUM), (MODE), (TYPE))				\
      && ((NAMED) || !TARGET_HITACHI))					\
!    ? gen_rtx_REG ((NEW_MODE),						\
! 		  ((BASE_ARG_REG (MODE) + ROUND_REG ((CUM), (MODE))) 	\
! 		   ^ ((MODE) == SFmode && TARGET_SH4			\
! 		      && TARGET_LITTLE_ENDIAN != 0)))			\
     : TARGET_SH5								\
     ? ((MODE) == VOIDmode && TARGET_SHCOMPACT				\
        ? GEN_INT ((CUM).call_cookie)					\
        /* The following test assumes unnamed arguments are promoted to	\
  	 DFmode.  */							\
        : (MODE) == SFmode && (CUM).free_single_fp_reg			\
!       ? SH5_PROTOTYPED_FLOAT_ARG ((CUM), (NEW_MODE), (CUM).free_single_fp_reg) \
        : (GET_SH_ARG_CLASS (MODE) == SH_ARG_FLOAT			\
           && ((NAMED) || ! (CUM).prototype_p)				\
           && (CUM).arg_count[(int) SH_ARG_FLOAT] < NPARM_REGS (SFmode))	\
        ? ((! (CUM).prototype_p && TARGET_SHMEDIA)			\
! 	 ? SH5_PROTOTYPELESS_FLOAT_ARG ((CUM), (NEW_MODE))		\
! 	 : SH5_PROTOTYPED_FLOAT_ARG ((CUM), (NEW_MODE),			\
  				     FIRST_FP_PARM_REG			\
  				     + (CUM).arg_count[(int) SH_ARG_FLOAT])) \
        : ((CUM).arg_count[(int) SH_ARG_INT] < NPARM_REGS (SImode)	\
--- 2044,2087 ----
     its data type forbids.  */
  
  #define FUNCTION_ARG(CUM, MODE, TYPE, NAMED) \
    ((! TARGET_SH5 \
      && PASS_IN_REG_P ((CUM), (MODE), (TYPE))				\
      && ((NAMED) || !TARGET_HITACHI))					\
!    ? (((MODE) == SCmode && TARGET_SH4 && TARGET_LITTLE_ENDIAN		\
!        && (! FUNCTION_ARG_SCmode_WART || (ROUND_REG ((CUM), (MODE)) & 1)))\
!       ? (gen_rtx_PARALLEL						\
! 	 (SCmode,							\
! 	  (gen_rtvec							\
! 	   (2, 								\
! 	    (gen_rtx_EXPR_LIST						\
! 	     (VOIDmode,							\
! 	      gen_rtx_REG (SFmode,					\
! 			   BASE_ARG_REG (MODE)				\
! 			   + ROUND_REG ((CUM), (MODE)) ^ 1),		\
! 	      const0_rtx)),						\
! 	    (gen_rtx_EXPR_LIST						\
! 	     (VOIDmode,							\
! 	      gen_rtx_REG (SFmode,					\
! 			   BASE_ARG_REG (MODE)				\
! 			   + (ROUND_REG ((CUM), (MODE)) + 1) ^ 1),	\
! 	      GEN_INT (4)))))))						\
!       : gen_rtx_REG ((MODE),						\
! 		     ((BASE_ARG_REG (MODE) + ROUND_REG ((CUM), (MODE))) \
! 		      ^ ((MODE) == SFmode && TARGET_SH4			\
! 			 && TARGET_LITTLE_ENDIAN != 0))))		\
     : TARGET_SH5								\
     ? ((MODE) == VOIDmode && TARGET_SHCOMPACT				\
        ? GEN_INT ((CUM).call_cookie)					\
        /* The following test assumes unnamed arguments are promoted to	\
  	 DFmode.  */							\
        : (MODE) == SFmode && (CUM).free_single_fp_reg			\
!       ? SH5_PROTOTYPED_FLOAT_ARG ((CUM), (MODE), (CUM).free_single_fp_reg) \
        : (GET_SH_ARG_CLASS (MODE) == SH_ARG_FLOAT			\
           && ((NAMED) || ! (CUM).prototype_p)				\
           && (CUM).arg_count[(int) SH_ARG_FLOAT] < NPARM_REGS (SFmode))	\
        ? ((! (CUM).prototype_p && TARGET_SHMEDIA)			\
! 	 ? SH5_PROTOTYPELESS_FLOAT_ARG ((CUM), (MODE))			\
! 	 : SH5_PROTOTYPED_FLOAT_ARG ((CUM), (MODE),			\
  				     FIRST_FP_PARM_REG			\
  				     + (CUM).arg_count[(int) SH_ARG_FLOAT])) \
        : ((CUM).arg_count[(int) SH_ARG_INT] < NPARM_REGS (SImode)	\
*************** do {							\
*** 2065,2071 ****
  	     || (! SHCOMPACT_FORCE_ON_STACK ((MODE), (TYPE))		\
  	         && ! SH5_WOULD_BE_PARTIAL_NREGS ((CUM), (MODE),	\
  						  (TYPE), (NAMED)))))	\
!       ? gen_rtx_REG ((NEW_MODE), (FIRST_PARM_REG			\
   			      + (CUM).arg_count[(int) SH_ARG_INT]))	\
        : 0)								\
     : 0)
--- 2089,2095 ----
  	     || (! SHCOMPACT_FORCE_ON_STACK ((MODE), (TYPE))		\
  	         && ! SH5_WOULD_BE_PARTIAL_NREGS ((CUM), (MODE),	\
  						  (TYPE), (NAMED)))))	\
!       ? gen_rtx_REG ((MODE), (FIRST_PARM_REG				\
   			      + (CUM).arg_count[(int) SH_ARG_INT]))	\
        : 0)								\
     : 0)
*************** do {							\
*** 2076,2082 ****
     loads them into the full 64-bits registers.  */
  #define FUNCTION_ARG_PASS_BY_REFERENCE(CUM,MODE,TYPE,NAMED) \
    (MUST_PASS_IN_STACK ((MODE), (TYPE)) \
!    || SHCOMPACT_BYREF ((CUM), OLD_ARG_MODE ((MODE), (TYPE)), (TYPE), (NAMED)))
  
  #define SHCOMPACT_BYREF(CUM, MODE, TYPE, NAMED) \
    ((CUM).byref								\
--- 2100,2106 ----
     loads them into the full 64-bits registers.  */
  #define FUNCTION_ARG_PASS_BY_REFERENCE(CUM,MODE,TYPE,NAMED) \
    (MUST_PASS_IN_STACK ((MODE), (TYPE)) \
!    || SHCOMPACT_BYREF ((CUM), (MODE), (TYPE), (NAMED)))
  
  #define SHCOMPACT_BYREF(CUM, MODE, TYPE, NAMED) \
    ((CUM).byref								\

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