This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 3/3] Parse SVE registers in aarch64 core file reading/writing
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Alan Hayward <alan dot hayward at arm dot com>, <gdb-patches at sourceware dot org>
- Cc: <nd at arm dot com>
- Date: Mon, 6 Aug 2018 14:29:05 -0400
- Subject: Re: [PATCH v2 3/3] Parse SVE registers in aarch64 core file reading/writing
- References: <20180730092528.98739-1-alan.hayward@arm.com> <20180730092528.98739-4-alan.hayward@arm.com>
On 2018-07-30 05:25 AM, Alan Hayward wrote:
> sve_regmap cannot be global static as the size is dependant on the current
> vector length.
>
> 2018-07-30 Alan Hayward <alan.hayward@arm.com>
>
> * aarch64-linux-tdep.c (aarch64_linux_supply_sve_regset): New function.
> (aarch64_linux_collect_sve_regset): Likewise.
> (aarch64_linux_iterate_over_regset_sections): Check for SVE.
> * regcache.h (regcache_map_entry_size): New function.
> ---
> gdb/aarch64-linux-tdep.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++-
> gdb/regcache.h | 8 ++++
> 2 files changed, 118 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index f9a95950da..bd61a2d722 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -288,6 +288,85 @@ aarch64_linux_core_read_vq (struct gdbarch *gdbarch, bfd *abfd)
> return vq;
> }
>
> +/* Supply register REGNUM from BUF to REGCACHE, using the register map
> + in REGSET. If REGNUM is -1, do this for all registers in REGSET.
> + If BUF is NULL, set the registers to "unavailable" status. */
> +
> +static void
> +aarch64_linux_supply_sve_regset (const struct regset *regset,
> + struct regcache *regcache,
> + int regnum, const void *buf, size_t size)
> +{
> + struct gdbarch *gdbarch = regcache->arch ();
> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +
> + if (buf == nullptr)
> + return regcache->supply_regset (regset, regnum, nullptr, size);
> + gdb_assert (size > SVE_HEADER_SIZE);
> +
> + /* BUF contains an SVE header followed by a register dump of either the
> + passed in SVE regset or a NEON fpregset. */
> +
> + /* Extract required fields from the header. */
> + uint64_t vg = sve_vg_from_vl (SVE_HEADER_READ (buf, 2, byte_order));
> + uint16_t flags = SVE_HEADER_READ (buf, 4, byte_order);
> +
> + if (regnum == -1 || regnum == AARCH64_SVE_VG_REGNUM)
> + regcache->raw_supply (AARCH64_SVE_VG_REGNUM, &vg);
I think this raw_supply is wrong. The vg local variable is in host byte order,
but the data in the buffer passed to raw_supply should be in target. So you
may need to do a store_unsigned_integer in a buffer and supply that. If this
is a pattern that happens often enough, maybe it would be worth having a
raw_supply that does this conversion from integer to register buffer, a bit
like "regcache::raw_write (int regnum, T val)" does.
> +
> + if (flags & 1)
> + {
> + /* Register dump is a SVE structure. */
> + regcache->supply_regset (regset, regnum,
> + (gdb_byte *) buf + SVE_HEADER_SIZE,
> + size - SVE_HEADER_SIZE);
> + }
> + else
> + {
> + /* Register dump is a fpsimd structure. First clear the SVE
> + registers. */
> + for (int i = 0; i < AARCH64_SVE_Z_REGS_NUM; i++)
> + regcache->raw_supply_zeroed (AARCH64_SVE_Z0_REGNUM + i);
> + for (int i = 0; i < AARCH64_SVE_P_REGS_NUM; i++)
> + regcache->raw_supply_zeroed (AARCH64_SVE_P0_REGNUM + i);
> + regcache->raw_supply_zeroed (AARCH64_SVE_FFR_REGNUM);
Just wondering, should they be made unavailable instead of cleared?
Simon