[PATCH/RFA] Update m68k function return value handling

[PATCH/RFA] Update m68k function return value handling

[Mark Kettenis]

Hi Andreas,

Here's a patch that converts the m68k to the new function return value
handling mechanism.  I tried very hard to make it possible to deal
with all the different m68k ABI's.  I'm pretty sure I've got it right
for at least OpenBSD, NetBSD and Linux.  On other systems it can't
really be worse than the old code.  There is now a rather big comment
explaining the conventions that have been used in the past.  I've
tested this on m68k-unknown-netbsdelf1.6.2, which uses the official
SVR4 convention, and all return values tests in the testsuite pass
there now.  I'm currently testing m68k-unknown-openbsd3.4.
Unfortunately I can't test m68k-unknown-linux, since I don't have any
disk space left on my Mac :-(.  But I'm pretty confident this will
work for Linux too.

Anyway, are the Linux changes OK with you?

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* m68k-tdep.h (struct gdbarch_tdep): Add member
	struct_value_regnum.
	(m68k_svr4_init_abi): New prototype.
	* m68k-tdep.c: Add comment about all the different calling
	conventions.
	(m68k_extract_return_value): Remove code dealing with single-field
	structs.
	(m68k_store_return_value): Remove code dealing with single-field
	structs.  Correctly store return values of 5, 6, 7 or 8 bytes.
	(m68k_extract_struct_value_address): Remove function.
	(m68k_svr4_extract_return_value,m68k_svr4_store_return_value)
	(m68k_reg_struct_return_p, m68k_return_value)
	(m68k_svr4_return_value): New functions.
	(m68k_use_struct_convention): Remove function.
	(m68k_push_dummy_call): Use new struct_value_regnum member of
	`struct gdbarch_tdep' instead of hardcoded register number to
	store STRUCT_ADDR.
	(m68k_svr4_init_abi): New function.
	(m68k_gdbarch_init): Don't set extract_return_value,
	store_return_values, deprecated_extract_struct_value_address and
	use_struct_convention.  Set return_value instead.  Initialize new
	struct_value_regnum member of `struct gdbarch_tdep'.
	* m68klinux-tdep.c: Update copyright year.
	(m68k_linux_extract_return_value, m68k_linux_store_return_value)
	(m68k_linux_extract_struct_value_address): Remove function.
	(m68k_linux_init_abi): Don't set extract_return_value,
	store_return_values, deprecated_extract_struct_value_address and
	use_struct_convention.  Call m68k_svr4_init_abi but override the
	new struct_value_regnum member of `struct gdbarch_tdep'.

Index: m68k-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m68k-tdep.c,v
retrieving revision 1.84
diff -u -p -r1.84 m68k-tdep.c
--- m68k-tdep.c 1 May 2004 15:10:15 -0000 1.84
+++ m68k-tdep.c 2 May 2004 21:41:48 -0000
@@ -25,6 +25,7 @@
 #include "frame.h"
 #include "frame-base.h"
 #include "frame-unwind.h"
+#include "floatformat.h"
 #include "symtab.h"
 #include "gdbcore.h"
 #include "value.h"
@@ -129,8 +130,36 @@ m68k_register_name (int regnum)
     return register_names[regnum];
 }
 \f
-/* Extract from an array REGBUF containing the (raw) register state, a
-   function return value of TYPE, and copy that, in virtual format,
+/* There is a fair number of calling conventions that are in somewhat
+   wide use.  The 68000/08/10 don't support an FPU, not even as a
+   coprocessor.  All function return values are stored in %d0/%d1.
+   Structures are returned in a static buffer, a pointer to which is
+   returned in %d0.  This means that functions returning a structure
+   are not re-entrant.  To avoid this problem some systems use a
+   convention where the caller passes a pointer to a buffer in %a1
+   where the return values is to be stored.  This convention is the
+   default, and is implemented in the function m68k_return_value.
+
+   The 68020/030/040/060 do support an FPU, either as a coprocessor
+   (68881/2) or built-in (68040/68060).  That's why System V release 4
+   (SVR4) instroduces a new calling convention specified by the SVR4
+   psABI.  Integer values are returned in %d0/%d1, pointer return
+   values in %a0 and floating values in %fp0.  When calling functions
+   returning a structure the caller should pass a pointer to a buffer
+   for the return value in %a0.  This convention is implemented in the
+   function m68k_svr4_return_value, and by appropriately setting the
+   struct_value_regnum member of `struct gdbarch_tdep'.
+
+   GNU/Linux returns values in the same way as SVR4 does, but uses %a1
+   for passing the structure return value buffer.
+
+   GCC can also generate code where small structures are returned in
+   %d0/%d1 instead of in memory by using -freg-struct-return.  This is
+   the default on NetBSD a.out, OpenBSD and GNU/Linux and several
+   embedded systems.  This convention is implemented by setting the
+   struct_return member of `struct gdbarch_tdep' to reg_struct_return.  */
+
+/* Read a function return value of TYPE from REGCACHE, and copy that
    into VALBUF.  */
 
 static void
@@ -140,13 +169,6 @@ m68k_extract_return_value (struct type *
   int len = TYPE_LENGTH (type);
   char buf[M68K_MAX_REGISTER_SIZE];
 
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
-      && TYPE_NFIELDS (type) == 1)
-    {
-      m68k_extract_return_value (TYPE_FIELD_TYPE (type, 0), regcache, valbuf);
-      return;
-    }
-
   if (len <= 4)
     {
       regcache_raw_read (regcache, M68K_D0_REGNUM, buf);
@@ -164,29 +186,39 @@ m68k_extract_return_value (struct type *
 		    "Cannot extract return value of %d bytes long.", len);
 }
 
-/* Write into the appropriate registers a function return value stored
-   in VALBUF of type TYPE, given in virtual format.  */
-
 static void
-m68k_store_return_value (struct type *type, struct regcache *regcache,
-			 const void *valbuf)
+m68k_svr4_extract_return_value (struct type *type, struct regcache *regcache,
+				void *valbuf)
 {
   int len = TYPE_LENGTH (type);
+  char buf[M68K_MAX_REGISTER_SIZE];
 
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
-      && TYPE_NFIELDS (type) == 1)
+  if (TYPE_CODE (type) == TYPE_CODE_FLT)
     {
-      m68k_store_return_value (TYPE_FIELD_TYPE (type, 0), regcache, valbuf);
-      return;
+      regcache_raw_read (regcache, M68K_FP0_REGNUM, buf);
+      convert_typed_floating (buf, builtin_type_m68881_ext, valbuf, type);
     }
+  else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
+    regcache_raw_read (regcache, M68K_A0_REGNUM, valbuf);
+  else
+    m68k_extract_return_value (type, regcache, valbuf);
+}
+
+/* Write a function return value of TYPE from VALBUF into REGCACHE.  */
+
+static void
+m68k_store_return_value (struct type *type, struct regcache *regcache,
+			 const void *valbuf)
+{
+  int len = TYPE_LENGTH (type);
 
   if (len <= 4)
     regcache_raw_write_part (regcache, M68K_D0_REGNUM, 4 - len, len, valbuf);
   else if (len <= 8)
     {
-      regcache_raw_write_part (regcache, M68K_D1_REGNUM, 8 - len,
+      regcache_raw_write_part (regcache, M68K_D0_REGNUM, 8 - len,
 			       len - 4, valbuf);
-      regcache_raw_write (regcache, M68K_D0_REGNUM,
+      regcache_raw_write (regcache, M68K_D1_REGNUM,
 			  (char *) valbuf + (len - 4));
     }
   else
@@ -194,29 +226,108 @@ m68k_store_return_value (struct type *ty
 		    "Cannot store return value of %d bytes long.", len);
 }
 
-/* Extract from REGCACHE, which contains the (raw) register state, the
-   address in which a function should return its structure value, as a
-   CORE_ADDR.  */
-
-static CORE_ADDR
-m68k_extract_struct_value_address (struct regcache *regcache)
+static void
+m68k_svr4_store_return_value (struct type *type, struct regcache *regcache,
+			      const void *valbuf)
 {
-  char buf[4];
+  int len = TYPE_LENGTH (type);
 
-  regcache_cooked_read (regcache, M68K_D0_REGNUM, buf);
-  return extract_unsigned_integer (buf, 4);
+  if (TYPE_CODE (type) == TYPE_CODE_FLT)
+    {
+      char buf[M68K_MAX_REGISTER_SIZE];
+      convert_typed_floating (valbuf, type, buf, builtin_type_m68881_ext);
+      regcache_raw_write (regcache, M68K_FP0_REGNUM, buf);
+    }
+  else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
+    {
+      regcache_raw_write (regcache, M68K_A0_REGNUM, valbuf);
+      regcache_raw_write (regcache, M68K_D0_REGNUM, valbuf);
+    }
+  else
+    m68k_store_return_value (type, regcache, valbuf);
 }
 
+/* Return non-zero if TYPE, which is assumed to be a structure or
+   union type, should be returned in registers for architecture
+   GDBARCH.  */
+
 static int
-m68k_use_struct_convention (int gcc_p, struct type *type)
+m68k_reg_struct_return_p (struct gdbarch *gdbarch, struct type *type)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  enum type_code code = TYPE_CODE (type);
+  int len = TYPE_LENGTH (type);
+
+  gdb_assert (code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION);
+
+  if (tdep->struct_return == pcc_struct_return)
+    return 0;
+
+  return (len == 1 || len == 2 || len == 4 || len == 8);
+}
+
+/* Determine, for architecture GDBARCH, how a return value of TYPE
+   should be returned.  If it is supposed to be returned in registers,
+   and READBUF is non-zero, read the appropriate value from REGCACHE,
+   and copy it into READBUF.  If WRITEBUF is non-zero, write the value
+   from WRITEBUF into REGCACHE.  */
+
+static enum return_value_convention
+m68k_return_value (struct gdbarch *gdbarch, struct type *type,
+		   struct regcache *regcache, void *readbuf,
+		   const void *writebuf)
 {
-  enum struct_return struct_return;
+  enum type_code code = TYPE_CODE (type);
 
-  struct_return = gdbarch_tdep (current_gdbarch)->struct_return;
-  return generic_use_struct_convention (struct_return == reg_struct_return,
-					type);
+  if ((code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION)
+      && !m68k_reg_struct_return_p (gdbarch, type))
+    return RETURN_VALUE_STRUCT_CONVENTION;
+
+  /* GCC returns a `long double' in memory.  */
+  if (code == TYPE_CODE_FLT && TYPE_LENGTH (type) == 12)
+    return RETURN_VALUE_STRUCT_CONVENTION;
+
+  if (readbuf)
+    m68k_extract_return_value (type, regcache, readbuf);
+  if (writebuf)
+    m68k_store_return_value (type, regcache, writebuf);
+
+  return RETURN_VALUE_REGISTER_CONVENTION;
 }
 
