[patch] Add support for VFP d16 layout for Cortex-M4

[patch] Add support for VFP d16 layout for Cortex-M4

[Jonathan Larmour]

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

As mentioned in my response to the GDB list mail
<http://sourceware.org/ml/gdb/2012-04/msg00149.html>, I have a patch to
allow easy automatic use of Cortex-M targets with a register layout with
16 double precision / 32 single precision regs. This is as used in the
increasingly popular Cortex-M4 core.

I have chosen to use the remote packet guessing approach again, rather
than inferring from the executable's ELF header, because not everyone will
actually want to use hardware FP just because they're using that core, so
what the stub actually supports is a better indicator of what is wanted.

There's a couple of trivial little fixes from earlier changes too (a
comment and missing entries in features/Makefile). FAOD I have
write-after-approval commit perms.

Jifl


2012-04-19  Jonathan Larmour  <jifl@eCosCentric.com>

	* arm-dep.h (VFP_REGISTER_SIZE): Define.
	* features/Makefile: Add arm-with-m* entries currently missing.
	* features/arm-with-m-vfp-d16.xml: New file. Describes
	Cortex-M with VFPv4-sp-d16 FPU register layout.
	* features/arm-with-m-vfp-d16.c: Generate from above.
	* arm-tdep.c: Include arm-with-m-vfp-d16.c.
	(arm-register_g_packet_guesses): Fix comment. Add vfp-d16 guess.
	(_initialise_arm_tdep): Initialize arm-with-m-vfp-d16 tdesc.


arm-tdep.c                      |   12 ++++++++
arm-tdep.h                      |    4 ++
features/Makefile               |    3 +-
features/arm-with-m-vfp-d16.c   |   54
++++++++++++++++++++++++++++++++++++++++
features/arm-with-m-vfp-d16.xml |   13 +++++++++
5 files changed, 84 insertions(+), 2 deletions(-)

-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
  **   Visit us at NEW:UK - the UK Embedded & Electronics Exhibition  **
  **   ARM Partner Pavilion/Stand #114. 18-19 April. Birmingham NEC   **
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

