[m68k] correct m68k_convert_register_p for coldfire

[m68k] correct m68k_convert_register_p for coldfire

[Vladimir Prus]

[-- Attachment #1: Type: Text/Plain, Size: 610 bytes --]


The m68k_register_type function will return different types for
fp0 depending on whether we're on Coldfire, or classic m68k.
However, m68k_convert_register_p contains code such as:

	static int
	m68k_convert_register_p (struct gdbarch *gdbarch, int regnum, struct type *type)
	{
	  if (!gdbarch_tdep (gdbarch)->fpregs_present)
    	return 0;
	  return (regnum >= M68K_FP0_REGNUM && regnum <= M68K_FP0_REGNUM + 7
		  && type != m68881_ext_type (gdbarch));
	}

and therefore will return true on Coldfire. The attached patch addresses
this. Is it OK (with the name of changelog file adjusted)?


Thanks,
Volodya

[-- Attachment #2: m68k_convert_register_p.diff --]
[-- Type: text/x-patch, Size: 947 bytes --]

Index: gdb/m68k-tdep.c
===================================================================
--- gdb/m68k-tdep.c	(revision 219692)
+++ gdb/m68k-tdep.c	(revision 219693)
@@ -177,7 +177,7 @@
   if (!gdbarch_tdep (gdbarch)->fpregs_present)
     return 0;
   return (regnum >= M68K_FP0_REGNUM && regnum <= M68K_FP0_REGNUM + 7
-	  && type != builtin_type_m68881_ext);
+	  && type != register_type (current_gdbarch, M68K_FP0_REGNUM));
 }
 
 /* Read a value of type TYPE from register REGNUM in frame FRAME, and
Index: ChangeLog.csl
===================================================================
--- ChangeLog.csl	(revision 219692)
+++ ChangeLog.csl	(revision 219693)
@@ -1,3 +1,10 @@
+2008-08-31  Vladimir Prus  <vladimir@codesourcery.com>
+
+	gdb/
+	* m68k-tdep.c (m68k_convert_register_p): Compare with
+	the actual type of fp registers, not one of the possible
+	values.
+	
 2008-08-27  Julian Brown  <julian@codesourcery.com>
 
 	gdb/sim/common/

Re: [m68k] correct m68k_convert_register_p for coldfire

[Mark Kettenis]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Fri, 6 Nov 2009 16:04:18 +0300
> 
> The m68k_register_type function will return different types for
> fp0 depending on whether we're on Coldfire, or classic m68k.
> However, m68k_convert_register_p contains code such as:
> 
> 	static int
> 	m68k_convert_register_p (struct gdbarch *gdbarch, int regnum, struct type *type)
> 	{
> 	  if (!gdbarch_tdep (gdbarch)->fpregs_present)
>     	return 0;
> 	  return (regnum >= M68K_FP0_REGNUM && regnum <= M68K_FP0_REGNUM + 7
> 		  && type != m68881_ext_type (gdbarch));
> 	}
> 
> and therefore will return true on Coldfire. The attached patch addresses
> this. Is it OK (with the name of changelog file adjusted)?

Looks ok to me.

Re: [m68k] correct m68k_convert_register_p for coldfire

[Pedro Alves]

On Friday 06 November 2009 13:04:18, Vladimir Prus wrote:
>   Index: gdb/m68k-tdep.c
> ===================================================================
> --- gdb/m68k-tdep.c     (revision 219692)
> +++ gdb/m68k-tdep.c     (revision 219693)
> @@ -177,7 +177,7 @@
>    if (!gdbarch_tdep (gdbarch)->fpregs_present)
>      return 0;
>    return (regnum >= M68K_FP0_REGNUM && regnum <= M68K_FP0_REGNUM + 7
> -         && type != builtin_type_m68881_ext);
> +         && type != register_type (current_gdbarch, M68K_FP0_REGNUM));
>  }
>  

Please don't apply this as is without at least trying to
build it...  current_gdbarch doesn't exist anymore.

-- 
Pedro Alves

Re: [m68k] correct m68k_convert_register_p for coldfire

[Vladimir Prus]

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

Pedro Alves wrote:

> On Friday 06 November 2009 13:04:18, Vladimir Prus wrote:
>>   Index: gdb/m68k-tdep.c
>> ===================================================================
>> --- gdb/m68k-tdep.c     (revision 219692)
>> +++ gdb/m68k-tdep.c     (revision 219693)
>> @@ -177,7 +177,7 @@
>> if (!gdbarch_tdep (gdbarch)->fpregs_present)
>> return 0;
>> return (regnum >= M68K_FP0_REGNUM && regnum <= M68K_FP0_REGNUM + 7
>> -         && type != builtin_type_m68881_ext);
>> +         && type != register_type (current_gdbarch, M68K_FP0_REGNUM));
>> }
>> 
> 
> Please don't apply this as is without at least trying to
> build it...  current_gdbarch doesn't exist anymore.

Doh! Here's what I've ended up checking it.

- Volodya



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: final.diff --]
[-- Type: text/x-diff; name="final.diff", Size: 1310 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.11036
diff -u -p -r1.11036 ChangeLog
--- gdb/ChangeLog	6 Nov 2009 14:26:09 -0000	1.11036
+++ gdb/ChangeLog	6 Nov 2009 17:21:57 -0000
@@ -1,5 +1,11 @@
 2009-11-06  Vladimir Prus  <vladimir@codesourcery.com>
 
+	* m68k-tdep.c (m68k_convert_register_p): Compare with
+	the actual type of fp registers, not one of the possible
+	values.
+
+2009-11-06  Vladimir Prus  <vladimir@codesourcery.com>
+
         Prevent program output from mix with "^running".
 
 	gdb/
Index: gdb/m68k-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m68k-tdep.c,v
retrieving revision 1.145
diff -u -p -r1.145 m68k-tdep.c
--- gdb/m68k-tdep.c	7 Sep 2009 17:52:41 -0000	1.145
+++ gdb/m68k-tdep.c	6 Nov 2009 17:21:57 -0000
@@ -192,7 +192,8 @@ m68k_convert_register_p (struct gdbarch 
   if (!gdbarch_tdep (gdbarch)->fpregs_present)
     return 0;
   return (regnum >= M68K_FP0_REGNUM && regnum <= M68K_FP0_REGNUM + 7
-	  && type != m68881_ext_type (gdbarch));
+	  && type != m68881_ext_type (gdbarch)
+	  && type != register_type (gdbarch, M68K_FP0_REGNUM));
 }
 
 /* Read a value of type TYPE from register REGNUM in frame FRAME, and

Re: [m68k] correct m68k_convert_register_p for coldfire

[Daniel Jacobowitz]

On Fri, Nov 06, 2009 at 08:24:47PM +0300, Vladimir Prus wrote:
> -	  && type != m68881_ext_type (gdbarch));
> +	  && type != m68881_ext_type (gdbarch)
> +	  && type != register_type (gdbarch, M68K_FP0_REGNUM));

Did you mean to leave both?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [m68k] correct m68k_convert_register_p for coldfire

[Vladimir Prus]

On Friday 06 November 2009 Daniel Jacobowitz wrote:

> On Fri, Nov 06, 2009 at 08:24:47PM +0300, Vladimir Prus wrote:
> > -	  && type != m68881_ext_type (gdbarch));
> > +	  && type != m68881_ext_type (gdbarch)
> > +	  && type != register_type (gdbarch, M68K_FP0_REGNUM));
> 
> Did you mean to leave both?

Sigh, today is not the best day. I've checked in a follow-up:

	http://sourceware.org/ml/gdb-cvs/2009-11/msg00044.html

- Volodya

Re: [m68k] correct m68k_convert_register_p for coldfire

[Mark Kettenis]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date:  Fri, 06 Nov 2009 20:24:47 +0300
> > Please don't apply this as is without at least trying to
> > build it...  current_gdbarch doesn't exist anymore.
> 
> Doh! Here's what I've ended up checking it.

But that doesn't make any sense.  On a real m68k,
register_type(gdbarch, M68K_FP0_REGNUM) will return
m68881_ext_type(gdbarch) and on coldfire, type !=
m68881_ext_type(gdbarch) is probably always true.

> Index: gdb/ChangeLog
> ===================================================================
> RCS file: /cvs/src/src/gdb/ChangeLog,v
> retrieving revision 1.11036
> diff -u -p -r1.11036 ChangeLog
> --- gdb/ChangeLog	6 Nov 2009 14:26:09 -0000	1.11036
> +++ gdb/ChangeLog	6 Nov 2009 17:21:57 -0000
> @@ -1,5 +1,11 @@
>  2009-11-06  Vladimir Prus  <vladimir@codesourcery.com>
>  
> +	* m68k-tdep.c (m68k_convert_register_p): Compare with
> +	the actual type of fp registers, not one of the possible
> +	values.
> +
> +2009-11-06  Vladimir Prus  <vladimir@codesourcery.com>
> +
>          Prevent program output from mix with "^running".
>  
>  	gdb/
> Index: gdb/m68k-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/m68k-tdep.c,v
> retrieving revision 1.145
> diff -u -p -r1.145 m68k-tdep.c
> --- gdb/m68k-tdep.c	7 Sep 2009 17:52:41 -0000	1.145
> +++ gdb/m68k-tdep.c	6 Nov 2009 17:21:57 -0000
> @@ -192,7 +192,8 @@ m68k_convert_register_p (struct gdbarch 
>    if (!gdbarch_tdep (gdbarch)->fpregs_present)
>      return 0;
>    return (regnum >= M68K_FP0_REGNUM && regnum <= M68K_FP0_REGNUM + 7
> -	  && type != m68881_ext_type (gdbarch));
> +	  && type != m68881_ext_type (gdbarch)
> +	  && type != register_type (gdbarch, M68K_FP0_REGNUM));
>  }
>  
>  /* Read a value of type TYPE from register REGNUM in frame FRAME, and
> 

Re: [m68k] correct m68k_convert_register_p for coldfire

[Vladimir Prus]

On Saturday 07 November 2009 Mark Kettenis wrote:

> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date:  Fri, 06 Nov 2009 20:24:47 +0300
> > > Please don't apply this as is without at least trying to
> > > build it...  current_gdbarch doesn't exist anymore.
> > 
> > Doh! Here's what I've ended up checking it.
> 
> But that doesn't make any sense.  On a real m68k,
> register_type(gdbarch, M68K_FP0_REGNUM) will return
> m68881_ext_type(gdbarch) and on coldfire, type !=
> m68881_ext_type(gdbarch) is probably always true.

Does the correction in

	http://sourceware.org/ml/gdb-cvs/2009-11/msg00044.html

make the code sensible?

- Volodya

Re: [m68k] correct m68k_convert_register_p for coldfire

[Mark Kettenis]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sat, 7 Nov 2009 14:02:19 +0300
> 
> On Saturday 07 November 2009 Mark Kettenis wrote:
> 
> > > From: Vladimir Prus <vladimir@codesourcery.com>
> > > Date:  Fri, 06 Nov 2009 20:24:47 +0300
> > > > Please don't apply this as is without at least trying to
> > > > build it...  current_gdbarch doesn't exist anymore.
> > > 
> > > Doh! Here's what I've ended up checking it.
> > 
> > But that doesn't make any sense.  On a real m68k,
> > register_type(gdbarch, M68K_FP0_REGNUM) will return
> > m68881_ext_type(gdbarch) and on coldfire, type !=
> > m68881_ext_type(gdbarch) is probably always true.
> 
> Does the correction in
> 
> 	http://sourceware.org/ml/gdb-cvs/2009-11/msg00044.html
> 
> make the code sensible?

It does.  Thanks.