+static enum return_value_convention
+m68k_svr4_return_value (struct gdbarch *gdbarch, struct type *type,
+			struct regcache *regcache, void *readbuf,
+			const void *writebuf)
+{
+  enum type_code code = TYPE_CODE (type);
+
+  if ((code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION)
+      && !m68k_reg_struct_return_p (gdbarch, type))
+    return RETURN_VALUE_STRUCT_CONVENTION;
+
+  /* This special case is for structures consisting of a single
+     `float' or `double' member.  These structures are returned in
+     %fp0.  For these structures, we call ourselves recursively,
+     changing TYPE into the type of the first member of the structure.
+     Since that should work for all structures that have only one
+     member, we don't bother to check the member's type here.  */
+  if (code == TYPE_CODE_STRUCT && TYPE_NFIELDS (type) == 1)
+    {
+      type = check_typedef (TYPE_FIELD_TYPE (type, 0));
+      return m68k_svr4_return_value (gdbarch, type, regcache,
+				     readbuf, writebuf);
+    }
+
+  if (readbuf)
+    m68k_svr4_extract_return_value (type, regcache, readbuf);
+  if (writebuf)
+    m68k_svr4_store_return_value (type, regcache, writebuf);
+
+  return RETURN_VALUE_REGISTER_CONVENTION;
+}
+\f
+
 /* A function that tells us whether the function invocation represented
    by fi does not have a frame on the stack associated with it.  If it
    does not, FRAMELESS is set to 1, else 0.  */
@@ -293,6 +404,7 @@ m68k_push_dummy_call (struct gdbarch *gd
 		      struct value **args, CORE_ADDR sp, int struct_return,
 		      CORE_ADDR struct_addr)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   char buf[4];
   int i;
 
@@ -321,7 +433,7 @@ m68k_push_dummy_call (struct gdbarch *gd
   if (struct_return)
     {
       store_unsigned_integer (buf, 4, struct_addr);
-      regcache_cooked_write (regcache, M68K_A1_REGNUM, buf);
+      regcache_cooked_write (regcache, tdep->struct_value_regnum, buf);
     }
 
   /* Store return address.  */
@@ -952,6 +1064,22 @@ m68k_get_longjmp_target (CORE_ADDR *pc)
   *pc = extract_unsigned_integer (buf, TARGET_PTR_BIT / TARGET_CHAR_BIT);
   return 1;
 }
+\f
+
+/* System V Release 4 (SVR4).  */
+
+void
+m68k_svr4_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  /* SVR4 uses a different calling convention.  */
+  set_gdbarch_return_value (gdbarch, m68k_svr4_return_value);
+
+  /* SVR4 uses %a0 instead of %a1.  */
+  tdep->struct_value_regnum = M68K_A0_REGNUM;
+}
+\f
 
 /* Function: m68k_gdbarch_init
    Initializer function for the m68k gdbarch vector.
@@ -984,11 +1112,6 @@ m68k_gdbarch_init (struct gdbarch_info i
   set_gdbarch_believe_pcc_promotion (gdbarch, 1);
   set_gdbarch_decr_pc_after_break (gdbarch, 2);
 
-  set_gdbarch_extract_return_value (gdbarch, m68k_extract_return_value);
-  set_gdbarch_store_return_value (gdbarch, m68k_store_return_value);
-  set_gdbarch_deprecated_extract_struct_value_address (gdbarch, m68k_extract_struct_value_address);
-  set_gdbarch_use_struct_convention (gdbarch, m68k_use_struct_convention);
-
   set_gdbarch_deprecated_frameless_function_invocation (gdbarch, m68k_frameless_function_invocation);
   set_gdbarch_frame_args_skip (gdbarch, 8);
 
@@ -1002,6 +1125,7 @@ m68k_gdbarch_init (struct gdbarch_info i
   set_gdbarch_fp0_regnum (gdbarch, M68K_FP0_REGNUM);
 
   set_gdbarch_push_dummy_call (gdbarch, m68k_push_dummy_call);
+  set_gdbarch_return_value (gdbarch, m68k_return_value);
 
   /* Disassembler.  */
   set_gdbarch_print_insn (gdbarch, print_insn_m68k);
@@ -1012,6 +1136,7 @@ m68k_gdbarch_init (struct gdbarch_info i
 #else
   tdep->jb_pc = -1;
 #endif
+  tdep->struct_value_regnum = M68K_A1_REGNUM;
   tdep->struct_return = pcc_struct_return;
 
   /* Frame unwinder.  */
Index: m68k-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/m68k-tdep.h,v
retrieving revision 1.7
diff -u -p -r1.7 m68k-tdep.h
--- m68k-tdep.h 1 May 2004 15:10:15 -0000 1.7
+++ m68k-tdep.h 2 May 2004 21:41:48 -0000
@@ -67,9 +67,17 @@ struct gdbarch_tdep
   /* The size of each entry in the jump buffer.  */
   size_t jb_elt_size;
 
+  /* Register in which the address to store a structure value is
+     passed to a function.  */
+  int struct_value_regnum;
+
   /* Convention for returning structures.  */
   enum struct_return struct_return;
 };
+
+/* Initialize a SVR4 architecture variant.  */
+extern void m68k_svr4_init_abi (struct gdbarch_info, struct gdbarch *);
+\f
 
 /* Functions exported from m68kbsd-tdep.c.  */
 
Index: m68klinux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m68klinux-tdep.c,v
retrieving revision 1.11
diff -u -p -r1.11 m68klinux-tdep.c
--- m68klinux-tdep.c 1 May 2004 15:10:15 -0000 1.11
+++ m68klinux-tdep.c 2 May 2004 21:41:48 -0000
@@ -1,7 +1,7 @@
 /* Motorola m68k target-dependent support for GNU/Linux.
 
-   Copyright 1996, 1998, 2000, 2001, 2002, 2003 Free Software Foundation,
-   Inc.
+   Copyright 1996, 1998, 2000, 2001, 2002, 2003, 2004
+   Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -275,108 +275,6 @@ m68k_linux_sigtramp_frame_sniffer (struc
   return NULL;
 }
 
-/* Extract from an array REGBUF containing the (raw) register state, a
-   function return value of TYPE, and copy that, in virtual format,
-   into VALBUF.  */
-
-static void
-m68k_linux_extract_return_value (struct type *type, struct regcache *regcache,
-				 void *valbuf)
-{
-  int len = TYPE_LENGTH (type);
-  char buf[M68K_MAX_REGISTER_SIZE];
-
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
-      && TYPE_NFIELDS (type) == 1)
-    {
-      m68k_linux_extract_return_value (TYPE_FIELD_TYPE (type, 0), regcache,
-				       valbuf);
-      return;
-    }
-
-  if (TYPE_CODE (type) == TYPE_CODE_FLT)
-    {
-      regcache_raw_read (regcache, M68K_FP0_REGNUM, buf);
-      convert_typed_floating (buf, builtin_type_m68881_ext, valbuf, type);
-    }
-  else if (TYPE_CODE (type) == TYPE_CODE_PTR)
-    regcache_raw_read (regcache, M68K_A0_REGNUM, valbuf);
-  else
-    {
-      if (len <= 4)
-	{
-	  regcache_raw_read (regcache, M68K_D0_REGNUM, buf);
-	  memcpy (valbuf, buf + (4 - len), len);
-	}
-      else if (len <= 8)
-	{
-	  regcache_raw_read (regcache, M68K_D0_REGNUM, buf);
-	  memcpy (valbuf, buf + (8 - len), len - 4);
-	  regcache_raw_read (regcache, M68K_D1_REGNUM,
-			     (char *) valbuf + (len - 4));
-	}
-      else
-	internal_error (__FILE__, __LINE__,
-			"Cannot extract return value of %d bytes long.", len);
-    }
-}
-
-/* Write into the appropriate registers a function return value stored
-   in VALBUF of type TYPE, given in virtual format.  */
-
-static void
-m68k_linux_store_return_value (struct type *type, struct regcache *regcache,
-			       const void *valbuf)
-{
-  int len = TYPE_LENGTH (type);
-
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
-      && TYPE_NFIELDS (type) == 1)
-    {
-      m68k_linux_store_return_value (TYPE_FIELD_TYPE (type, 0), regcache,
-				     valbuf);
-      return;
-    }
-
-  if (TYPE_CODE (type) == TYPE_CODE_FLT)
-    {
-      char buf[M68K_MAX_REGISTER_SIZE];
-      convert_typed_floating (valbuf, type, buf, builtin_type_m68881_ext);
-      regcache_raw_write (regcache, M68K_FP0_REGNUM, buf);
-    }
-  else if (TYPE_CODE (type) == TYPE_CODE_PTR)
-    regcache_raw_write (regcache, M68K_A0_REGNUM, valbuf);
-  else
-    {
-      if (len <= 4)
-	regcache_raw_write_part (regcache, M68K_D0_REGNUM,
-				 4 - len, len, valbuf);
-      else if (len <= 8)
-	{
-	  regcache_raw_write_part (regcache, M68K_D1_REGNUM, 8 - len,
-				   len - 4, valbuf);
-	  regcache_raw_write (regcache, M68K_D0_REGNUM,
-			      (char *) valbuf + (len - 4));
-	}
-      else
-	internal_error (__FILE__, __LINE__,
-			"Cannot store return value of %d bytes long.", len);
-    }
-}
-
-/* Extract from an array REGBUF containing the (raw) register state
-   the address in which a function should return its structure value,
-   as a CORE_ADDR.  */
-
-static CORE_ADDR
-m68k_linux_extract_struct_value_address (struct regcache *regcache)
-{
-  char buf[4];
-
-  regcache_cooked_read (regcache, M68K_A0_REGNUM, buf);
-  return extract_unsigned_integer (buf, 4);
-}
-
 static void
 m68k_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -384,11 +282,15 @@ m68k_linux_init_abi (struct gdbarch_info
 
   tdep->jb_pc = M68K_LINUX_JB_PC;
   tdep->jb_elt_size = M68K_LINUX_JB_ELEMENT_SIZE;
-  tdep->struct_return = reg_struct_return;
 
-  set_gdbarch_extract_return_value (gdbarch, m68k_linux_extract_return_value);
-  set_gdbarch_store_return_value (gdbarch, m68k_linux_store_return_value);
-  set_gdbarch_deprecated_extract_struct_value_address (gdbarch, m68k_linux_extract_struct_value_address);
+  /* GNU/Linux uses a calling convention that's similar to SVR4.  It
+     returns integer values in %d0/%di, pointer values in %a0 and
+     floating values in %fp0, just like SVR4, but uses %a1 to pass the
+     address to store a structure value.  It also returns small
+     structures in registers instead of memory.  */
+  m68k_svr4_init_abi (info, gdbarch);
+  tdep->struct_value_regnum = M68K_A1_REGNUM;
+  tdep->struct_return = reg_struct_return;
 
   frame_unwind_append_sniffer (gdbarch, m68k_linux_sigtramp_frame_sniffer);
 

Re: [PATCH/RFA] Update m68k function return value handling

[Andreas Schwab]

Mark Kettenis <kettenis@chello.nl> writes:

> Anyway, are the Linux changes OK with you?

Thanks, they look OK.  I have sucessfully built it natively on m68k-linux
and ran a few tests, no failures.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH/RFA] Update m68k function return value handling