[-- Attachment #2: vfpd16.patch --]
[-- Type: text/plain, Size: 8020 bytes --]

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.360
diff -u -5 -p -r1.360 arm-tdep.c
--- arm-tdep.c	27 Mar 2012 15:46:33 -0000	1.360
+++ arm-tdep.c	19 Apr 2012 14:19:39 -0000
@@ -57,10 +57,11 @@
 
 #include "record.h"
 
 #include "features/arm-with-m.c"
 #include "features/arm-with-m-fpa-layout.c"
+#include "features/arm-with-m-vfp-d16.c"
 #include "features/arm-with-iwmmxt.c"
 #include "features/arm-with-vfpv2.c"
 #include "features/arm-with-vfpv3.c"
 #include "features/arm-with-neon.c"
 
@@ -9684,11 +9685,11 @@ arm_register_g_packet_guesses (struct gd
     {
       /* If we know from the executable this is an M-profile target,
 	 cater for remote targets whose register set layout is the
 	 same as the FPA layout.  */
       register_remote_g_packet_guess (gdbarch,
-				      /* r0-r12,sp,lr,pc; f0-f7; fps,cpsr */
+				      /* r0-r12,sp,lr,pc; f0-f7; fps,xpsr */
 				      (16 * INT_REGISTER_SIZE)
 				      + (8 * FP_REGISTER_SIZE)
 				      + (2 * INT_REGISTER_SIZE),
 				      tdesc_arm_with_m_fpa_layout);
 
@@ -9696,10 +9697,18 @@ arm_register_g_packet_guesses (struct gd
       register_remote_g_packet_guess (gdbarch,
 				      /* r0-r12,sp,lr,pc; xpsr */
 				      (16 * INT_REGISTER_SIZE)
 				      + INT_REGISTER_SIZE,
 				      tdesc_arm_with_m);
+
+      /* M-profile plus M4F VFP.  */
+      register_remote_g_packet_guess (gdbarch,
+				      /* r0-r12,sp,lr,pc; d0-d15; fpscr,xpsr */
+				      (16 * INT_REGISTER_SIZE)
+				      + (16 * VFP_REGISTER_SIZE)
+				      + (2 * INT_REGISTER_SIZE),
+				      tdesc_arm_with_m_vfp_d16);
     }
 
   /* Otherwise we don't have a useful guess.  */
 }
 
@@ -10331,10 +10340,11 @@ _initialize_arm_tdep (void)
 				  arm_elf_osabi_sniffer);
 
   /* Initialize the standard target descriptions.  */
   initialize_tdesc_arm_with_m ();
   initialize_tdesc_arm_with_m_fpa_layout ();
+  initialize_tdesc_arm_with_m_vfp_d16 ();
   initialize_tdesc_arm_with_iwmmxt ();
   initialize_tdesc_arm_with_vfpv2 ();
   initialize_tdesc_arm_with_vfpv3 ();
   initialize_tdesc_arm_with_neon ();
 
Index: arm-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.h,v
retrieving revision 1.56
diff -u -5 -p -r1.56 arm-tdep.h
--- arm-tdep.h	27 Mar 2012 15:46:33 -0000	1.56
+++ arm-tdep.h	19 Apr 2012 14:19:39 -0000
@@ -69,10 +69,14 @@ enum gdb_regnum {
 /* Say how long FP registers are.  Used for documentation purposes and
    code readability in this header.  IEEE extended doubles are 80
    bits.  DWORD aligned they use 96 bits.  */
 #define FP_REGISTER_SIZE	12
 
+/* Say how long VFP double precision registers are. Used for documentation
+   purposes and code readability.  These are fixed at 64 bits. */
+#define VFP_REGISTER_SIZE	8
+
 /* Number of machine registers.  The only define actually required 
    is gdbarch_num_regs.  The other definitions are used for documentation
    purposes and code readability.  */
 /* For 26 bit ARM code, a fake copy of the PC is placed in register 25 (PS)
    (and called PS for processor status) so the status bits can be cleared
Index: features/Makefile
===================================================================
RCS file: /cvs/src/src/gdb/features/Makefile,v
retrieving revision 1.26
diff -u -5 -p -r1.26 Makefile
--- features/Makefile	10 Apr 2012 15:44:23 -0000	1.26
+++ features/Makefile	19 Apr 2012 14:19:39 -0000
@@ -28,11 +28,12 @@
 # XML master copies.  This relies on a GDB linked with expat and
 # configured for the correct architecture, so the files are again kept
 # in the GDB repository.  To generate C files:
 #   make GDB=/path/to/gdb XMLTOC="xml files" cfiles
 
-WHICH = arm-with-iwmmxt arm-with-vfpv2 arm-with-vfpv3 arm-with-neon \
+WHICH = arm-with-iwmmxt arm-with-vfpv2 arm-with-vfpv3 \
+	arm-with-m arm-with-m-fpa-layout arm-with-m-vfp-d16 arm-with-neon \
 	i386/i386 i386/i386-linux \
 	i386/i386-mmx i386/i386-mmx-linux \
 	i386/amd64 i386/amd64-linux \
 	i386/i386-avx i386/i386-avx-linux \
 	i386/amd64-avx i386/amd64-avx-linux \
Index: features/arm-with-m-vfp-d16.c
===================================================================
RCS file: features/arm-with-m-vfp-d16.c
diff -N features/arm-with-m-vfp-d16.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ features/arm-with-m-vfp-d16.c	19 Apr 2012 14:19:39 -0000
@@ -0,0 +1,54 @@
+/* THIS FILE IS GENERATED.  Original: arm-with-m-vfp-d16.xml */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+struct target_desc *tdesc_arm_with_m_vfp_d16;
+static void
+initialize_tdesc_arm_with_m_vfp_d16 (void)
+{
+  struct target_desc *result = allocate_target_description ();
+  struct tdesc_feature *feature;
+  struct tdesc_type *field_type, *type;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arm.m-profile");
+  tdesc_create_reg (feature, "r0", 0, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r1", 1, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r2", 2, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r3", 3, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r4", 4, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r5", 5, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r6", 6, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r7", 7, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r8", 8, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r9", 9, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r10", 10, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r11", 11, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r12", 12, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "sp", 13, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "lr", 14, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "pc", 15, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "xpsr", 25, 1, NULL, 32, "int");
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arm.vfp");
+  tdesc_create_reg (feature, "d0", 26, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d1", 27, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d2", 28, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d3", 29, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d4", 30, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d5", 31, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d6", 32, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d7", 33, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d8", 34, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d9", 35, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d10", 36, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d11", 37, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d12", 38, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d13", 39, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d14", 40, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d15", 41, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "fpscr", 42, 1, "float", 32, "int");
+
+  tdesc_arm_with_m_vfp_d16 = result;
+}
Index: features/arm-with-m-vfp-d16.xml
===================================================================
RCS file: features/arm-with-m-vfp-d16.xml
diff -N features/arm-with-m-vfp-d16.xml
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ features/arm-with-m-vfp-d16.xml	19 Apr 2012 14:19:39 -0000
@@ -0,0 +1,13 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2010-2012 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <xi:include href="arm-m-profile.xml"/>
+  <!-- Layout of vfpv4-sp-d16 is identical to vfpv2 -->
+  <xi:include href="arm-vfpv2.xml"/>
+</target>

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Terry Guo]

Hi Jonathan,

This is Terry and I am using my gmail cause I am at home.

I support this patch and your idea that using g_packet_guess is
smarter and more flexible. I will try your patch out when I am at
office tomorrow. What kind of board and gdb stub are you using?
Currently I only have a STM32F4 board in hand but couldn't find a
suitable gdb stub to access its FPU registers. Such functionality in
OpenOCD is still in development. Do you have any recommendations on
gdb stub for M4 board?

BR,
Terry

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Jonathan Larmour]

On 19/04/12 16:45, Terry Guo wrote:
> 
> I support this patch and your idea that using g_packet_guess is
> smarter and more flexible. I will try your patch out when I am at
> office tomorrow. What kind of board and gdb stub are you using?
> Currently I only have a STM32F4 board in hand but couldn't find a
> suitable gdb stub to access its FPU registers. Such functionality in
> OpenOCD is still in development. Do you have any recommendations on
> gdb stub for M4 board?

It is currently work in progress in eCos, but as yet uncommitted (far more
info and history than you need to know is here:
<http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001524>). Most
development is taking place on a Freescale Kinetis board, primarily by
Ilija Kocho, with me interfering along the way ;-). The eCos changes still
need to be reviewed before they're committed.

STM32F4 is likely to follow very soon after. Ironically, it's not likely
to be committed until we have some confidence that what goes into GDB will
match up with what the stub is doing! We wouldn't want to commit something
that has to change in an incompatible way.

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
  **   Visit us at NEW:UK - the UK Embedded & Electronics Exhibition  **
  **   ARM Partner Pavilion/Stand #114. 18-19 April. Birmingham NEC   **
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Terry Guo]

On Thu, Apr 19, 2012 at 11:57 PM, Jonathan Larmour <jifl@ecoscentric.com> wrote:
>
> STM32F4 is likely to follow very soon after. Ironically, it's not likely
> to be committed until we have some confidence that what goes into GDB will
> match up with what the stub is doing! We wouldn't want to commit something
> that has to change in an incompatible way.
>

Agree. I am also worrying about the gdb stub. If they return something
different from what we currently expect, we will have to make another
change. So I am talking to OpenOCD to share with them what the gdb is
going to do. There are many other gdb stub vendors like JLink. I plan
to talk with them and hope to achieve some consensus between GDB and
stub.

BR,
Terry

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Jonathan Larmour]

On 19/04/12 17:18, Terry Guo wrote:
> On Thu, Apr 19, 2012 at 11:57 PM, Jonathan Larmour <jifl@ecoscentric.com> wrote:
>>
>> STM32F4 is likely to follow very soon after. Ironically, it's not likely
>> to be committed until we have some confidence that what goes into GDB will
>> match up with what the stub is doing! We wouldn't want to commit something
>> that has to change in an incompatible way.
> 
> Agree. I am also worrying about the gdb stub. If they return something
> different from what we currently expect, we will have to make another
> change.

I think there's probably very little variation in what would make sense.
Ultimately the layout used on the wire reflects the realities of the real
register set: the standard Cortex-M regs plus the VFP ones. Theoretically
it's possible someone might like to do away with the VFP pseudo regs and
make the format be 32 single precision registers; but if you did, it would
need a fair bit more GDB work for no actual practical benefit.

So I expect the format I've proposed is the natural format everyone would
want to use.

> So I am talking to OpenOCD to share with them what the gdb is
> going to do. There are many other gdb stub vendors like JLink. I plan
> to talk with them and hope to achieve some consensus between GDB and
> stub.

Because they run on the host, something that's easier for them to do is
supply their own target description XML over the wire. It's also easier to
just update and run a new version if they need to. In contrast, eCos's
stub is programmed into target flash and run on the target, and as you
know, many of the Cortex-M implementations out there have very limited
memory - bytes count. Using a GDB stub on the target is hard enough,
without increasing the footprint further with that. So speaking selfishly,
it doesn't matter so much about OpenOCD or J-link compared to things which
are programmed directly onto hardware, which are harder to change. They
won't care as much about the format as it's easier for them to adapt.

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
  **   Visit us at NEW:UK - the UK Embedded & Electronics Exhibition  **
  **   ARM Partner Pavilion/Stand #114. 18-19 April. Birmingham NEC   **
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Ilija Kocho]

On 19.04.2012 17:57, Jonathan Larmour wrote:
> On 19/04/12 16:45, Terry Guo wrote:
>> I support this patch and your idea that using g_packet_guess is
>> smarter and more flexible. I will try your patch out when I am at
>> office tomorrow. What kind of board and gdb stub are you using?
>> Currently I only have a STM32F4 board in hand but couldn't find a
>> suitable gdb stub to access its FPU registers. Such functionality in
>> OpenOCD is still in development. Do you have any recommendations on
>> gdb stub for M4 board?
> It is currently work in progress in eCos, but as yet uncommitted (far more
> info and history than you need to know is here:
> <http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001524>). Most
> development is taking place on a Freescale Kinetis board, primarily by
> Ilija Kocho, with me interfering along the way ;-). The eCos changes still
> need to be reviewed before they're committed.

The stubware is pretty much ready but I still have work with other parts
of Cortex-M4F FPU architectural support
I am currently on a trip, tomorrow I return and I will intensify the
work. I hope to have patches ready for Bugzilla till end of next week
but I can't promise.

Terry, I could provide you Internet access to a RedBoot target fitted
with the new stub. You will be able to connect with GDB and do some
tests. I could assist you by means of Skype.

> STM32F4 is likely to follow very soon after. 

Currently I am working with Kinetis K70, but support for STM32F4 should
work almost out of box, probably few CDL lines.

