From: "Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>
To: Kamil Rytarowski <n54@gmx.com>
Cc: Alan Hayward <Alan.Hayward@arm.com>,
gdb-patches <gdb-patches@sourceware.org>, nd <nd@arm.com>
Subject: Re: [PATCH] Fix arm-netbsd build error
Date: Wed, 12 Feb 2020 23:43:00 -0000 [thread overview]
Message-ID: <CAPTJ0XF3bu+9S0uuRTjXgWw7kZcBjTYsNwOn++EFdy=heGyccQ@mail.gmail.com> (raw)
In-Reply-To: <CAPTJ0XEEFdO-AyWZRakZLBFwVDhWir2xffoh_c_BSKCd3k--Uw@mail.gmail.com>
And one more question -- since VFP 1 is deprecated (not even mentioned
on http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0472j/chr1359124231926.html)
and your example codes don't include VFP 2, should I assume that
fpu_present implies VFP 3 and that VFPv3-D16 doesn't happen?
Thanks again,
Christian
On Wed, Feb 12, 2020 at 5:28 PM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> Thanks, Kamil!
>
> Please forgive the stupid question, but is SIMD in this context the
> same as IWMMXT?
>
> Christian
>
> On Wed, Feb 12, 2020 at 11:16 AM Kamil Rytarowski <n54@gmx.com> wrote:
> >
> > On 12.02.2020 18:09, Kamil Rytarowski wrote:
> > > On 12.02.2020 17:28, Christian Biesinger wrote:
> > >> On Wed, Feb 12, 2020 at 7:29 AM Alan Hayward <Alan.Hayward@arm.com> wrote:
> > >>>
> > >>>
> > >>>
> > >>>> On 11 Feb 2020, at 23:34, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
> > >>>>
> > >>>> On Tue, Feb 11, 2020 at 4:55 PM Christian Biesinger
> > >>>> <cbiesinger@google.com> wrote:
> > >>>>>
> > >>>>> The floating point register interface has changed to this:
> > >>>>> https://github.com/NetBSD/src/blob/trunk/sys/arch/arm/include/reg.h
> > >>>>>
> > >>>>> It now uses VFP instead of FPA registers. This patch updates
> > >>>>> arm-nbsd-nat.c accordingly.
> > >>>>>
> > >>>>> Tested by compiling on arm-netbsd on qemu. For actually testing, there
> > >>>>> seems to be something missing as "info registers" only shows FPA
> > >>>>> registers and no VFP ones. I am still investigating why this is;
> > >>>>> please let me know if you know. However, I think this is still good
> > >>>>> to check in as-is.
> > >>>>
> > >>>> Hm... this is perhaps because arm_netbsd_nat_target does not implement
> > >>>> read_description; if it returned arm_read_description
> > >>>> (ARM_FP_TYPE_VFPV2) this may work?
> > >>>>
> > >>>
> > >>> Yes, looks like netbsd isn’t using any target description functionality.
> > >>>
> > >>> I suspect the code is getting into arm_gdbarch_init() with a null tdesc,
> > >>> and then using the AUTO setting. But that’s just a guess.
> > >>>
> > >>> Implementing read_description as you suggest should help. However,
> > >>> read_description should probably do HWCAP checking similar to the
> > >>> arm_linux_nat_target and arm_fbsd_nat_target versions.
> > >>>
> > >>> Without that, I’d worry that your patch below might start writing off the
> > >>> end of the regcache that had been allocated for a fewer number of registers.
> > >>
> > >> Hm... well, I've probably spent entirely too much time on this
> > >> already, but just in case there's an easy fix -- Kamil, does NetBSD
> > >> provide an API similar to Linux's HWCAP API that would let me check
> > >> which VFP version the current CPU is using? It seems AUXV does not
> > >> contain HWCAP data on NetBSD, unlike Linux/FreeBSD, though I may be
> > >> missing something.
> > >>
> > >> (Compare arm_fbsd_read_description_auxv /
> > >> arm_linux_nat_target::read_description)
> > >>
> > >> Christian
> > >>
> > >
> > > Thank you for your work.
> > >
> > > HWCAP in auxv is not supported (although there is a stub for it).
> > >
> > > Here is a complete algorithm to detect FPU on ARM 32-bit.
> > >
> > > http://netbsd.org/~kamil/arm-fpu.c
> > >
> >
> > Here is another example with SIMD:
> >
> > http://cvsweb.netbsd.org/bsdweb.cgi/xsrc/external/mit/pixman/dist/pixman/pixman-arm.c.diff?r1=1.1&r2=1.2&f=h
> >
> > >>>
> > >>>>>
> > >>>>> gdb/ChangeLog:
> > >>>>>
> > >>>>> 2020-02-11 Christian Biesinger <cbiesinger@google.com>
> > >>>>>
> > >>>>> * arm-nbsd-nat.c (arm_supply_fparegset): Rename to...
> > >>>>> (arm_supply_vfpregset): ...this, and update to use VFP registers.
> > >>>>> (fetch_fp_register): Update.
> > >>>>> (fetch_fp_regs): Update.
> > >>>>> (store_fp_register): Update.
> > >>>>> (store_fp_regs): Update.
> > >>>>> (fetch_elfcore_registers): Update.
> > >>>>> ---
> > >>>>> gdb/arm-nbsd-nat.c | 80 ++++++++++++++++++++--------------------------
> > >>>>> 1 file changed, 34 insertions(+), 46 deletions(-)
> > >>>>>
> > >>>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> > >>>>> index 11afc289c3..8027f54dfe 100644
> > >>>>> --- a/gdb/arm-nbsd-nat.c
> > >>>>> +++ b/gdb/arm-nbsd-nat.c
> > >>>>> @@ -65,15 +65,13 @@ arm_supply_gregset (struct regcache *regcache, struct reg *gregset)
> > >>>>> }
> > >>>>>
> > >>>>> static void
> > >>>>> -arm_supply_fparegset (struct regcache *regcache, struct fpreg *fparegset)
> > >>>>> +arm_supply_vfpregset (struct regcache *regcache, struct fpreg *fpregset)
> > >>>>> {
> > >>>>> - int regno;
> > >>>>> -
> > >>>>> - for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
> > >>>>> - regcache->raw_supply (regno,
> > >>>>> - (char *) &fparegset->fpr[regno - ARM_F0_REGNUM]);
> > >>>>> + struct vfpreg &vfp = fpregset->fpr_vfp;
> > >>>>> + for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
> > >>>>> + regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> > >>>>>
> > >>>>> - regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
> > >>>>> + regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> > >>>>> }
> > >>>>>
> > >>>>> static void
> > >>>>> @@ -147,10 +145,10 @@ static void
> > >>>>> fetch_fp_register (struct regcache *regcache, int regno)
> > >>>>> {
> > >>>>> struct fpreg inferior_fp_registers;
> > >>>>> - int ret;
> > >>>>> + int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
> > >>>>> + (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> > >>>>>
> > >>>>> - ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
> > >>>>> - (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> > >>>>> + struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;
> > >>>>>
> > >>>>> if (ret < 0)
> > >>>>> {
> > >>>>> @@ -158,18 +156,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
> > >>>>> return;
> > >>>>> }
> > >>>>>
> > >>>>> - switch (regno)
> > >>>>> + if (regno == ARM_FPSCR_REGNUM)
> > >>>>> + regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> > >>>>> + else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
> > >>>>> {
> > >>>>> - case ARM_FPS_REGNUM:
> > >>>>> - regcache->raw_supply (ARM_FPS_REGNUM,
> > >>>>> - (char *) &inferior_fp_registers.fpr_fpsr);
> > >>>>> - break;
> > >>>>> -
> > >>>>> - default:
> > >>>>> - regcache->raw_supply
> > >>>>> - (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
> > >>>>> - break;
> > >>>>> + regcache->raw_supply (regno,
> > >>>>> + (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> > >>>>> }
> > >>>>> + else
> > >>>>> + warning (_("Invalid register number."));
> > >>>>> }
> > >>>>>
> > >>>>> static void
> > >>>>> @@ -188,7 +183,7 @@ fetch_fp_regs (struct regcache *regcache)
> > >>>>> return;
> > >>>>> }
> > >>>>>
> > >>>>> - arm_supply_fparegset (regcache, &inferior_fp_registers);
> > >>>>> + arm_supply_vfpregset (regcache, &inferior_fp_registers);
> > >>>>> }
> > >>>>>
> > >>>>> void
> > >>>>> @@ -327,10 +322,9 @@ static void
> > >>>>> store_fp_register (const struct regcache *regcache, int regno)
> > >>>>> {
> > >>>>> struct fpreg inferior_fp_registers;
> > >>>>> - int ret;
> > >>>>> -
> > >>>>> - ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
> > >>>>> - (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> > >>>>> + int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
> > >>>>> + (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> > >>>>> + struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;
> > >>>>>
> > >>>>> if (ret < 0)
> > >>>>> {
> > >>>>> @@ -338,18 +332,15 @@ store_fp_register (const struct regcache *regcache, int regno)
> > >>>>> return;
> > >>>>> }
> > >>>>>
> > >>>>> - switch (regno)
> > >>>>> + if (regno == ARM_FPSCR_REGNUM)
> > >>>>> + regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> > >>>>> + else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
> > >>>>> {
> > >>>>> - case ARM_FPS_REGNUM:
> > >>>>> - regcache->raw_collect (ARM_FPS_REGNUM,
> > >>>>> - (char *) &inferior_fp_registers.fpr_fpsr);
> > >>>>> - break;
> > >>>>> -
> > >>>>> - default:
> > >>>>> - regcache->raw_collect
> > >>>>> - (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
> > >>>>> - break;
> > >>>>> + regcache->raw_collect (regno,
> > >>>>> + (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> > >>>>> }
> > >>>>> + else
> > >>>>> + warning (_("Invalid register number."));
> > >>>>>
> > >>>>> ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
> > >>>>> (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> > >>>>> @@ -361,20 +352,17 @@ store_fp_register (const struct regcache *regcache, int regno)
> > >>>>> static void
> > >>>>> store_fp_regs (const struct regcache *regcache)
> > >>>>> {
> > >>>>> - struct fpreg inferior_fp_registers;
> > >>>>> - int ret;
> > >>>>> - int regno;
> > >>>>> + struct fpreg fpregs;
> > >>>>>
> > >>>>> -
> > >>>>> - for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
> > >>>>> + for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
> > >>>>> regcache->raw_collect
> > >>>>> - (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
> > >>>>> + (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> > >>>>>
> > >>>>> - regcache->raw_collect (ARM_FPS_REGNUM,
> > >>>>> - (char *) &inferior_fp_registers.fpr_fpsr);
> > >>>>> + regcache->raw_collect (ARM_FPSCR_REGNUM,
> > >>>>> + (char *) &fpregs.fpr_vfp.vfp_fpscr);
> > >>>>>
> > >>>>> - ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
> > >>>>> - (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> > >>>>> + int ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
> > >>>>> + (PTRACE_TYPE_ARG3) &fpregs, 0);
> > >>>>>
> > >>>>> if (ret < 0)
> > >>>>> warning (_("unable to store floating-point registers"));
> > >>>>> @@ -427,7 +415,7 @@ fetch_elfcore_registers (struct regcache *regcache,
> > >>>>> /* The memcpy may be unnecessary, but we can't really be sure
> > >>>>> of the alignment of the data in the core file. */
> > >>>>> memcpy (&fparegset, core_reg_sect, sizeof (fparegset));
> > >>>>> - arm_supply_fparegset (regcache, &fparegset);
> > >>>>> + arm_supply_vfpregset (regcache, &fparegset);
> > >>>>> }
> > >>>>> break;
> > >>>>>
> > >>>>> --
> > >>>>> 2.25.0.225.g125e21ebc7-goog
> > >>>>>
> > >>>
> > >
> > >
> >
> >
next prev parent reply other threads:[~2020-02-12 23:43 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200211225503.32992-1-cbiesinger@google.com>
2020-02-11 23:35 ` Christian Biesinger via gdb-patches
2020-02-12 13:30 ` Alan Hayward
2020-02-12 16:29 ` Christian Biesinger via gdb-patches
2020-02-12 17:10 ` Kamil Rytarowski
2020-02-12 17:17 ` Kamil Rytarowski
2020-02-12 23:28 ` Christian Biesinger via gdb-patches
2020-02-12 23:43 ` Christian Biesinger via gdb-patches [this message]
2020-02-13 0:01 ` Kamil Rytarowski
2020-02-28 13:18 ` [PATCH v2] Fix arm-netbsd build error: convert from FPA to VFP Christian Biesinger via gdb-patches
2020-02-28 14:35 ` Alan Hayward
2020-02-28 19:19 ` [PATCH v3] " Christian Biesinger via gdb-patches
[not found] ` <ECBA936B-BF56-4F49-9C57-35E0F29D560E@arm.com>
2020-03-02 17:29 ` Christian Biesinger via gdb-patches
2020-03-02 17:32 ` Kamil Rytarowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPTJ0XF3bu+9S0uuRTjXgWw7kZcBjTYsNwOn++EFdy=heGyccQ@mail.gmail.com' \
--to=gdb-patches@sourceware.org \
--cc=Alan.Hayward@arm.com \
--cc=cbiesinger@google.com \
--cc=n54@gmx.com \
--cc=nd@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox