[PATCH]: SH 2a - Part 4: Begin using RETURN_VALUE

[PATCH]: SH 2a - Part 4: Begin using RETURN_VALUE

[Corinna Vinschen]

Hi,

this is SH 2a patch 4.  It does not introduce new functionality, but
instead it converts sh-tdep.c to use RETURN_VALUE instead of
STORE_RETURN_VALUE, EXTRACT_RETURN_VALUE, RETURN_VALUE_ON_STACK and
USE_STRUCT_CONVENTION.

The old functions are still used and just called from the RETURN_VALUE
implementations.  We have two of them, one for fpu and one for nofpu
types.

What this patch does *not* do is this:  I would love to rename the
functions sh_default_store/extract_value and sh3e_sh4_store/extract_value
to sh_store/extract_value_nofpu and sh_store/extract_value_fpu.
This would decouple the sense of these functions from the cpu names,
which is rather irritating and wrong anyway.  If that's ok, I'd
submit another patch which just performs the renaming.

Ok, for the sake of completeness, here are the testresults of various
SH cpu types, tested on GNU/Linux host and sh-sim, before and after
applying the whole set of SH 2a patches.  I hope the choice of CPUs is
representative enough:

                       BEFORE  AFTER
                       -----   -----
sh-sim/-m2a              --      88
sh-sim/-m2a-nofpu        --      79
sh-sim/-m2e             100     100
sh-sim/-m3               79      79
sh-sim/-m4               88      88
sh-sim/-m4/-ml           86      86
sh-sim/-m4a-single       88      88


Corinna

	* sh-tdep.c (sh_return_value_nofpu): New function, implementing
	RETURN_VALUE.
	(sh_return_value_fpu): Ditto for CPUs with FPU.
	(sh_gdbarch_init): Remove STORE_RETURN_VALUE, EXTRACT_RETURN_VALUE,
	RETURN_VALUE_ON_STACK and USE_STRUCT_CONVENTION throughout.
	Install sh_return_value_nofpu resp. sh_return_value_fpu as
	RETURN_VALUE functions.

--- sh-tdep.c.3	2004-09-08 18:46:55.000000000 +0200
+++ sh-tdep.c	2004-09-08 18:47:01.000000000 +0200
@@ -1281,6 +1281,34 @@ sh3e_sh4_store_return_value (struct type
     sh_default_store_return_value (type, regcache, valbuf);
 }
 
+static enum return_value_convention
+sh_return_value_nofpu (struct gdbarch *gdbarch, struct type *type,
+		       struct regcache *regcache,
+		       void *readbuf, const void *writebuf)
+{
+  if (sh_use_struct_convention (0, type))
+    return RETURN_VALUE_STRUCT_CONVENTION;
+  if (writebuf)
+    sh_default_store_return_value (type, regcache, writebuf);
+  else if (readbuf)
+    sh_default_extract_return_value (type, regcache, readbuf);
+  return RETURN_VALUE_REGISTER_CONVENTION;
+}
+
+static enum return_value_convention
+sh_return_value_fpu (struct gdbarch *gdbarch, struct type *type,
+		     struct regcache *regcache,
+		     void *readbuf, const void *writebuf)
+{
+  if (sh_use_struct_convention (0, type))
+    return RETURN_VALUE_STRUCT_CONVENTION;
+  if (writebuf)
+    sh3e_sh4_store_return_value (type, regcache, writebuf);
+  else if (readbuf)
+    sh3e_sh4_extract_return_value (type, regcache, readbuf);
+  return RETURN_VALUE_REGISTER_CONVENTION;
+}
+
 /* Print the registers in a form similar to the E7000 */
 
 static void
@@ -2564,16 +2592,15 @@ sh_gdbarch_init (struct gdbarch_info inf
   set_gdbarch_print_registers_info (gdbarch, sh_print_registers_info);
 
   set_gdbarch_breakpoint_from_pc (gdbarch, sh_breakpoint_from_pc);
-  set_gdbarch_deprecated_use_struct_convention (gdbarch, sh_use_struct_convention);
 
   set_gdbarch_print_insn (gdbarch, gdb_print_insn_sh);
   set_gdbarch_register_sim_regno (gdbarch, legacy_register_sim_regno);
 
   set_gdbarch_write_pc (gdbarch, generic_target_write_pc);
 
-  set_gdbarch_store_return_value (gdbarch, sh_default_store_return_value);
-  set_gdbarch_extract_return_value (gdbarch, sh_default_extract_return_value);
-  set_gdbarch_deprecated_extract_struct_value_address (gdbarch, sh_extract_struct_value_address);
+  set_gdbarch_return_value (gdbarch, sh_return_value_nofpu);
+  set_gdbarch_deprecated_extract_struct_value_address (gdbarch,
+					    sh_extract_struct_value_address);
 
   set_gdbarch_skip_prologue (gdbarch, sh_skip_prologue);
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
@@ -2607,9 +2634,7 @@ sh_gdbarch_init (struct gdbarch_info inf
       set_gdbarch_register_name (gdbarch, sh_sh2e_register_name);
       set_gdbarch_register_type (gdbarch, sh_sh3e_register_type);
       set_gdbarch_fp0_regnum (gdbarch, 25);
-      set_gdbarch_store_return_value (gdbarch, sh3e_sh4_store_return_value);
-      set_gdbarch_extract_return_value (gdbarch,
-					sh3e_sh4_extract_return_value);
+      set_gdbarch_return_value (gdbarch, sh_return_value_fpu);
       set_gdbarch_push_dummy_call (gdbarch, sh_push_dummy_call_fpu);
       break;
 
@@ -2622,8 +2647,7 @@ sh_gdbarch_init (struct gdbarch_info inf
       set_gdbarch_num_pseudo_regs (gdbarch, 9);
       set_gdbarch_pseudo_register_read (gdbarch, sh_pseudo_register_read);
       set_gdbarch_pseudo_register_write (gdbarch, sh_pseudo_register_write);
-      set_gdbarch_store_return_value (gdbarch, sh3e_sh4_store_return_value);
-      set_gdbarch_extract_return_value (gdbarch, sh3e_sh4_extract_return_value);
+      set_gdbarch_return_value (gdbarch, sh_return_value_fpu);
       set_gdbarch_push_dummy_call (gdbarch, sh_push_dummy_call_fpu);
       break;
 
@@ -2652,9 +2676,7 @@ sh_gdbarch_init (struct gdbarch_info inf
       set_gdbarch_register_name (gdbarch, sh_sh3e_register_name);
       set_gdbarch_register_type (gdbarch, sh_sh3e_register_type);
       set_gdbarch_fp0_regnum (gdbarch, 25);
-      set_gdbarch_store_return_value (gdbarch, sh3e_sh4_store_return_value);
-      set_gdbarch_extract_return_value (gdbarch,
-					sh3e_sh4_extract_return_value);
+      set_gdbarch_return_value (gdbarch, sh_return_value_fpu);
       set_gdbarch_push_dummy_call (gdbarch, sh_push_dummy_call_fpu);
       break;
 
@@ -2671,9 +2693,7 @@ sh_gdbarch_init (struct gdbarch_info inf
       set_gdbarch_num_pseudo_regs (gdbarch, 13);
       set_gdbarch_pseudo_register_read (gdbarch, sh_pseudo_register_read);
       set_gdbarch_pseudo_register_write (gdbarch, sh_pseudo_register_write);
-      set_gdbarch_store_return_value (gdbarch, sh3e_sh4_store_return_value);
-      set_gdbarch_extract_return_value (gdbarch,
-					sh3e_sh4_extract_return_value);
+      set_gdbarch_return_value (gdbarch, sh_return_value_fpu);
       set_gdbarch_push_dummy_call (gdbarch, sh_push_dummy_call_fpu);
       break;
 
-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat, Inc.

Re: [PATCH]: SH 2a - Part 4: Begin using RETURN_VALUE

[Elena Zannoni]

Corinna Vinschen writes:
 > Hi,
 > 
 > this is SH 2a patch 4.  It does not introduce new functionality, but
 > instead it converts sh-tdep.c to use RETURN_VALUE instead of
 > STORE_RETURN_VALUE, EXTRACT_RETURN_VALUE, RETURN_VALUE_ON_STACK and
 > USE_STRUCT_CONVENTION.
 > 
 > The old functions are still used and just called from the RETURN_VALUE
 > implementations.  We have two of them, one for fpu and one for nofpu
 > types.
 > 

OK

 > What this patch does *not* do is this:  I would love to rename the
 > functions sh_default_store/extract_value and sh3e_sh4_store/extract_value
 > to sh_store/extract_value_nofpu and sh_store/extract_value_fpu.
 > This would decouple the sense of these functions from the cpu names,
 > which is rather irritating and wrong anyway.  If that's ok, I'd
 > submit another patch which just performs the renaming.
 >

It wasn't wrong and irritating at the time it was introduced. 
 
 > Ok, for the sake of completeness, here are the testresults of various
 > SH cpu types, tested on GNU/Linux host and sh-sim, before and after
 > applying the whole set of SH 2a patches.  I hope the choice of CPUs is
 > representative enough:
 > 
 >                        BEFORE  AFTER
 >                        -----   -----
 > sh-sim/-m2a              --      88
 > sh-sim/-m2a-nofpu        --      79
 > sh-sim/-m2e             100     100
 > sh-sim/-m3               79      79
 > sh-sim/-m4               88      88
 > sh-sim/-m4/-ml           86      86
 > sh-sim/-m4a-single       88      88
 > 

Thanks for providing the test results. Are these with
sources.redhat.com sources? Or some internal only tree? What version
of gcc? So we have a good reference point.

elena

Re: [PATCH]: SH 2a - Part 4: Begin using RETURN_VALUE

[Corinna Vinschen]

On Oct  5 10:11, Elena Zannoni wrote:
> Corinna Vinschen writes:
>  > this is SH 2a patch 4.  It does not introduce new functionality, but
>  > instead it converts sh-tdep.c to use RETURN_VALUE instead of
>  > STORE_RETURN_VALUE, EXTRACT_RETURN_VALUE, RETURN_VALUE_ON_STACK and
>  > USE_STRUCT_CONVENTION.
> 
> OK

Thanks, applied.

>  > What this patch does *not* do is this:  I would love to rename the
>  > functions sh_default_store/extract_value and sh3e_sh4_store/extract_value
>  > to sh_store/extract_value_nofpu and sh_store/extract_value_fpu.
>  > This would decouple the sense of these functions from the cpu names,
>  > which is rather irritating and wrong anyway.  If that's ok, I'd
>  > submit another patch which just performs the renaming.
> 
> It wasn't wrong and irritating at the time it was introduced. 

Yes, that's apparently right.  As I wrote, "I would love to...", because
time has gone a bit over that naming scheme and I think, changing the
names to reflect their usage in a more general way would help when reading
the code in future.

Is it ok to rename them?


>  >                        BEFORE  AFTER
> 
> Thanks for providing the test results. Are these with
> sources.redhat.com sources? Or some internal only tree? What version
> of gcc? So we have a good reference point.

I used sources sources (erm...) to evaluate these results, exactly
CVS HEAD from 2004-09-08.  GCC was a 3.4 with SH2a patches applied.


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat, Inc.

Re: [PATCH]: SH 2a - Part 4: Begin using RETURN_VALUE

[Elena Zannoni]

Corinna Vinschen writes:
 > 
 > Yes, that's apparently right.  As I wrote, "I would love to...", because
 > time has gone a bit over that naming scheme and I think, changing the
 > names to reflect their usage in a more general way would help when reading
 > the code in future.
 > 
 > Is it ok to rename them?

Sure, send a patch.

[PATCH] sh-tdep.c: Rename store/extract functions for better readability (was Re: [PATCH]: SH 2a - Part 4: Begin using RETURN_VALUE)

[Corinna Vinschen]

On Oct  6 08:20, Elena Zannoni wrote:
> Corinna Vinschen writes:
>  > Is it ok to rename them?
> 
> Sure, send a patch.

Cool, see below.

Corinna

	* sh-tdep.c (sh_extract_return_value_nofpu): Rename from
	sh_default_extract_return_value.
	(sh_extract_return_value_fpu): Rename from
	sh3e_sh4_extract_return_value.
	(sh_store_return_value_nofpu): Rename from
	sh_default_store_return_value.
	(sh_store_return_value_fpu): Rename from sh3e_sh4_store_return_value.
	(sh_return_value_nofpu): Accomodate above changes.
	(sh_return_value_fpu): Ditto.

Index: sh-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sh-tdep.c,v
retrieving revision 1.177
diff -u -p -r1.177 sh-tdep.c
--- sh-tdep.c	6 Oct 2004 08:59:02 -0000	1.177
+++ sh-tdep.c	6 Oct 2004 12:44:08 -0000
@@ -1194,8 +1194,8 @@ sh_push_dummy_call_nofpu (struct gdbarch
    containing the (raw) register state a function return value of type
    TYPE, and copy that, in virtual format, into VALBUF.  */
 static void
-sh_default_extract_return_value (struct type *type, struct regcache *regcache,
-				 void *valbuf)
+sh_extract_return_value_nofpu (struct type *type, struct regcache *regcache,
+			       void *valbuf)
 {
   int len = TYPE_LENGTH (type);
   int return_register = R0_REGNUM;
@@ -1219,8 +1219,8 @@ sh_default_extract_return_value (struct 
 }
 
 static void
-sh3e_sh4_extract_return_value (struct type *type, struct regcache *regcache,
-			       void *valbuf)
+sh_extract_return_value_fpu (struct type *type, struct regcache *regcache,
+			     void *valbuf)
 {
   if (sh_treat_as_flt_p (type))
     {
@@ -1233,7 +1233,7 @@ sh3e_sh4_extract_return_value (struct ty
 	  regcache_raw_read (regcache, regnum++, (char *) valbuf + i);
     }
   else
-    sh_default_extract_return_value (type, regcache, valbuf);
+    sh_extract_return_value_nofpu (type, regcache, valbuf);
 }
 
 /* Write into appropriate registers a function return value
@@ -1243,8 +1243,8 @@ sh3e_sh4_extract_return_value (struct ty
    depending on the type of the return value. In all the other cases
    the result is stored in r0, left-justified. */
 static void
-sh_default_store_return_value (struct type *type, struct regcache *regcache,
-			       const void *valbuf)
+sh_store_return_value_nofpu (struct type *type, struct regcache *regcache,
+			     const void *valbuf)
 {
   ULONGEST val;
   int len = TYPE_LENGTH (type);
@@ -1263,8 +1263,8 @@ sh_default_store_return_value (struct ty
 }
 
 static void
-sh3e_sh4_store_return_value (struct type *type, struct regcache *regcache,
-			     const void *valbuf)
+sh_store_return_value_fpu (struct type *type, struct regcache *regcache,
+			   const void *valbuf)
 {
   if (sh_treat_as_flt_p (type))
     {
@@ -1278,7 +1278,7 @@ sh3e_sh4_store_return_value (struct type
 	  regcache_raw_write (regcache, regnum++, (char *) valbuf + i);
     }
   else
-    sh_default_store_return_value (type, regcache, valbuf);
+    sh_store_return_value_nofpu (type, regcache, valbuf);
 }
 
 static enum return_value_convention
@@ -1289,9 +1289,9 @@ sh_return_value_nofpu (struct gdbarch *g
   if (sh_use_struct_convention (0, type))
     return RETURN_VALUE_STRUCT_CONVENTION;
   if (writebuf)
-    sh_default_store_return_value (type, regcache, writebuf);
+    sh_store_return_value_nofpu (type, regcache, writebuf);
   else if (readbuf)
-    sh_default_extract_return_value (type, regcache, readbuf);
+    sh_extract_return_value_nofpu (type, regcache, readbuf);
   return RETURN_VALUE_REGISTER_CONVENTION;
 }
 
@@ -1303,9 +1303,9 @@ sh_return_value_fpu (struct gdbarch *gdb
   if (sh_use_struct_convention (0, type))
     return RETURN_VALUE_STRUCT_CONVENTION;
   if (writebuf)
-    sh3e_sh4_store_return_value (type, regcache, writebuf);
+    sh_store_return_value_fpu (type, regcache, writebuf);
   else if (readbuf)
-    sh3e_sh4_extract_return_value (type, regcache, readbuf);
+    sh_extract_return_value_fpu (type, regcache, readbuf);
   return RETURN_VALUE_REGISTER_CONVENTION;
 }
 
-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat, Inc.

Re: [PATCH] sh-tdep.c: Rename store/extract functions for better readability (was Re: [PATCH]: SH 2a - Part 4: Begin using RETURN_VALUE)

[Elena Zannoni]

Corinna Vinschen writes:
 > On Oct  6 08:20, Elena Zannoni wrote:
 > > Corinna Vinschen writes:
 > >  > Is it ok to rename them?
 > > 
 > > Sure, send a patch.
 > 
 > Cool, see below.
 > 
 > Corinna
 > 
 > 	* sh-tdep.c (sh_extract_return_value_nofpu): Rename from
 > 	sh_default_extract_return_value.
 > 	(sh_extract_return_value_fpu): Rename from
 > 	sh3e_sh4_extract_return_value.
 > 	(sh_store_return_value_nofpu): Rename from
 > 	sh_default_store_return_value.
 > 	(sh_store_return_value_fpu): Rename from sh3e_sh4_store_return_value.
 > 	(sh_return_value_nofpu): Accomodate above changes.
 > 	(sh_return_value_fpu): Ditto.


OK

Re: [PATCH] sh-tdep.c: Rename store/extract functions for better readability (was Re: [PATCH]: SH 2a - Part 4: Begin using RETURN_VALUE)

[Corinna Vinschen]

On Oct 21 23:21, Elena Zannoni wrote:
> Corinna Vinschen writes:
>  > On Oct  6 08:20, Elena Zannoni wrote:
>  > > Corinna Vinschen writes:
>  > >  > Is it ok to rename them?
>  > > 
>  > > Sure, send a patch.
>  > 
>  > Cool, see below.
>  > 
>  > Corinna
>  > 
>  > 	* sh-tdep.c (sh_extract_return_value_nofpu): Rename from
>  > 	sh_default_extract_return_value.
>  > 	(sh_extract_return_value_fpu): Rename from
>  > 	sh3e_sh4_extract_return_value.
>  > 	(sh_store_return_value_nofpu): Rename from
>  > 	sh_default_store_return_value.
>  > 	(sh_store_return_value_fpu): Rename from sh3e_sh4_store_return_value.
>  > 	(sh_return_value_nofpu): Accomodate above changes.
>  > 	(sh_return_value_fpu): Ditto.
> 
> OK

Applied.

Thanks,
Corinna