[Mark Kettenis]

   From: Andreas Schwab <schwab@suse.de>
   Date: Mon, 03 May 2004 23:21:04 +0200

   Mark Kettenis <kettenis@chello.nl> writes:

   > Anyway, are the Linux changes OK with you?

   Thanks, they look OK.  I have sucessfully built it natively on m68k-linux
   and ran a few tests, no failures.

Thanks for testing it on m68k-linux.  I noticed that I forgot about
including "floatformat.h" so I didn't update Makefile.in.  Fixed with
the attached patch which is what I actually checked in.

Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* m68k-tdep.h (struct gdbarch_tdep): Add member
	struct_value_regnum.
	(m68k_svr4_init_abi): New prototype.
	* m68k-tdep.c: Include "floatformat.h".  Add comment about all the
	different calling conventions.
	(m68k_extract_return_value): Remove code dealing with single-field
	structs.
	(m68k_store_return_value): Remove code dealing with single-field
	structs.  Correctly store return values of 5, 6, 7 or 8 bytes.
	(m68k_extract_struct_value_address): Remove function.
	(m68k_svr4_extract_return_value,m68k_svr4_store_return_value)
	(m68k_reg_struct_return_p, m68k_return_value)
	(m68k_svr4_return_value): New functions.
	(m68k_use_struct_convention): Remove function.
	(m68k_push_dummy_call): Use new struct_value_regnum member of
	`struct gdbarch_tdep' instead of hardcoded register number to
	store STRUCT_ADDR.
	(m68k_svr4_init_abi): New function.
	(m68k_gdbarch_init): Don't set extract_return_value,
	store_return_values, deprecated_extract_struct_value_address and
	use_struct_convention.  Set return_value instead.  Initialize new
	struct_value_regnum member of `struct gdbarch_tdep'.
	* m68klinux-tdep.c: Update copyright year.
	(m68k_linux_extract_return_value, m68k_linux_store_return_value)
	(m68k_linux_extract_struct_value_address): Remove function.
	(m68k_linux_init_abi): Don't set extract_return_value,
	store_return_values, deprecated_extract_struct_value_address and
	use_struct_convention.  Call m68k_svr4_init_abi but override the
	new struct_value_regnum member of `struct gdbarch_tdep'.
	* Makefile.in (m68k-tdep.o): Update dependencies.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.556
diff -u -p -r1.556 Makefile.in
--- Makefile.in 2 May 2004 10:14:01 -0000 1.556
+++ Makefile.in 3 May 2004 21:54:24 -0000
@@ -2032,10 +2032,10 @@ m68knbsd-nat.o: m68knbsd-nat.c $(defs_h)
 m68knbsd-tdep.o: m68knbsd-tdep.c $(defs_h) $(gdbtypes_h) $(regcache_h)
 m68k-stub.o: m68k-stub.c
 m68k-tdep.o: m68k-tdep.c $(defs_h) $(dwarf2_frame_h) $(frame_h) \
-	$(frame_base_h) $(frame_unwind_h) $(symtab_h) $(gdbcore_h) \
-	$(value_h) $(gdb_string_h) $(gdb_assert_h) $(inferior_h) \
-	$(regcache_h) $(arch_utils_h) $(osabi_h) $(dis_asm_h) $(m68k_tdep_h) \
-	$(gregset_h)
+	$(frame_base_h) $(frame_unwind_h) $(floatformat_h) $(symtab_h)\
+	$(gdbcore_h) $(value_h) $(gdb_string_h) $(gdb_assert_h)	\
+	$(inferior_h) $(regcache_h) $(arch_utils_h) $(osabi_h) $(dis_asm_h) \
+	$(m68k_tdep_h) $(gregset_h)
 macrocmd.o: macrocmd.c $(defs_h) $(macrotab_h) $(macroexp_h) $(macroscope_h) \
 	$(command_h) $(gdbcmd_h)
 macroexp.o: macroexp.c $(defs_h) $(gdb_obstack_h) $(bcache_h) $(macrotab_h) \
Index: m68k-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m68k-tdep.c,v
retrieving revision 1.84
diff -u -p -r1.84 m68k-tdep.c
--- m68k-tdep.c 1 May 2004 15:10:15 -0000 1.84
+++ m68k-tdep.c 3 May 2004 21:54:24 -0000
@@ -25,6 +25,7 @@
 #include "frame.h"
 #include "frame-base.h"
 #include "frame-unwind.h"
+#include "floatformat.h"
 #include "symtab.h"
 #include "gdbcore.h"
 #include "value.h"
@@ -129,8 +130,36 @@ m68k_register_name (int regnum)
     return register_names[regnum];
 }
 \f
-/* Extract from an array REGBUF containing the (raw) register state, a
-   function return value of TYPE, and copy that, in virtual format,
+/* There is a fair number of calling conventions that are in somewhat
+   wide use.  The 68000/08/10 don't support an FPU, not even as a
+   coprocessor.  All function return values are stored in %d0/%d1.
+   Structures are returned in a static buffer, a pointer to which is
+   returned in %d0.  This means that functions returning a structure
+   are not re-entrant.  To avoid this problem some systems use a
+   convention where the caller passes a pointer to a buffer in %a1
+   where the return values is to be stored.  This convention is the
+   default, and is implemented in the function m68k_return_value.
+
+   The 68020/030/040/060 do support an FPU, either as a coprocessor
+   (68881/2) or built-in (68040/68060).  That's why System V release 4
+   (SVR4) instroduces a new calling convention specified by the SVR4
+   psABI.  Integer values are returned in %d0/%d1, pointer return
+   values in %a0 and floating values in %fp0.  When calling functions
+   returning a structure the caller should pass a pointer to a buffer
+   for the return value in %a0.  This convention is implemented in the
+   function m68k_svr4_return_value, and by appropriately setting the
+   struct_value_regnum member of `struct gdbarch_tdep'.
+
+   GNU/Linux returns values in the same way as SVR4 does, but uses %a1
+   for passing the structure return value buffer.
+
+   GCC can also generate code where small structures are returned in
+   %d0/%d1 instead of in memory by using -freg-struct-return.  This is
+   the default on NetBSD a.out, OpenBSD and GNU/Linux and several
+   embedded systems.  This convention is implemented by setting the
+   struct_return member of `struct gdbarch_tdep' to reg_struct_return.  */
+
+/* Read a function return value of TYPE from REGCACHE, and copy that
    into VALBUF.  */
 
 static void
@@ -140,13 +169,6 @@ m68k_extract_return_value (struct type *
   int len = TYPE_LENGTH (type);
   char buf[M68K_MAX_REGISTER_SIZE];
 
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
-      && TYPE_NFIELDS (type) == 1)
-    {
-      m68k_extract_return_value (TYPE_FIELD_TYPE (type, 0), regcache, valbuf);
-      return;
-    }
-
   if (len <= 4)
     {
       regcache_raw_read (regcache, M68K_D0_REGNUM, buf);
@@ -164,29 +186,39 @@ m68k_extract_return_value (struct type *
 		    "Cannot extract return value of %d bytes long.", len);
 }
 
-/* Write into the appropriate registers a function return value stored
-   in VALBUF of type TYPE, given in virtual format.  */
-
 static void
-m68k_store_return_value (struct type *type, struct regcache *regcache,
-			 const void *valbuf)
+m68k_svr4_extract_return_value (struct type *type, struct regcache *regcache,
+				void *valbuf)
 {
   int len = TYPE_LENGTH (type);
+  char buf[M68K_MAX_REGISTER_SIZE];
 
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
-      && TYPE_NFIELDS (type) == 1)
+  if (TYPE_CODE (type) == TYPE_CODE_FLT)
     {
-      m68k_store_return_value (TYPE_FIELD_TYPE (type, 0), regcache, valbuf);
-      return;
+      regcache_raw_read (regcache, M68K_FP0_REGNUM, buf);
+      convert_typed_floating (buf, builtin_type_m68881_ext, valbuf, type);
     }
+  else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
+    regcache_raw_read (regcache, M68K_A0_REGNUM, valbuf);
+  else
+    m68k_extract_return_value (type, regcache, valbuf);
+}
+
+/* Write a function return value of TYPE from VALBUF into REGCACHE.  */
+
+static void
+m68k_store_return_value (struct type *type, struct regcache *regcache,
+			 const void *valbuf)
+{
+  int len = TYPE_LENGTH (type);
 
   if (len <= 4)
     regcache_raw_write_part (regcache, M68K_D0_REGNUM, 4 - len, len, valbuf);
   else if (len <= 8)
     {
-      regcache_raw_write_part (regcache, M68K_D1_REGNUM, 8 - len,
+      regcache_raw_write_part (regcache, M68K_D0_REGNUM, 8 - len,
 			       len - 4, valbuf);
-      regcache_raw_write (regcache, M68K_D0_REGNUM,
+      regcache_raw_write (regcache, M68K_D1_REGNUM,
 			  (char *) valbuf + (len - 4));
     }
   else
@@ -194,29 +226,108 @@ m68k_store_return_value (struct type *ty
 		    "Cannot store return value of %d bytes long.", len);
 }
 
-/* Extract from REGCACHE, which contains the (raw) register state, the
-   address in which a function should return its structure value, as a
-   CORE_ADDR.  */
-
-static CORE_ADDR
-m68k_extract_struct_value_address (struct regcache *regcache)
+static void
+m68k_svr4_store_return_value (struct type *type, struct regcache *regcache,
+			      const void *valbuf)
 {
-  char buf[4];
+  int len = TYPE_LENGTH (type);
 
-  regcache_cooked_read (regcache, M68K_D0_REGNUM, buf);
-  return extract_unsigned_integer (buf, 4);
+  if (TYPE_CODE (type) == TYPE_CODE_FLT)
+    {
+      char buf[M68K_MAX_REGISTER_SIZE];
+      convert_typed_floating (valbuf, type, buf, builtin_type_m68881_ext);
+      regcache_raw_write (regcache, M68K_FP0_REGNUM, buf);
+    }
+  else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
+    {
+      regcache_raw_write (regcache, M68K_A0_REGNUM, valbuf);
+      regcache_raw_write (regcache, M68K_D0_REGNUM, valbuf);
+    }
+  else
+    m68k_store_return_value (type, regcache, valbuf);
 }
 
+/* Return non-zero if TYPE, which is assumed to be a structure or
+   union type, should be returned in registers for architecture
+   GDBARCH.  */
+
 static int
-m68k_use_struct_convention (int gcc_p, struct type *type)
+m68k_reg_struct_return_p (struct gdbarch *gdbarch, struct type *type)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  enum type_code code = TYPE_CODE (type);
+  int len = TYPE_LENGTH (type);
+
+  gdb_assert (code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION);
+
+  if (tdep->struct_return == pcc_struct_return)
+    return 0;
+
+  return (len == 1 || len == 2 || len == 4 || len == 8);
+}
+
+/* Determine, for architecture GDBARCH, how a return value of TYPE
+   should be returned.  If it is supposed to be returned in registers,
+   and READBUF is non-zero, read the appropriate value from REGCACHE,
+   and copy it into READBUF.  If WRITEBUF is non-zero, write the value
+   from WRITEBUF into REGCACHE.  */
+
+static enum return_value_convention
+m68k_return_value (struct gdbarch *gdbarch, struct type *type,
+		   struct regcache *regcache, void *readbuf,
+		   const void *writebuf)
 {
-  enum struct_return struct_return;
+  enum type_code code = TYPE_CODE (type);
 
-  struct_return = gdbarch_tdep (current_gdbarch)->struct_return;
-  return generic_use_struct_convention (struct_return == reg_struct_return,
-					type);
+  if ((code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION)
+      && !m68k_reg_struct_return_p (gdbarch, type))
+    return RETURN_VALUE_STRUCT_CONVENTION;
+
+  /* GCC returns a `long double' in memory.  */
+  if (code == TYPE_CODE_FLT && TYPE_LENGTH (type) == 12)
+    return RETURN_VALUE_STRUCT_CONVENTION;
+
+  if (readbuf)
+    m68k_extract_return_value (type, regcache, readbuf);
+  if (writebuf)
+    m68k_store_return_value (type, regcache, writebuf);
+
+  return RETURN_VALUE_REGISTER_CONVENTION;
 }
 