> Ironically, it's not likely
> to be committed until we have some confidence that what goes into GDB will
> match up with what the stub is doing! We wouldn't want to commit something
> that has to change in an incompatible way.

Indeed, it would be better if this question is resolved in GDB sooner
rather than later, than GDB will set standard that all stubs will have
to follow.

Ilija

RE: [patch] Add support for VFP d16 layout for Cortex-M4

[Terry Guo]

Hi Ilija,

> -----Original Message-----
> From: Ilija Kocho [mailto:ilijak@siva.com.mk]
> Sent: Friday, April 20, 2012 3:50 PM
> To: Jonathan Larmour; Terry Guo
> Cc: Terry Guo; gdb-patches@sourceware.org
> Subject: Re: [patch] Add support for VFP d16 layout for Cortex-M4
> 
> The stubware is pretty much ready but I still have work with other
> parts
> of Cortex-M4F FPU architectural support
> I am currently on a trip, tomorrow I return and I will intensify the
> work. I hope to have patches ready for Bugzilla till end of next week
> but I can't promise.
> 

Glad to know that and have a good trip. By the way, is your stubware a open
source project or close software?

> Terry, I could provide you Internet access to a RedBoot target fitted
> with the new stub. You will be able to connect with GDB and do some
> tests. I could assist you by means of Skype.
> 
> > STM32F4 is likely to follow very soon after.
> 
> Currently I am working with Kinetis K70, but support for STM32F4 should
> work almost out of box, probably few CDL lines.
> 

I got a K70 board too and its model is TWR K70F120M. My Skype name is
terry.xpguo. I am in Shanghai China. The timezone is GMT+8. Looking forward
to working with you.

BR,
Terry

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Ilija Kocho]

On 20.04.2012 10:14, Terry Guo wrote:
> Hi Ilija,
>
>> -----Original Message-----
>> From: Ilija Kocho [mailto:ilijak@siva.com.mk]
>> Sent: Friday, April 20, 2012 3:50 PM
>> To: Jonathan Larmour; Terry Guo
>> Cc: Terry Guo; gdb-patches@sourceware.org
>> Subject: Re: [patch] Add support for VFP d16 layout for Cortex-M4
>>
>> The stubware is pretty much ready but I still have work with other
>> parts
>> of Cortex-M4F FPU architectural support
>> I am currently on a trip, tomorrow I return and I will intensify the
>> work. I hope to have patches ready for Bugzilla till end of next week
>> but I can't promise.
>>
> Glad to know that and have a good trip. By the way, is your stubware a open
> source project or close software?

Open source indeed, the complete project brings Cortex-M4F Floating
Point architectural support to eCos <http://ecos.sourceware.org> and the
stub is part of that.

>> Terry, I could provide you Internet access to a RedBoot target fitted
>> with the new stub. You will be able to connect with GDB and do some
>> tests. I could assist you by means of Skype.
>>
>>> STM32F4 is likely to follow very soon after.
>> Currently I am working with Kinetis K70, but support for STM32F4 should
>> work almost out of box, probably few CDL lines.
>>
> I got a K70 board too and its model is TWR K70F120M. My Skype name is
> terry.xpguo. I am in Shanghai China. The timezone is GMT+8. Looking forward
> to working with you.

Fine. I have the same board. I am in Skopje, Macedonia GMT+2(with DST,
otherwise GMT+1).

CU
Ilija

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Pedro Alves]

On 04/19/2012 04:12 PM, Jonathan Larmour wrote:

> 
> There's a couple of trivial little fixes from earlier changes too (a
> comment and missing entries in features/Makefile). FAOD I have
> write-after-approval commit perms.


Please split these into separate patches, and post as separate
threads, with their own standalone descriptions.  They're pre-approved,
so go ahead and check them in.

But for this,

> 	* features/Makefile: Add arm-with-m* entries currently missing.

Say instead:

 	* features/Makefile (WHICH): Add arm-with-m and arm-with-m-fpa-layout.

-- 
Pedro Alves

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Pedro Alves]

On 04/19/2012 04:12 PM, Jonathan Larmour wrote:

> As mentioned in my response to the GDB list mail
> <http://sourceware.org/ml/gdb/2012-04/msg00149.html>, I have a patch to
> allow easy automatic use of Cortex-M targets with a register layout with
> 16 double precision / 32 single precision regs. This is as used in the
> increasingly popular Cortex-M4 core.
> 
> I have chosen to use the remote packet guessing approach again, rather
> than inferring from the executable's ELF header, because not everyone will
> actually want to use hardware FP just because they're using that core, so
> what the stub actually supports is a better indicator of what is wanted.


I'd like to make it clear that the guesses mechanism is a just a fallback
mechanism, useful when its too late to change the stubs out there to
send the xml description to gdb themselves.  It's not the ideal way forward,
and it can't scale beyond a few guesses.  The right thing is for
the stubs themselves to report the xml descriptions to GDB (with qXfer:features:read),
not to have them depend on GDB being able to guess it.

