[patch/rfc] Eliminate register_value_being_returned and legacy_return_value

[patch/rfc] Eliminate register_value_being_returned and legacy_return_value

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]

Hello,

This patch eliminates calls to both register_value_being_returned and 
legacy_return_value by inlining / simplifying the callers.  Turns out 
there was much duplication of effort with both the caller and callees 
all performing the same tasks.

I'll commit tomorrow once I've got a second set of test results.

Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 8464 bytes --]

2004-06-10  Andrew Cagney  <cagney@gnu.org>

	* values.c (register_value_being_returned): Delete function.
	* infcmd.c (legacy_return_value): Delete function.
	* infcall.c (call_function_by_hand): Inline
	"register_value_being_returned", simplify.
	* values.c (using_struct_return): Update comment, refer to
	print_return_value instead of register_value_being_returned.
	* infcmd.c (print_return_value): Inline calls to
	register_value_being_returned and legacy_return_value.  Simplify.

Index: infcall.c
===================================================================
RCS file: /cvs/src/src/gdb/infcall.c,v
retrieving revision 1.49
diff -p -u -r1.49 infcall.c
--- infcall.c	10 Jun 2004 17:39:26 -0000	1.49
+++ infcall.c	10 Jun 2004 21:01:16 -0000
@@ -922,9 +922,14 @@ the function call).", name);
      leave the RETBUF alone.  */
   do_cleanups (inf_status_cleanup);
 
-  /* Figure out the value returned by the function.  */
-  if (struct_return)
-    {
+  /* Figure out the value returned by the function, return that.  */
+  {
+    struct value *retval;
+    if (TYPE_CODE (value_type) == TYPE_CODE_VOID)
+      /* If the function returns void, don't bother fetching the
+	 return value.  */
+      retval = allocate_value (value_type);
+    else if (struct_return)
       /* NOTE: cagney/2003-09-27: This assumes that PUSH_DUMMY_CALL
 	 has correctly stored STRUCT_ADDR in the target.  In the past
 	 that hasn't been the case, the old MIPS PUSH_ARGUMENTS
@@ -933,18 +938,30 @@ the function call).", name);
 	 you're seeing problems with values being returned using the
 	 "struct return convention", check that PUSH_DUMMY_CALL isn't
 	 playing tricks.  */
-      struct value *retval = value_at (value_type, struct_addr, NULL);
-      do_cleanups (retbuf_cleanup);
-      return retval;
-    }
-  else
-    {
-      /* The non-register case was handled above.  */
-      struct value *retval = register_value_being_returned (value_type,
-							    retbuf);
-      do_cleanups (retbuf_cleanup);
-      return retval;
-    }
+      retval = value_at (value_type, struct_addr, NULL);
+    else if (gdbarch_return_value_p (current_gdbarch))
+      {
+	/* This code only handles "register convention".  */
+	retval = allocate_value (value_type);
+	gdb_assert (gdbarch_return_value (current_gdbarch, value_type,
+					  NULL, NULL, NULL)
+		    == RETURN_VALUE_REGISTER_CONVENTION);
+	gdbarch_return_value (current_gdbarch, value_type, retbuf,
+			      VALUE_CONTENTS_RAW (retval) /*read*/,
+			      NULL /*write*/);
+      }
+    else
+      {
+	/* NOTE: cagney/2003-10-20: Unlike "gdbarch_return_value", the
+	   EXTRACT_RETURN_VALUE and USE_STRUCT_CONVENTION methods do
+	   not handle the edge case of a function returning a small
+	   structure / union in registers.  */
+	retval = allocate_value (value_type);
+	EXTRACT_RETURN_VALUE (value_type, retbuf, VALUE_CONTENTS_RAW (retval));
+      }
+    do_cleanups (retbuf_cleanup);
+    return retval;
+  }
 }
 
 void _initialize_infcall (void);
Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.114
diff -p -u -r1.114 infcmd.c
--- infcmd.c	9 Jun 2004 20:42:29 -0000	1.114
+++ infcmd.c	10 Jun 2004 21:01:16 -0000
@@ -1072,41 +1072,6 @@ advance_command (char *arg, int from_tty
   until_break_command (arg, from_tty, 1);
 }
 \f
-
-static struct value *
-legacy_return_value (int struct_return, struct type *value_type)
-{
-  struct value *value;
-
-  if (!struct_return)
-    {
-      /* The return value can be found in the inferior's registers.  */
-      return register_value_being_returned (value_type, stop_registers);
-    }
-
-  if (DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS_P ())
-    {
-      CORE_ADDR addr;
-
-      addr = DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS (stop_registers);
-      if (!addr)
-	error ("Function return value unknown.");
-      return value_at (value_type, addr, NULL);
-    }
-
-  /* It is "struct return" yet the value is being extracted,
-     presumably from registers, using EXTRACT_RETURN_VALUE.  This
-     doesn't make sense.  Unfortunately, the legacy interfaces allowed
-     this behavior.  Sigh!  */
-  value = allocate_value (value_type);
-  CHECK_TYPEDEF (value_type);
-  /* If the function returns void, don't bother fetching the return
-     value.  */
-  EXTRACT_RETURN_VALUE (value_type, stop_registers,
-			VALUE_CONTENTS_RAW (value));
-  return value;
-}
-
 /* Print the result of a function at the end of a 'finish' command.  */
 
 static void