+static enum return_value_convention
+m68k_svr4_return_value (struct gdbarch *gdbarch, struct type *type,
+			struct regcache *regcache, void *readbuf,
+			const void *writebuf)
+{
+  enum type_code code = TYPE_CODE (type);
+
+  if ((code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION)
+      && !m68k_reg_struct_return_p (gdbarch, type))
+    return RETURN_VALUE_STRUCT_CONVENTION;
+
+  /* This special case is for structures consisting of a single
+     `float' or `double' member.  These structures are returned in
+     %fp0.  For these structures, we call ourselves recursively,
+     changing TYPE into the type of the first member of the structure.
+     Since that should work for all structures that have only one
+     member, we don't bother to check the member's type here.  */
+  if (code == TYPE_CODE_STRUCT && TYPE_NFIELDS (type) == 1)
+    {
+      type = check_typedef (TYPE_FIELD_TYPE (type, 0));
+      return m68k_svr4_return_value (gdbarch, type, regcache,
+				     readbuf, writebuf);
+    }
+
+  if (readbuf)
+    m68k_svr4_extract_return_value (type, regcache, readbuf);
+  if (writebuf)
+    m68k_svr4_store_return_value (type, regcache, writebuf);
+
+  return RETURN_VALUE_REGISTER_CONVENTION;
+}
+\f
+
 /* A function that tells us whether the function invocation represented
    by fi does not have a frame on the stack associated with it.  If it
    does not, FRAMELESS is set to 1, else 0.  */
@@ -293,6 +404,7 @@ m68k_push_dummy_call (struct gdbarch *gd
 		      struct value **args, CORE_ADDR sp, int struct_return,
 		      CORE_ADDR struct_addr)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   char buf[4];
   int i;
 
@@ -321,7 +433,7 @@ m68k_push_dummy_call (struct gdbarch *gd
   if (struct_return)
     {
       store_unsigned_integer (buf, 4, struct_addr);
-      regcache_cooked_write (regcache, M68K_A1_REGNUM, buf);
+      regcache_cooked_write (regcache, tdep->struct_value_regnum, buf);
     }
 
   /* Store return address.  */
@@ -952,6 +1064,22 @@ m68k_get_longjmp_target (CORE_ADDR *pc)
   *pc = extract_unsigned_integer (buf, TARGET_PTR_BIT / TARGET_CHAR_BIT);
   return 1;
 }
+\f
+
+/* System V Release 4 (SVR4).  */
+
+void
+m68k_svr4_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  /* SVR4 uses a different calling convention.  */
+  set_gdbarch_return_value (gdbarch, m68k_svr4_return_value);
+
+  /* SVR4 uses %a0 instead of %a1.  */
+  tdep->struct_value_regnum = M68K_A0_REGNUM;
+}
+\f
 
 /* Function: m68k_gdbarch_init
    Initializer function for the m68k gdbarch vector.
@@ -984,11 +1112,6 @@ m68k_gdbarch_init (struct gdbarch_info i
   set_gdbarch_believe_pcc_promotion (gdbarch, 1);
   set_gdbarch_decr_pc_after_break (gdbarch, 2);
 
-  set_gdbarch_extract_return_value (gdbarch, m68k_extract_return_value);
-  set_gdbarch_store_return_value (gdbarch, m68k_store_return_value);
-  set_gdbarch_deprecated_extract_struct_value_address (gdbarch, m68k_extract_struct_value_address);
-  set_gdbarch_use_struct_convention (gdbarch, m68k_use_struct_convention);
-
   set_gdbarch_deprecated_frameless_function_invocation (gdbarch, m68k_frameless_function_invocation);
   set_gdbarch_frame_args_skip (gdbarch, 8);
 
@@ -1002,6 +1125,7 @@ m68k_gdbarch_init (struct gdbarch_info i
   set_gdbarch_fp0_regnum (gdbarch, M68K_FP0_REGNUM);
 
   set_gdbarch_push_dummy_call (gdbarch, m68k_push_dummy_call);
+  set_gdbarch_return_value (gdbarch, m68k_return_value);
 
   /* Disassembler.  */
   set_gdbarch_print_insn (gdbarch, print_insn_m68k);