Cortex-M4 stubs can already send a xml description equivalent to this new
description to gdb today (without this patch), and things will work equally well,
since the xml description can be (as yours is) simply built using the
org.gnu.gdb.arm.m-profile and org.gnu.gdb.arm.vfp standard description
features.

Is it too late change the stubs that triggered this need to have them
send a xml description?  Despite this patch going in, that's the right
right to do.

That said, I'm no ARM expert, but I don't see anything wrong with the patch.
And having the xml file in the tree that stubs can copy freely is always good.

> +/* Say how long VFP double precision registers are. Used for documentation
                                                      ^
Double space after full stop, please.

> +   purposes and code readability.  These are fixed at 64 bits. */
                                                                 ^
Ditto.

-- 
Pedro Alves

RE: [patch] Add support for VFP d16 layout for Cortex-M4

[Terry Guo]

Hi Pedro,

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Friday, April 20, 2012 8:40 PM
> To: Jonathan Larmour
> Cc: gdb-patches@sourceware.org; Ilija Kocho; Terry Guo; Pedro Alves
> Subject: Re: [patch] Add support for VFP d16 layout for Cortex-M4
> 
> On 04/19/2012 04:12 PM, Jonathan Larmour wrote:
> 
> > As mentioned in my response to the GDB list mail
> > <http://sourceware.org/ml/gdb/2012-04/msg00149.html>, I have a patch
> to
> > allow easy automatic use of Cortex-M targets with a register layout
> with
> > 16 double precision / 32 single precision regs. This is as used in
> the
> > increasingly popular Cortex-M4 core.
> >
> > I have chosen to use the remote packet guessing approach again,
> rather
> > than inferring from the executable's ELF header, because not everyone
> will
> > actually want to use hardware FP just because they're using that core,
> so
> > what the stub actually supports is a better indicator of what is
> wanted.
> 
> 
> I'd like to make it clear that the guesses mechanism is a just a
> fallback
> mechanism, useful when its too late to change the stubs out there to
> send the xml description to gdb themselves.  It's not the ideal way
> forward,
> and it can't scale beyond a few guesses.  The right thing is for
> the stubs themselves to report the xml descriptions to GDB (with
> qXfer:features:read),
> not to have them depend on GDB being able to guess it.
> 

Ideally using the qXfer:features:read is the most natural way. Can you
confirm that once it is used, the guess mechanism won't be resorted for M4?
The code in trunk already has two guess methods for Cortex-M. I don't want
things get messed up.

Another thing in my mind is as Jonathan said before, the stub that will be
programmed into flash is sensitive to the bytes. So I guess they may tend to
save bytes by not returning target.xml. I am not sure what I am assuming is
true. Please correct me if I am wrong.

BR,
Terry

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Pedro Alves]

Hi Terry,

On 04/20/2012 02:16 PM, Terry Guo wrote:

> From: Pedro Alves [mailto:palves@redhat.com]

>> I'd like to make it clear that the guesses mechanism is a just a

>> fallback
>> mechanism, useful when its too late to change the stubs out there to
>> send the xml description to gdb themselves.  It's not the ideal way
>> forward,
>> and it can't scale beyond a few guesses.  The right thing is for
>> the stubs themselves to report the xml descriptions to GDB (with
>> qXfer:features:read),
>> not to have them depend on GDB being able to guess it.
>>
> 
> Ideally using the qXfer:features:read is the most natural way. Can you
> confirm that once it is used, the guess mechanism won't be resorted for M4?
> The code in trunk already has two guess methods for Cortex-M. I don't want
> things get messed up.


Correct.  If the target returns a xml description with qXfer:features:read,
the guess mechanism is never triggered.

> 
> Another thing in my mind is as Jonathan said before, the stub that will be
> programmed into flash is sensitive to the bytes. So I guess they may tend to
> save bytes by not returning target.xml. I am not sure what I am assuming is
> true. Please correct me if I am wrong.


-- 
Pedro Alves

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Jonathan Larmour]

On 20/04/12 13:40, Pedro Alves wrote:
> 
> I'd like to make it clear that the guesses mechanism is a just a fallback
> mechanism, useful when its too late to change the stubs out there to
> send the xml description to gdb themselves.  It's not the ideal way forward,
> and it can't scale beyond a few guesses.  The right thing is for
> the stubs themselves to report the xml descriptions to GDB (with qXfer:features:read),
> not to have them depend on GDB being able to guess it.

I don't think that's unreasonable for a principle, but as I mentioned
elsewhere, some of these targets are very resource constrained. Bytes
really do count. I know it's not ideal, but on the other hand I hope that
there aren't going to be that many layouts to support, and this argument
only really applies to the processors which tend to be very
resource-limited, not big x86 or PowerPC chips.

> Cortex-M4 stubs can already send a xml description equivalent to this new
> description to gdb today (without this patch), and things will work equally well,
> since the xml description can be (as yours is) simply built using the
> org.gnu.gdb.arm.m-profile and org.gnu.gdb.arm.vfp standard description
> features.
> 
> Is it too late change the stubs that triggered this need to have them
> send a xml description?  Despite this patch going in, that's the right
> right to do.