@@ -1115,7 +1080,9 @@ print_return_value (int struct_return, s
   struct gdbarch *gdbarch = current_gdbarch;
   struct cleanup *old_chain;
   struct ui_stream *stb;
-  struct value *value = NULL;
+  struct value *value;
+
+  gdb_assert (TYPE_CODE (value_type) != TYPE_CODE_VOID);
 
   /* FIXME: 2003-09-27: When returning from a nested inferior function
      call, it's possible (with no help from the architecture vector)
@@ -1135,13 +1102,26 @@ print_return_value (int struct_return, s
 	  gdbarch_return_value (current_gdbarch, value_type, stop_registers,
 				VALUE_CONTENTS_RAW (value), NULL);
 	  break;
-
 	case RETURN_VALUE_STRUCT_CONVENTION:
+	  value = NULL;
 	  break;
+	default:
+	  internal_error (__FILE__, __LINE__, "bad switch");
 	}
     }
+  else if (!struct_return && DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS_P ())
+    {
+      CORE_ADDR addr = DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS (stop_registers);
+      if (!addr)
+	error ("Function return value unknown.");
+      value = value_at (value_type, addr, NULL);
+    }
   else
-    value = legacy_return_value (struct_return, value_type);
+    {
+      value = allocate_value (value_type);
+      EXTRACT_RETURN_VALUE (value_type, stop_registers,
+			    VALUE_CONTENTS_RAW (value));
+    }
 
   if (value)
     {
Index: values.c
===================================================================
RCS file: /cvs/src/src/gdb/values.c,v
retrieving revision 1.65
diff -p -u -r1.65 values.c
--- values.c	8 May 2004 23:02:10 -0000	1.65
+++ values.c	10 Jun 2004 21:01:16 -0000
@@ -1202,50 +1202,6 @@ value_from_double (struct type *type, DO
   return val;
 }
 \f
-/* Deal with the return-value of a function that has "just returned".
-
-   Extract the return-value (as a "struct value") that a function,
-   using register convention, has just returned to its caller.  Assume
-   that the type of the function is VALTYPE, and that the "just
-   returned" register state is found in RETBUF.
-
-   The function has "just returned" because GDB halts a returning
-   function by setting a breakpoint at the return address (in the
-   caller), and not the return instruction (in the callee).
-
-   Because, in the case of a return from an inferior function call,
-   GDB needs to restore the inferiors registers, RETBUF is normally a
-   copy of the inferior's registers.  */
-
-struct value *
-register_value_being_returned (struct type *valtype, struct regcache *retbuf)
-{
-  struct value *val = allocate_value (valtype);
-
-  /* If the function returns void, don't bother fetching the return
-     value.  See also "using_struct_return".  */
-  if (TYPE_CODE (valtype) == TYPE_CODE_VOID)
-    return val;
-
-  if (!gdbarch_return_value_p (current_gdbarch))
-    {
-      /* NOTE: cagney/2003-10-20: Unlike "gdbarch_return_value", the
-         EXTRACT_RETURN_VALUE and USE_STRUCT_CONVENTION methods do not
-         handle the edge case of a function returning a small
-         structure / union in registers.  */
-      CHECK_TYPEDEF (valtype);
-      EXTRACT_RETURN_VALUE (valtype, retbuf, VALUE_CONTENTS_RAW (val));
-      return val;
-    }
-
-  /* This function only handles "register convention".  */
-  gdb_assert (gdbarch_return_value (current_gdbarch, valtype,
-				    NULL, NULL, NULL)
-	      == RETURN_VALUE_REGISTER_CONVENTION);
-  gdbarch_return_value (current_gdbarch, valtype, retbuf,
-			VALUE_CONTENTS_RAW (val) /*read*/, NULL /*write*/);
-  return val;
-}
 
 /* Should we use DEPRECATED_EXTRACT_STRUCT_VALUE_ADDRESS instead of
    EXTRACT_RETURN_VALUE?  GCC_P is true if compiled with gcc and TYPE
@@ -1287,7 +1243,7 @@ using_struct_return (struct type *value_
 
   if (code == TYPE_CODE_VOID)
     /* A void return value is never in memory.  See also corresponding
-       code in "register_value_being_returned".  */
+       code in "print_return_value".  */
     return 0;
 
   if (!gdbarch_return_value_p (current_gdbarch))

Re: [patch/rfc] Eliminate register_value_being_returned and legacy_return_value

[Mark Kettenis]

   Date: Thu, 10 Jun 2004 17:06:42 -0400
   From: Andrew Cagney <cagney@gnu.org>

   Hello,

   This patch eliminates calls to both register_value_being_returned and 
   legacy_return_value by inlining / simplifying the callers.  Turns out 
   there was much duplication of effort with both the caller and callees 
   all performing the same tasks.

This patch gives me a bad feeling.  I had intentionally introduced
legacy_return_value to isolate all the legacy code in a single
function.  Yes, that duplicates some of the code, but I think it makes
the code much, much easier to understand.  It also makes removing the
legacy code much more easier later on.

Mark

Re: [patch/rfc] Eliminate register_value_being_returned and legacy_return_value

[Andrew Cagney]

>    Date: Thu, 10 Jun 2004 17:06:42 -0400
>    From: Andrew Cagney <cagney@gnu.org>
> 
>    Hello,
> 
>    This patch eliminates calls to both register_value_being_returned and 
>    legacy_return_value by inlining / simplifying the callers.  Turns out 
>    there was much duplication of effort with both the caller and callees 
>    all performing the same tasks.
> 
> This patch gives me a bad feeling.  I had intentionally introduced
> legacy_return_value to isolate all the legacy code in a single
> function.  Yes, that duplicates some of the code, but I think it makes
> the code much, much easier to understand.  It also makes removing the
> legacy code much more easier later on.

Did you apply the patch and look at the result?  I think what's 
deprecated is pretty clear.

Andrew

Re: [patch/rfc] Eliminate register_value_being_returned and legacy_return_value

[Andrew Cagney]

> Did you apply the patch and look at the result?  I think what's deprecated is pretty clear.

I've checked this in.

Andrew