@@ -1012,6 +1136,7 @@ m68k_gdbarch_init (struct gdbarch_info i
 #else
   tdep->jb_pc = -1;
 #endif
+  tdep->struct_value_regnum = M68K_A1_REGNUM;
   tdep->struct_return = pcc_struct_return;
 
   /* Frame unwinder.  */
Index: m68k-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/m68k-tdep.h,v
retrieving revision 1.7
diff -u -p -r1.7 m68k-tdep.h
--- m68k-tdep.h 1 May 2004 15:10:15 -0000 1.7
+++ m68k-tdep.h 3 May 2004 21:54:24 -0000
@@ -67,9 +67,17 @@ struct gdbarch_tdep
   /* The size of each entry in the jump buffer.  */
   size_t jb_elt_size;
 
+  /* Register in which the address to store a structure value is
+     passed to a function.  */
+  int struct_value_regnum;
+
   /* Convention for returning structures.  */
   enum struct_return struct_return;
 };
+
+/* Initialize a SVR4 architecture variant.  */
+extern void m68k_svr4_init_abi (struct gdbarch_info, struct gdbarch *);
+\f
 
 /* Functions exported from m68kbsd-tdep.c.  */
 
Index: m68klinux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m68klinux-tdep.c,v
retrieving revision 1.11
diff -u -p -r1.11 m68klinux-tdep.c
--- m68klinux-tdep.c 1 May 2004 15:10:15 -0000 1.11
+++ m68klinux-tdep.c 3 May 2004 21:54:24 -0000
@@ -1,7 +1,7 @@
 /* Motorola m68k target-dependent support for GNU/Linux.
 
-   Copyright 1996, 1998, 2000, 2001, 2002, 2003 Free Software Foundation,
-   Inc.
+   Copyright 1996, 1998, 2000, 2001, 2002, 2003, 2004
+   Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -275,108 +275,6 @@ m68k_linux_sigtramp_frame_sniffer (struc
   return NULL;
 }
 
-/* Extract from an array REGBUF containing the (raw) register state, a
-   function return value of TYPE, and copy that, in virtual format,
-   into VALBUF.  */
-
-static void
-m68k_linux_extract_return_value (struct type *type, struct regcache *regcache,
-				 void *valbuf)
-{
-  int len = TYPE_LENGTH (type);
-  char buf[M68K_MAX_REGISTER_SIZE];
-
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
-      && TYPE_NFIELDS (type) == 1)
-    {
-      m68k_linux_extract_return_value (TYPE_FIELD_TYPE (type, 0), regcache,
-				       valbuf);
-      return;
-    }
-
-  if (TYPE_CODE (type) == TYPE_CODE_FLT)
-    {
-      regcache_raw_read (regcache, M68K_FP0_REGNUM, buf);
-      convert_typed_floating (buf, builtin_type_m68881_ext, valbuf, type);
-    }
-  else if (TYPE_CODE (type) == TYPE_CODE_PTR)
-    regcache_raw_read (regcache, M68K_A0_REGNUM, valbuf);
-  else
-    {
-      if (len <= 4)
-	{
-	  regcache_raw_read (regcache, M68K_D0_REGNUM, buf);
-	  memcpy (valbuf, buf + (4 - len), len);
-	}
-      else if (len <= 8)
-	{
-	  regcache_raw_read (regcache, M68K_D0_REGNUM, buf);
-	  memcpy (valbuf, buf + (8 - len), len - 4);
-	  regcache_raw_read (regcache, M68K_D1_REGNUM,
-			     (char *) valbuf + (len - 4));
-	}
-      else
-	internal_error (__FILE__, __LINE__,
-			"Cannot extract return value of %d bytes long.", len);
-    }
-}
-
-/* Write into the appropriate registers a function return value stored
-   in VALBUF of type TYPE, given in virtual format.  */
-
-static void
-m68k_linux_store_return_value (struct type *type, struct regcache *regcache,
-			       const void *valbuf)
-{
-  int len = TYPE_LENGTH (type);
-
-  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
-      && TYPE_NFIELDS (type) == 1)
-    {
-      m68k_linux_store_return_value (TYPE_FIELD_TYPE (type, 0), regcache,
-				     valbuf);
-      return;
-    }
-
-  if (TYPE_CODE (type) == TYPE_CODE_FLT)
-    {
-      char buf[M68K_MAX_REGISTER_SIZE];
-      convert_typed_floating (valbuf, type, buf, builtin_type_m68881_ext);
-      regcache_raw_write (regcache, M68K_FP0_REGNUM, buf);
-    }
-  else if (TYPE_CODE (type) == TYPE_CODE_PTR)
-    regcache_raw_write (regcache, M68K_A0_REGNUM, valbuf);
-  else
-    {
-      if (len <= 4)
-	regcache_raw_write_part (regcache, M68K_D0_REGNUM,
-				 4 - len, len, valbuf);
-      else if (len <= 8)
-	{
-	  regcache_raw_write_part (regcache, M68K_D1_REGNUM, 8 - len,
-				   len - 4, valbuf);
-	  regcache_raw_write (regcache, M68K_D0_REGNUM,
-			      (char *) valbuf + (len - 4));
-	}
-      else
-	internal_error (__FILE__, __LINE__,
-			"Cannot store return value of %d bytes long.", len);
-    }
-}
-
-/* Extract from an array REGBUF containing the (raw) register state
-   the address in which a function should return its structure value,
-   as a CORE_ADDR.  */
-
-static CORE_ADDR
-m68k_linux_extract_struct_value_address (struct regcache *regcache)
-{
-  char buf[4];
-
-  regcache_cooked_read (regcache, M68K_A0_REGNUM, buf);
-  return extract_unsigned_integer (buf, 4);
-}
-
 static void
 m68k_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -384,11 +282,15 @@ m68k_linux_init_abi (struct gdbarch_info
 
   tdep->jb_pc = M68K_LINUX_JB_PC;
   tdep->jb_elt_size = M68K_LINUX_JB_ELEMENT_SIZE;
-  tdep->struct_return = reg_struct_return;
 
-  set_gdbarch_extract_return_value (gdbarch, m68k_linux_extract_return_value);
-  set_gdbarch_store_return_value (gdbarch, m68k_linux_store_return_value);
-  set_gdbarch_deprecated_extract_struct_value_address (gdbarch, m68k_linux_extract_struct_value_address);
+  /* GNU/Linux uses a calling convention that's similar to SVR4.  It
+     returns integer values in %d0/%di, pointer values in %a0 and
+     floating values in %fp0, just like SVR4, but uses %a1 to pass the
+     address to store a structure value.  It also returns small
+     structures in registers instead of memory.  */
+  m68k_svr4_init_abi (info, gdbarch);
+  tdep->struct_value_regnum = M68K_A1_REGNUM;
+  tdep->struct_return = reg_struct_return;
 
   frame_unwind_append_sniffer (gdbarch, m68k_linux_sigtramp_frame_sniffer);
 

Re: [PATCH/RFA] Update m68k function return value handling

[Michael Snyder]

Mark Kettenis wrote:
> Hi Andreas,
> 
> Here's a patch that converts the m68k to the new function return value
> handling mechanism.  I tried very hard to make it possible to deal
> with all the different m68k ABI's.  I'm pretty sure I've got it right
> for at least OpenBSD, NetBSD and Linux.  On other systems it can't
> really be worse than the old code. 

Unfortunately, it can.  The problem is that when you use the new
gdbarch_return_value_p method, print_return_value simply gives up
on any function that returns a struct via a pointer argument
(what we refer to as the "struct convention").  I don't know why
it does (esp. in view of all the code you've added for determining
eg. which register this pointer is passed in), but it does.

This is especially bad for cross-m68k targets, since when
pcc_struct_convention is used, m68k_reg_struct_return_p
always returns false.  BTW this is wrong for gcc m68k-elf,
so I don't understand why pcc_struct_convention is set for
all cross targets...

 > There is now a rather big comment
> explaining the conventions that have been used in the past.  I've
> tested this on m68k-unknown-netbsdelf1.6.2, which uses the official
> SVR4 convention, and all return values tests in the testsuite pass
> there now.  I'm currently testing m68k-unknown-openbsd3.4.
> Unfortunately I can't test m68k-unknown-linux, since I don't have any
> disk space left on my Mac :-(.  But I'm pretty confident this will
> work for Linux too.
> 
> Anyway, are the Linux changes OK with you?
> 
> Mark
> 
> 
> Index: ChangeLog
> from  Mark Kettenis  <kettenis@gnu.org>
> 
> 	* m68k-tdep.h (struct gdbarch_tdep): Add member
> 	struct_value_regnum.
> 	(m68k_svr4_init_abi): New prototype.
> 	* m68k-tdep.c: Add comment about all the different calling
> 	conventions.
> 	(m68k_extract_return_value): Remove code dealing with single-field
> 	structs.
> 	(m68k_store_return_value): Remove code dealing with single-field
> 	structs.  Correctly store return values of 5, 6, 7 or 8 bytes.
> 	(m68k_extract_struct_value_address): Remove function.
> 	(m68k_svr4_extract_return_value,m68k_svr4_store_return_value)
> 	(m68k_reg_struct_return_p, m68k_return_value)
> 	(m68k_svr4_return_value): New functions.
> 	(m68k_use_struct_convention): Remove function.
> 	(m68k_push_dummy_call): Use new struct_value_regnum member of
> 	`struct gdbarch_tdep' instead of hardcoded register number to
> 	store STRUCT_ADDR.
> 	(m68k_svr4_init_abi): New function.
> 	(m68k_gdbarch_init): Don't set extract_return_value,
> 	store_return_values, deprecated_extract_struct_value_address and
> 	use_struct_convention.  Set return_value instead.  Initialize new
> 	struct_value_regnum member of `struct gdbarch_tdep'.
> 	* m68klinux-tdep.c: Update copyright year.
> 	(m68k_linux_extract_return_value, m68k_linux_store_return_value)
> 	(m68k_linux_extract_struct_value_address): Remove function.
> 	(m68k_linux_init_abi): Don't set extract_return_value,
> 	store_return_values, deprecated_extract_struct_value_address and
> 	use_struct_convention.  Call m68k_svr4_init_abi but override the
> 	new struct_value_regnum member of `struct gdbarch_tdep'.
> 
> Index: m68k-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/m68k-tdep.c,v
> retrieving revision 1.84
> diff -u -p -r1.84 m68k-tdep.c
> --- m68k-tdep.c 1 May 2004 15:10:15 -0000 1.84
> +++ m68k-tdep.c 2 May 2004 21:41:48 -0000
> @@ -25,6 +25,7 @@
>  #include "frame.h"
>  #include "frame-base.h"
>  #include "frame-unwind.h"
> +#include "floatformat.h"
>  #include "symtab.h"
>  #include "gdbcore.h"
>  #include "value.h"
> @@ -129,8 +130,36 @@ m68k_register_name (int regnum)
>      return register_names[regnum];
>  }
>  \f
> -/* Extract from an array REGBUF containing the (raw) register state, a
> -   function return value of TYPE, and copy that, in virtual format,
> +/* There is a fair number of calling conventions that are in somewhat
> +   wide use.  The 68000/08/10 don't support an FPU, not even as a
> +   coprocessor.  All function return values are stored in %d0/%d1.
> +   Structures are returned in a static buffer, a pointer to which is
> +   returned in %d0.  This means that functions returning a structure
> +   are not re-entrant.  To avoid this problem some systems use a
> +   convention where the caller passes a pointer to a buffer in %a1
> +   where the return values is to be stored.  This convention is the
> +   default, and is implemented in the function m68k_return_value.
> +
> +   The 68020/030/040/060 do support an FPU, either as a coprocessor
> +   (68881/2) or built-in (68040/68060).  That's why System V release 4
> +   (SVR4) instroduces a new calling convention specified by the SVR4
> +   psABI.  Integer values are returned in %d0/%d1, pointer return
> +   values in %a0 and floating values in %fp0.  When calling functions
> +   returning a structure the caller should pass a pointer to a buffer
> +   for the return value in %a0.  This convention is implemented in the
> +   function m68k_svr4_return_value, and by appropriately setting the
> +   struct_value_regnum member of `struct gdbarch_tdep'.
> +
> +   GNU/Linux returns values in the same way as SVR4 does, but uses %a1
> +   for passing the structure return value buffer.
> +
> +   GCC can also generate code where small structures are returned in
> +   %d0/%d1 instead of in memory by using -freg-struct-return.  This is
> +   the default on NetBSD a.out, OpenBSD and GNU/Linux and several
> +   embedded systems.  This convention is implemented by setting the
> +   struct_return member of `struct gdbarch_tdep' to reg_struct_return.  */
> +
> +/* Read a function return value of TYPE from REGCACHE, and copy that
>     into VALBUF.  */
>  
>  static void
> @@ -140,13 +169,6 @@ m68k_extract_return_value (struct type *
>    int len = TYPE_LENGTH (type);
>    char buf[M68K_MAX_REGISTER_SIZE];
>  
> -  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
> -      && TYPE_NFIELDS (type) == 1)
> -    {
> -      m68k_extract_return_value (TYPE_FIELD_TYPE (type, 0), regcache, valbuf);
> -      return;
> -    }
> -
>    if (len <= 4)
>      {
>        regcache_raw_read (regcache, M68K_D0_REGNUM, buf);
> @@ -164,29 +186,39 @@ m68k_extract_return_value (struct type *
>  		    "Cannot extract return value of %d bytes long.", len);
>  }
>  
> -/* Write into the appropriate registers a function return value stored
> -   in VALBUF of type TYPE, given in virtual format.  */
> -
>  static void
> -m68k_store_return_value (struct type *type, struct regcache *regcache,
> -			 const void *valbuf)
> +m68k_svr4_extract_return_value (struct type *type, struct regcache *regcache,
> +				void *valbuf)
>  {
>    int len = TYPE_LENGTH (type);
> +  char buf[M68K_MAX_REGISTER_SIZE];
>  
> -  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
> -      && TYPE_NFIELDS (type) == 1)
> +  if (TYPE_CODE (type) == TYPE_CODE_FLT)
>      {
> -      m68k_store_return_value (TYPE_FIELD_TYPE (type, 0), regcache, valbuf);
> -      return;
> +      regcache_raw_read (regcache, M68K_FP0_REGNUM, buf);
> +      convert_typed_floating (buf, builtin_type_m68881_ext, valbuf, type);
>      }
> +  else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
> +    regcache_raw_read (regcache, M68K_A0_REGNUM, valbuf);
> +  else
> +    m68k_extract_return_value (type, regcache, valbuf);
> +}
> +
> +/* Write a function return value of TYPE from VALBUF into REGCACHE.  */
> +
> +static void
> +m68k_store_return_value (struct type *type, struct regcache *regcache,
> +			 const void *valbuf)
> +{
> +  int len = TYPE_LENGTH (type);
>  
>    if (len <= 4)
>      regcache_raw_write_part (regcache, M68K_D0_REGNUM, 4 - len, len, valbuf);
>    else if (len <= 8)
>      {
> -      regcache_raw_write_part (regcache, M68K_D1_REGNUM, 8 - len,
> +      regcache_raw_write_part (regcache, M68K_D0_REGNUM, 8 - len,
>  			       len - 4, valbuf);
> -      regcache_raw_write (regcache, M68K_D0_REGNUM,
> +      regcache_raw_write (regcache, M68K_D1_REGNUM,
>  			  (char *) valbuf + (len - 4));
>      }
>    else
> @@ -194,29 +226,108 @@ m68k_store_return_value (struct type *ty
>  		    "Cannot store return value of %d bytes long.", len);
>  }
>  
> -/* Extract from REGCACHE, which contains the (raw) register state, the
> -   address in which a function should return its structure value, as a
> -   CORE_ADDR.  */
> -
> -static CORE_ADDR
> -m68k_extract_struct_value_address (struct regcache *regcache)
> +static void
> +m68k_svr4_store_return_value (struct type *type, struct regcache *regcache,
> +			      const void *valbuf)
>  {
> -  char buf[4];
> +  int len = TYPE_LENGTH (type);
>  
> -  regcache_cooked_read (regcache, M68K_D0_REGNUM, buf);
> -  return extract_unsigned_integer (buf, 4);
> +  if (TYPE_CODE (type) == TYPE_CODE_FLT)
> +    {
> +      char buf[M68K_MAX_REGISTER_SIZE];
> +      convert_typed_floating (valbuf, type, buf, builtin_type_m68881_ext);
> +      regcache_raw_write (regcache, M68K_FP0_REGNUM, buf);
> +    }
> +  else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
> +    {
> +      regcache_raw_write (regcache, M68K_A0_REGNUM, valbuf);
> +      regcache_raw_write (regcache, M68K_D0_REGNUM, valbuf);
> +    }
> +  else
> +    m68k_store_return_value (type, regcache, valbuf);
>  }
>  
> +/* Return non-zero if TYPE, which is assumed to be a structure or
> +   union type, should be returned in registers for architecture
> +   GDBARCH.  */
> +
>  static int
> -m68k_use_struct_convention (int gcc_p, struct type *type)
> +m68k_reg_struct_return_p (struct gdbarch *gdbarch, struct type *type)
> +{
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +  enum type_code code = TYPE_CODE (type);
> +  int len = TYPE_LENGTH (type);
> +
> +  gdb_assert (code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION);
> +
> +  if (tdep->struct_return == pcc_struct_return)
> +    return 0;
> +
> +  return (len == 1 || len == 2 || len == 4 || len == 8);
> +}
> +
> +/* Determine, for architecture GDBARCH, how a return value of TYPE
> +   should be returned.  If it is supposed to be returned in registers,
> +   and READBUF is non-zero, read the appropriate value from REGCACHE,
> +   and copy it into READBUF.  If WRITEBUF is non-zero, write the value
> +   from WRITEBUF into REGCACHE.  */
> +
> +static enum return_value_convention
> +m68k_return_value (struct gdbarch *gdbarch, struct type *type,
> +		   struct regcache *regcache, void *readbuf,
> +		   const void *writebuf)
>  {
> -  enum struct_return struct_return;
> +  enum type_code code = TYPE_CODE (type);
>  
> -  struct_return = gdbarch_tdep (current_gdbarch)->struct_return;
> -  return generic_use_struct_convention (struct_return == reg_struct_return,
> -					type);
> +  if ((code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION)
> +      && !m68k_reg_struct_return_p (gdbarch, type))
> +    return RETURN_VALUE_STRUCT_CONVENTION;
> +
> +  /* GCC returns a `long double' in memory.  */
> +  if (code == TYPE_CODE_FLT && TYPE_LENGTH (type) == 12)
> +    return RETURN_VALUE_STRUCT_CONVENTION;
> +
> +  if (readbuf)
> +    m68k_extract_return_value (type, regcache, readbuf);
> +  if (writebuf)
> +    m68k_store_return_value (type, regcache, writebuf);
> +
> +  return RETURN_VALUE_REGISTER_CONVENTION;
>  }
>  
> +static enum return_value_convention
> +m68k_svr4_return_value (struct gdbarch *gdbarch, struct type *type,
> +			struct regcache *regcache, void *readbuf,
> +			const void *writebuf)
> +{
> +  enum type_code code = TYPE_CODE (type);
> +
> +  if ((code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION)
> +      && !m68k_reg_struct_return_p (gdbarch, type))
> +    return RETURN_VALUE_STRUCT_CONVENTION;
> +
> +  /* This special case is for structures consisting of a single
> +     `float' or `double' member.  These structures are returned in
> +     %fp0.  For these structures, we call ourselves recursively,
> +     changing TYPE into the type of the first member of the structure.
> +     Since that should work for all structures that have only one
> +     member, we don't bother to check the member's type here.  */
> +  if (code == TYPE_CODE_STRUCT && TYPE_NFIELDS (type) == 1)
> +    {
> +      type = check_typedef (TYPE_FIELD_TYPE (type, 0));
> +      return m68k_svr4_return_value (gdbarch, type, regcache,
> +				     readbuf, writebuf);
> +    }
> +
> +  if (readbuf)
> +    m68k_svr4_extract_return_value (type, regcache, readbuf);
> +  if (writebuf)
> +    m68k_svr4_store_return_value (type, regcache, writebuf);
> +
> +  return RETURN_VALUE_REGISTER_CONVENTION;
> +}
> +\f
> +
>  /* A function that tells us whether the function invocation represented
>     by fi does not have a frame on the stack associated with it.  If it
>     does not, FRAMELESS is set to 1, else 0.  */
> @@ -293,6 +404,7 @@ m68k_push_dummy_call (struct gdbarch *gd
>  		      struct value **args, CORE_ADDR sp, int struct_return,
>  		      CORE_ADDR struct_addr)
>  {
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>    char buf[4];
>    int i;
>  
> @@ -321,7 +433,7 @@ m68k_push_dummy_call (struct gdbarch *gd
>    if (struct_return)
>      {
>        store_unsigned_integer (buf, 4, struct_addr);
> -      regcache_cooked_write (regcache, M68K_A1_REGNUM, buf);
> +      regcache_cooked_write (regcache, tdep->struct_value_regnum, buf);
>      }
>  
>    /* Store return address.  */
> @@ -952,6 +1064,22 @@ m68k_get_longjmp_target (CORE_ADDR *pc)
>    *pc = extract_unsigned_integer (buf, TARGET_PTR_BIT / TARGET_CHAR_BIT);
>    return 1;
>  }
> +\f
> +
> +/* System V Release 4 (SVR4).  */
> +
> +void
> +m68k_svr4_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> +{
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
> +  /* SVR4 uses a different calling convention.  */
> +  set_gdbarch_return_value (gdbarch, m68k_svr4_return_value);
> +
> +  /* SVR4 uses %a0 instead of %a1.  */
> +  tdep->struct_value_regnum = M68K_A0_REGNUM;
> +}
> +\f
>  
>  /* Function: m68k_gdbarch_init
>     Initializer function for the m68k gdbarch vector.
> @@ -984,11 +1112,6 @@ m68k_gdbarch_init (struct gdbarch_info i
>    set_gdbarch_believe_pcc_promotion (gdbarch, 1);
>    set_gdbarch_decr_pc_after_break (gdbarch, 2);
>  
> -  set_gdbarch_extract_return_value (gdbarch, m68k_extract_return_value);
> -  set_gdbarch_store_return_value (gdbarch, m68k_store_return_value);
> -  set_gdbarch_deprecated_extract_struct_value_address (gdbarch, m68k_extract_struct_value_address);
> -  set_gdbarch_use_struct_convention (gdbarch, m68k_use_struct_convention);
> -
>    set_gdbarch_deprecated_frameless_function_invocation (gdbarch, m68k_frameless_function_invocation);
>    set_gdbarch_frame_args_skip (gdbarch, 8);
>  
> @@ -1002,6 +1125,7 @@ m68k_gdbarch_init (struct gdbarch_info i
>    set_gdbarch_fp0_regnum (gdbarch, M68K_FP0_REGNUM);
>  
>    set_gdbarch_push_dummy_call (gdbarch, m68k_push_dummy_call);
> +  set_gdbarch_return_value (gdbarch, m68k_return_value);
>  
>    /* Disassembler.  */
>    set_gdbarch_print_insn (gdbarch, print_insn_m68k);
> @@ -1012,6 +1136,7 @@ m68k_gdbarch_init (struct gdbarch_info i
>  #else
>    tdep->jb_pc = -1;
>  #endif
> +  tdep->struct_value_regnum = M68K_A1_REGNUM;
>    tdep->struct_return = pcc_struct_return;
>  
>    /* Frame unwinder.  */
> Index: m68k-tdep.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/m68k-tdep.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 m68k-tdep.h
> --- m68k-tdep.h 1 May 2004 15:10:15 -0000 1.7
> +++ m68k-tdep.h 2 May 2004 21:41:48 -0000
> @@ -67,9 +67,17 @@ struct gdbarch_tdep
>    /* The size of each entry in the jump buffer.  */
>    size_t jb_elt_size;
>  
> +  /* Register in which the address to store a structure value is
> +     passed to a function.  */
> +  int struct_value_regnum;
> +
>    /* Convention for returning structures.  */
>    enum struct_return struct_return;
>  };
> +
> +/* Initialize a SVR4 architecture variant.  */
> +extern void m68k_svr4_init_abi (struct gdbarch_info, struct gdbarch *);
> +\f
>  
>  /* Functions exported from m68kbsd-tdep.c.  */
>  
> Index: m68klinux-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/m68klinux-tdep.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 m68klinux-tdep.c
> --- m68klinux-tdep.c 1 May 2004 15:10:15 -0000 1.11
> +++ m68klinux-tdep.c 2 May 2004 21:41:48 -0000
> @@ -1,7 +1,7 @@
>  /* Motorola m68k target-dependent support for GNU/Linux.
>  
> -   Copyright 1996, 1998, 2000, 2001, 2002, 2003 Free Software Foundation,
> -   Inc.
> +   Copyright 1996, 1998, 2000, 2001, 2002, 2003, 2004
> +   Free Software Foundation, Inc.
>  
>     This file is part of GDB.
>  
> @@ -275,108 +275,6 @@ m68k_linux_sigtramp_frame_sniffer (struc
>    return NULL;
>  }
>  
> -/* Extract from an array REGBUF containing the (raw) register state, a
> -   function return value of TYPE, and copy that, in virtual format,
> -   into VALBUF.  */
> -
> -static void
> -m68k_linux_extract_return_value (struct type *type, struct regcache *regcache,
> -				 void *valbuf)
> -{
> -  int len = TYPE_LENGTH (type);
> -  char buf[M68K_MAX_REGISTER_SIZE];
> -
> -  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
> -      && TYPE_NFIELDS (type) == 1)
> -    {
> -      m68k_linux_extract_return_value (TYPE_FIELD_TYPE (type, 0), regcache,
> -				       valbuf);
> -      return;
> -    }
> -
> -  if (TYPE_CODE (type) == TYPE_CODE_FLT)
> -    {
> -      regcache_raw_read (regcache, M68K_FP0_REGNUM, buf);
> -      convert_typed_floating (buf, builtin_type_m68881_ext, valbuf, type);
> -    }
> -  else if (TYPE_CODE (type) == TYPE_CODE_PTR)
> -    regcache_raw_read (regcache, M68K_A0_REGNUM, valbuf);
> -  else
> -    {
> -      if (len <= 4)
> -	{
> -	  regcache_raw_read (regcache, M68K_D0_REGNUM, buf);
> -	  memcpy (valbuf, buf + (4 - len), len);
> -	}
> -      else if (len <= 8)
> -	{
> -	  regcache_raw_read (regcache, M68K_D0_REGNUM, buf);
> -	  memcpy (valbuf, buf + (8 - len), len - 4);
> -	  regcache_raw_read (regcache, M68K_D1_REGNUM,
> -			     (char *) valbuf + (len - 4));
> -	}
> -      else
> -	internal_error (__FILE__, __LINE__,
> -			"Cannot extract return value of %d bytes long.", len);
> -    }
> -}
> -
> -/* Write into the appropriate registers a function return value stored
> -   in VALBUF of type TYPE, given in virtual format.  */
> -
> -static void
> -m68k_linux_store_return_value (struct type *type, struct regcache *regcache,
> -			       const void *valbuf)
> -{
> -  int len = TYPE_LENGTH (type);
> -
> -  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
> -      && TYPE_NFIELDS (type) == 1)
> -    {
> -      m68k_linux_store_return_value (TYPE_FIELD_TYPE (type, 0), regcache,
> -				     valbuf);
> -      return;
> -    }
> -
> -  if (TYPE_CODE (type) == TYPE_CODE_FLT)
> -    {
> -      char buf[M68K_MAX_REGISTER_SIZE];
> -      convert_typed_floating (valbuf, type, buf, builtin_type_m68881_ext);
> -      regcache_raw_write (regcache, M68K_FP0_REGNUM, buf);
> -    }
> -  else if (TYPE_CODE (type) == TYPE_CODE_PTR)
> -    regcache_raw_write (regcache, M68K_A0_REGNUM, valbuf);
> -  else
> -    {
> -      if (len <= 4)
> -	regcache_raw_write_part (regcache, M68K_D0_REGNUM,
> -				 4 - len, len, valbuf);
> -      else if (len <= 8)
> -	{
> -	  regcache_raw_write_part (regcache, M68K_D1_REGNUM, 8 - len,
> -				   len - 4, valbuf);
> -	  regcache_raw_write (regcache, M68K_D0_REGNUM,
> -			      (char *) valbuf + (len - 4));
> -	}
> -      else
> -	internal_error (__FILE__, __LINE__,
> -			"Cannot store return value of %d bytes long.", len);
> -    }
> -}
> -
> -/* Extract from an array REGBUF containing the (raw) register state
> -   the address in which a function should return its structure value,
> -   as a CORE_ADDR.  */
> -
> -static CORE_ADDR
> -m68k_linux_extract_struct_value_address (struct regcache *regcache)
> -{
> -  char buf[4];
> -
> -  regcache_cooked_read (regcache, M68K_A0_REGNUM, buf);
> -  return extract_unsigned_integer (buf, 4);
> -}
> -
>  static void
>  m68k_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
> @@ -384,11 +282,15 @@ m68k_linux_init_abi (struct gdbarch_info
>  
>    tdep->jb_pc = M68K_LINUX_JB_PC;
>    tdep->jb_elt_size = M68K_LINUX_JB_ELEMENT_SIZE;
> -  tdep->struct_return = reg_struct_return;
>  
> -  set_gdbarch_extract_return_value (gdbarch, m68k_linux_extract_return_value);
> -  set_gdbarch_store_return_value (gdbarch, m68k_linux_store_return_value);
> -  set_gdbarch_deprecated_extract_struct_value_address (gdbarch, m68k_linux_extract_struct_value_address);
> +  /* GNU/Linux uses a calling convention that's similar to SVR4.  It
> +     returns integer values in %d0/%di, pointer values in %a0 and
> +     floating values in %fp0, just like SVR4, but uses %a1 to pass the
> +     address to store a structure value.  It also returns small
> +     structures in registers instead of memory.  */
> +  m68k_svr4_init_abi (info, gdbarch);
> +  tdep->struct_value_regnum = M68K_A1_REGNUM;
> +  tdep->struct_return = reg_struct_return;
>  
>    frame_unwind_append_sniffer (gdbarch, m68k_linux_sigtramp_frame_sniffer);
>  
> 

Re: [PATCH/RFA] Update m68k function return value handling

[Michael Snyder]

Mark Kettenis wrote:
>    From: Andreas Schwab <schwab@suse.de>
>    Date: Mon, 03 May 2004 23:21:04 +0200
> 
>    Mark Kettenis <kettenis@chello.nl> writes:
> 
>    > Anyway, are the Linux changes OK with you?
> 
>    Thanks, they look OK.  I have sucessfully built it natively on m68k-linux
>    and ran a few tests, no failures.
> 
> Thanks for testing it on m68k-linux.  I noticed that I forgot about
> including "floatformat.h" so I didn't update Makefile.in.  Fixed with
> the attached patch which is what I actually checked in.

FYI, there's 80 or more  fails for an m68k-elf cross target.
I'm not sure if it's better or worse with your patch.
Andreas, it seems like some of your recent changes (last six
months or so) have broken the cross target.

Re: [PATCH/RFA] Update m68k function return value handling

[Andreas Schwab]

Michael Snyder <msnyder@redhat.com> writes:

> FYI, there's 80 or more  fails for an m68k-elf cross target.
> I'm not sure if it's better or worse with your patch.
> Andreas, it seems like some of your recent changes (last six
> months or so) have broken the cross target.

The last time I changed something for m68k was on 2003-09-25, which is
already more than 6 months ago.  The biggest changes of mine happend about
one year ago, with another batch about 8 months ago, where I converted the
m68k backend to move away from the deprecated interfaces.

Would it be possible for you to find out which changes are bad?  I have
them all tested natively on m68k-linux as much as possible.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH/RFA] Update m68k function return value handling