Sorry it's taken me a few days to get back to you, but I've been
implementing what's required to send the XML target description support in
the eCos stub, so that I could have hard data. Unfortunately it has
confirmed my suspicions. Adding code and const data to send back the
description for a vanilla Cortex-M profile target has added 1252 bytes to
our GDB stub's footprint. If the VFP regs description is added to that,
that adds another 931 bytes to give 2183 bytes. That's a big penalty.

XML is not exactly concise. Theoretically we could try and create some
sort of encoding to programmatically generate the XML instead on a dynamic
basis, but that's quite a complex thing to be forced to do, and not
scalable. Unfortunately, in retrospect I'm not sure XML was really the
best format for sending important information from stub to host.

It might save a bit if it were possible for a stub to include XML files
located on the debugging host, but GDB currently assumes any referenced
inclusions must only be read from the target with another qXfer. Maybe the
current feature XML files could be installed by GDB somewhere central, and
be able to be looked up according to some path setting. But even if this
is a good idea, obviously it's only theoretical now. I'm not sure it would
save enough, but it would help.

I understand that for host gdbservers and beefy targets, sending target
descriptions is easy and a good thing; but we can't lose sight of the fact
that many modern System-on-Chips are not like that, and Cortex-M targets
fall into that category. I wouldn't expect to put a GDB stub on the
smallest Cortex-M I've seen with only 16K flash and 4K RAM, but once
you're talking about 64K, it may become more feasible, and little changes
make a big difference.

> That said, I'm no ARM expert, but I don't see anything wrong with the patch.
> And having the xml file in the tree that stubs can copy freely is always good.
> 
>> +/* Say how long VFP double precision registers are. Used for documentation
>                                                       ^
> Double space after full stop, please.
> 
>> +   purposes and code readability.  These are fixed at 64 bits. */
>                                                                  ^
> Ditto.

I'll fix those.

How would you like to move forward?

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Sergio Durigan Junior]

Hello Jonathan,

On Thursday, April 19 2012, Jonathan Larmour wrote:

> Index: features/arm-with-m-vfp-d16.c
> ===================================================================
> RCS file: features/arm-with-m-vfp-d16.c
> diff -N features/arm-with-m-vfp-d16.c
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ features/arm-with-m-vfp-d16.c	19 Apr 2012 14:19:39 -0000
> @@ -0,0 +1,54 @@
> +/* THIS FILE IS GENERATED.  Original: arm-with-m-vfp-d16.xml */
> +
> +#include "defs.h"
> +#include "osabi.h"
> +#include "target-descriptions.h"
> +
> +struct target_desc *tdesc_arm_with_m_vfp_d16;
> +static void
> +initialize_tdesc_arm_with_m_vfp_d16 (void)
> +{
> +  struct target_desc *result = allocate_target_description ();
> +  struct tdesc_feature *feature;
> +  struct tdesc_type *field_type, *type;

I have recently (a.k.a. today) modified the code that generates these
`features' .c files.  Could you please regenerate it when/if you repost
the patch?

Thanks.

-- 
Sergio

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Jonathan Larmour]

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

On 26/04/12 01:50, Sergio Durigan Junior wrote:
> 
> I have recently (a.k.a. today) modified the code that generates these
> `features' .c files.  Could you please regenerate it when/if you repost
> the patch?

No problem. Here is the updated patch. Pedro, let me know if it's ok to
commit or not, thanks.

Jifl

2012-04-26  Jonathan Larmour  <jifl@eCosCentric.com>

	* arm-tdep.h (VFP_REGISTER_SIZE): Define.
	* features/arm-with-m-vfp-d16.xml: New file. Describes
	Cortex-M with VFPv4-sp-d16 FPU register layout.
	* features/Makefile (WHICH): Add arm-with-m-vfp-d16.
	* features/arm-with-m-vfp-d16.c: New. Generated from above.
	* arm-tdep.c: Include arm-with-m-vfp-d16.c.
	(arm-register_g_packet_guesses): Add vfp-d16 guess.
	(_initialise_arm_tdep): Initialize arm-with-m-vfp-d16 tdesc.


-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

[-- Attachment #2: vfpd16a.patch --]
[-- Type: text/plain, Size: 7346 bytes --]

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.361
diff -u -5 -p -r1.361 arm-tdep.c
--- arm-tdep.c	20 Apr 2012 23:39:57 -0000	1.361
+++ arm-tdep.c	26 Apr 2012 14:55:38 -0000
@@ -57,10 +57,11 @@
 
 #include "record.h"
 
 #include "features/arm-with-m.c"
 #include "features/arm-with-m-fpa-layout.c"
+#include "features/arm-with-m-vfp-d16.c"
 #include "features/arm-with-iwmmxt.c"
 #include "features/arm-with-vfpv2.c"
 #include "features/arm-with-vfpv3.c"
 #include "features/arm-with-neon.c"
 
@@ -9696,10 +9697,18 @@ arm_register_g_packet_guesses (struct gd
       register_remote_g_packet_guess (gdbarch,
 				      /* r0-r12,sp,lr,pc; xpsr */
 				      (16 * INT_REGISTER_SIZE)
 				      + INT_REGISTER_SIZE,
 				      tdesc_arm_with_m);
+
+      /* M-profile plus M4F VFP.  */
+      register_remote_g_packet_guess (gdbarch,
+				      /* r0-r12,sp,lr,pc; d0-d15; fpscr,xpsr */
+				      (16 * INT_REGISTER_SIZE)
+				      + (16 * VFP_REGISTER_SIZE)
+				      + (2 * INT_REGISTER_SIZE),
+				      tdesc_arm_with_m_vfp_d16);
     }
 
   /* Otherwise we don't have a useful guess.  */
 }
 
@@ -10331,10 +10340,11 @@ _initialize_arm_tdep (void)
 				  arm_elf_osabi_sniffer);
 
   /* Initialize the standard target descriptions.  */
   initialize_tdesc_arm_with_m ();
   initialize_tdesc_arm_with_m_fpa_layout ();
+  initialize_tdesc_arm_with_m_vfp_d16 ();
   initialize_tdesc_arm_with_iwmmxt ();
   initialize_tdesc_arm_with_vfpv2 ();
   initialize_tdesc_arm_with_vfpv3 ();
   initialize_tdesc_arm_with_neon ();
 
Index: arm-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.h,v
retrieving revision 1.56
diff -u -5 -p -r1.56 arm-tdep.h
--- arm-tdep.h	27 Mar 2012 15:46:33 -0000	1.56
+++ arm-tdep.h	26 Apr 2012 14:55:38 -0000
@@ -69,10 +69,14 @@ enum gdb_regnum {
 /* Say how long FP registers are.  Used for documentation purposes and
    code readability in this header.  IEEE extended doubles are 80
    bits.  DWORD aligned they use 96 bits.  */
 #define FP_REGISTER_SIZE	12
 
+/* Say how long VFP double precision registers are.  Used for documentation
+   purposes and code readability.  These are fixed at 64 bits.  */
+#define VFP_REGISTER_SIZE	8
+
 /* Number of machine registers.  The only define actually required 
    is gdbarch_num_regs.  The other definitions are used for documentation
    purposes and code readability.  */
 /* For 26 bit ARM code, a fake copy of the PC is placed in register 25 (PS)
    (and called PS for processor status) so the status bits can be cleared
Index: features/Makefile
===================================================================
RCS file: /cvs/src/src/gdb/features/Makefile,v
retrieving revision 1.27
diff -u -5 -p -r1.27 Makefile
--- features/Makefile	20 Apr 2012 23:39:58 -0000	1.27
+++ features/Makefile	26 Apr 2012 14:55:38 -0000
@@ -29,11 +29,11 @@
 # configured for the correct architecture, so the files are again kept
 # in the GDB repository.  To generate C files:
 #   make GDB=/path/to/gdb XMLTOC="xml files" cfiles
 
 WHICH = arm-with-iwmmxt arm-with-vfpv2 arm-with-vfpv3 arm-with-neon \
-	arm-with-m arm-with-m-fpa-layout \
+	arm-with-m arm-with-m-fpa-layout arm-with-m-vfp-d16 \
 	i386/i386 i386/i386-linux \
 	i386/i386-mmx i386/i386-mmx-linux \
 	i386/amd64 i386/amd64-linux \
 	i386/i386-avx i386/i386-avx-linux \
 	i386/amd64-avx i386/amd64-avx-linux \
Index: features/arm-with-m-vfp-d16.c
===================================================================
RCS file: features/arm-with-m-vfp-d16.c
diff -N features/arm-with-m-vfp-d16.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ features/arm-with-m-vfp-d16.c	26 Apr 2012 14:55:38 -0000
@@ -0,0 +1,53 @@
+/* THIS FILE IS GENERATED.  Original: arm-with-m-vfp-d16.xml */
+
+#include "defs.h"
+#include "osabi.h"
+#include "target-descriptions.h"
+
+struct target_desc *tdesc_arm_with_m_vfp_d16;
+static void
+initialize_tdesc_arm_with_m_vfp_d16 (void)
+{
+  struct target_desc *result = allocate_target_description ();
+  struct tdesc_feature *feature;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arm.m-profile");
+  tdesc_create_reg (feature, "r0", 0, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r1", 1, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r2", 2, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r3", 3, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r4", 4, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r5", 5, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r6", 6, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r7", 7, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r8", 8, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r9", 9, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r10", 10, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r11", 11, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "r12", 12, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "sp", 13, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "lr", 14, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "pc", 15, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "xpsr", 25, 1, NULL, 32, "int");
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.arm.vfp");
+  tdesc_create_reg (feature, "d0", 26, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d1", 27, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d2", 28, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d3", 29, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d4", 30, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d5", 31, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d6", 32, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d7", 33, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d8", 34, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d9", 35, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d10", 36, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d11", 37, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d12", 38, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d13", 39, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d14", 40, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "d15", 41, 1, NULL, 64, "ieee_double");
+  tdesc_create_reg (feature, "fpscr", 42, 1, "float", 32, "int");
+
+  tdesc_arm_with_m_vfp_d16 = result;
+}
Index: features/arm-with-m-vfp-d16.xml
===================================================================
RCS file: features/arm-with-m-vfp-d16.xml
diff -N features/arm-with-m-vfp-d16.xml
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ features/arm-with-m-vfp-d16.xml	26 Apr 2012 14:55:38 -0000
@@ -0,0 +1,13 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2010-2012 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE target SYSTEM "gdb-target.dtd">
+<target>
+  <xi:include href="arm-m-profile.xml"/>
+  <!-- Layout of vfpv4-sp-d16 is identical to vfpv2 -->
+  <xi:include href="arm-vfpv2.xml"/>
+</target>

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Pedro Alves]

On 04/26/2012 04:04 PM, Jonathan Larmour wrote:

> On 26/04/12 01:50, Sergio Durigan Junior wrote:
>>
>> I have recently (a.k.a. today) modified the code that generates these
>> `features' .c files.  Could you please regenerate it when/if you repost
>> the patch?
> 
> No problem. Here is the updated patch. Pedro, let me know if it's ok to
> commit or not, thanks.


Fine with me.

-- 
Pedro Alves

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Jonathan Larmour]

On 26/04/12 16:13, Pedro Alves wrote:
> On 04/26/2012 04:04 PM, Jonathan Larmour wrote:
>> No problem. Here is the updated patch. Pedro, let me know if it's ok to
>> commit or not, thanks.
> 
> Fine with me.

Committed. Thanks for your patience and good review.

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Tom Tromey]

>>>>> "Jifl" == Jonathan Larmour <jifl@eCosCentric.com> writes:

Jifl> Sorry it's taken me a few days to get back to you, but I've been
Jifl> implementing what's required to send the XML target description
Jifl> support in the eCos stub, so that I could have hard
Jifl> data. Unfortunately it has confirmed my suspicions. Adding code
Jifl> and const data to send back the description for a vanilla Cortex-M
Jifl> profile target has added 1252 bytes to our GDB stub's
Jifl> footprint. If the VFP regs description is added to that, that adds
Jifl> another 931 bytes to give 2183 bytes. That's a big penalty.

Can the XML be compressed?
I don't know.  If not, what if it could be?

I'm curious what you use for your stub.
Based on the recent gdb@ threads, it seems like it can't be gdbserver.

Tom

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Jonathan Larmour]

On 26/04/12 20:35, Tom Tromey wrote:
>>>>>> "Jifl" == Jonathan Larmour <jifl@eCosCentric.com> writes:
> 
> Jifl> Adding code
> Jifl> and const data to send back the description for a vanilla Cortex-M
> Jifl> profile target has added 1252 bytes to our GDB stub's
> Jifl> footprint. If the VFP regs description is added to that, that adds
> Jifl> another 931 bytes to give 2183 bytes. That's a big penalty.
> 
> Can the XML be compressed?
> I don't know.  If not, what if it could be?

I assume you mean having it in the stub in a pre-compressed form; in which
case yes, that helps[1]. If you meant compressing at runtime at the point of
sending, that would mean having to include something zlib-y in the program,
which would then add to footprint, which is why I was initially suggesting
some sort of encoding of the information required (not merely encoding the
XML) rather than just compression. That would be complex though.

But if GDB could handle compressed target descriptions, that would be useful.
Perhaps the stub could use a different $qSupported response to indicate its
compressed, or perhaps GDB could recognise magic numbers at the start of a
$qXfer:features:read response and pass to zlib. (e.g. gzip is always 0x1f,0x8b).

> I'm curious what you use for your stub.
> Based on the recent gdb@ threads, it seems like it can't be gdbserver.

There's a stub as part of eCos. It originally was used in Cygnus, but
according to its headers came from HP before that. Of course it's been hacked
on a lot in eCos so may be quite different to the original now.

Jifl
[1] Using an xml file defining just the main GPRs for Cortex-M as an example,
gzip -9 compresses that from 674 bytes to 237 bytes. Add in the VFP regs and
it goes from 1551 bytes to 330. Evidently the additional code overhead to deal
with parsing and handling sending the target description is about 600 bytes,
with Thumb-2 anyway.

Re: [patch] Add support for VFP d16 layout for Cortex-M4

[Tom Tromey]

>>>>> "Jifl" == Jonathan Larmour <jifl@eCosCentric.com> writes:

Jifl> I assume you mean having it in the stub in a pre-compressed form; in which
Jifl> case yes, that helps[1].

Yeah.

Jifl> But if GDB could handle compressed target descriptions, that would
Jifl> be useful.  Perhaps the stub could use a different $qSupported
Jifl> response to indicate its compressed, or perhaps GDB could
Jifl> recognise magic numbers at the start of a $qXfer:features:read
Jifl> response and pass to zlib. (e.g. gzip is always 0x1f,0x8b).

Yeah, I think it would be a reasonable addition.

Tom