[Mark Kettenis]

   Date: Tue, 04 May 2004 01:51:35 +0000
   From: Michael Snyder <msnyder@redhat.com>

   Mark Kettenis wrote:
   > Hi Andreas,
   > 
   > Here's a patch that converts the m68k to the new function return value
   > handling mechanism.  I tried very hard to make it possible to deal
   > with all the different m68k ABI's.  I'm pretty sure I've got it right
   > for at least OpenBSD, NetBSD and Linux.  On other systems it can't
   > really be worse than the old code. 

   Unfortunately, it can.  The problem is that when you use the new
   gdbarch_return_value_p method, print_return_value simply gives up
   on any function that returns a struct via a pointer argument
   (what we refer to as the "struct convention").  I don't know why
   it does (esp. in view of all the code you've added for determining
   eg. which register this pointer is passed in), but it does.

This has been discussed in the past.  The problem is that on almost
every target, you cannot reliably tell where the return value is
stored.  On some targets you can't even tell this when the call is
finished; I believe Andrew said this was the case on powerpc.  Anyway,
most targets, and m68k might be one of them, can tell where the
structure is stored when the function is finished.  I've promised to
look into it some time ago (since i386 is in a similar situation).
Just didn't find the time yet...

   This is especially bad for cross-m68k targets, since when
   pcc_struct_convention is used, m68k_reg_struct_return_p
   always returns false.  BTW this is wrong for gcc m68k-elf,
   so I don't understand why pcc_struct_convention is set for
   all cross targets...

Hmm.  I looked at GCC, to see what the "default" convention would be,
and config/m68k/m68k.h implicates that all structures would be
returned in static storage.  I somehow overlooked that the majority,
if not all, "embedded" targets use config/m68k/m68kemb.h, which
overrides the default in config/m68k/m68k.h to return small structures
in registers.  So it seems I've simply looked in the wrong place and
chose the wrong default.  Feel free to change it to reg_struct_return.
The Linux, OpenBSD and NetBSD targets should be immune to such a
change.  If you don't get to making the change I'll do so in a few
days.

Re: [PATCH/RFA] Update m68k function return value handling

[Michael Snyder]

Andreas Schwab wrote:
> Michael Snyder <msnyder@redhat.com> writes:
> 
> 
>>FYI, there's 80 or more  fails for an m68k-elf cross target.
>>I'm not sure if it's better or worse with your patch.
>>Andreas, it seems like some of your recent changes (last six
>>months or so) have broken the cross target.
> 
> 
> The last time I changed something for m68k was on 2003-09-25, which is
> already more than 6 months ago.  The biggest changes of mine happend about
> one year ago, with another batch about 8 months ago, where I converted the
> m68k backend to move away from the deprecated interfaces.

Yeah, it seems likely that no one has built an m68k cross toolchain
in the intervening time.  ;-(


> Would it be possible for you to find out which changes are bad?  I have
> them all tested natively on m68k-linux as much as possible.

I'm investigating.  The first problem seems to be the choice of
pcc_struct_return as the default.  GCC doesn't use that, whether
for native or cross.  After that, I'm suspecting that we still
need stack_align.  One changelog entry says that you replaced
stack_align with gdbarch_parm_boundary -- the trouble is that
they aren't the same thing.

Re: [PATCH/RFA] Update m68k function return value handling

[Michael Snyder]

Mark Kettenis wrote:
>    Date: Tue, 04 May 2004 01:51:35 +0000
>    From: Michael Snyder <msnyder@redhat.com>
> 
>    Mark Kettenis wrote:
>    > Hi Andreas,
>    > 
>    > Here's a patch that converts the m68k to the new function return value
>    > handling mechanism.  I tried very hard to make it possible to deal
>    > with all the different m68k ABI's.  I'm pretty sure I've got it right
>    > for at least OpenBSD, NetBSD and Linux.  On other systems it can't
>    > really be worse than the old code. 
> 
>    Unfortunately, it can.  The problem is that when you use the new
>    gdbarch_return_value_p method, print_return_value simply gives up
>    on any function that returns a struct via a pointer argument
>    (what we refer to as the "struct convention").  I don't know why
>    it does (esp. in view of all the code you've added for determining
>    eg. which register this pointer is passed in), but it does.
> 
> This has been discussed in the past.  The problem is that on almost
> every target, you cannot reliably tell where the return value is
> stored.  On some targets you can't even tell this when the call is
> finished; I believe Andrew said this was the case on powerpc. 

Well, but it's not the case on m68k, is it?  It seems that
the location is returned in the same register as any other
return value.

> Anyway,
> most targets, and m68k might be one of them, can tell where the
> structure is stored when the function is finished.  I've promised to
> look into it some time ago (since i386 is in a similar situation).
> Just didn't find the time yet...

Didn't mean to nag -- it's just that I hoped your changes
would make the results better.


>    This is especially bad for cross-m68k targets, since when
>    pcc_struct_convention is used, m68k_reg_struct_return_p
>    always returns false.  BTW this is wrong for gcc m68k-elf,
>    so I don't understand why pcc_struct_convention is set for
>    all cross targets...
> 
> Hmm.  I looked at GCC, to see what the "default" convention would be,
> and config/m68k/m68k.h implicates that all structures would be
> returned in static storage.  I somehow overlooked that the majority,
> if not all, "embedded" targets use config/m68k/m68kemb.h, which
> overrides the default in config/m68k/m68k.h to return small structures
> in registers.  So it seems I've simply looked in the wrong place and
> chose the wrong default.  Feel free to change it to reg_struct_return.
> The Linux, OpenBSD and NetBSD targets should be immune to such a
> change.  If you don't get to making the change I'll do so in a few
> days.

OK, here are test results:
ppc_struct_return: 91 fails
reg_struct_return: 63 fails.

Not as big an improvement as I'd hoped, but it's an improvement.
I'll check it in.  Unfortunately I won't have time to look into
this further, I'm starting a new project (err) yesterday.

Michael