* [PATCH] gdb/riscv: Apply NaN boxing when writing return values into registers
@ 2020-03-20 23:25 Andrew Burgess
2020-03-25 11:36 ` Andrew Burgess
0 siblings, 1 reply; 2+ messages in thread
From: Andrew Burgess @ 2020-03-20 23:25 UTC (permalink / raw)
To: gdb-patches
This patch was discussed on the bug report here:
https://sourceware.org/bugzilla/show_bug.cgi?id=25489
If there's no additional feedback on the list then I'll merge this in
a few days.
Thanks,
Andrew
---
When setting up function parameters we already perform NaN boxing, as
required by the RISC-V ABI, however, we don't do this when writing
values into registers as part of setting up a return value.
This commit moves the NaN boxing code into a small helper function,
and then makes use of this function when setting up function
parameters, and also when setting up return values.
This should resolve this failure:
FAIL: gdb.base/return-nodebug.exp: float: full width of the returned result
gdb/ChangeLog:
PR gdb/25489
* riscv-tdep.c (riscv_arg_info::c_offset): Update comment.
(riscv_regcache_cooked_write): New function.
(riscv_push_dummy_call): Use new function.
(riscv_return_value): Likewise.
---
gdb/ChangeLog | 8 +++++++
gdb/riscv-tdep.c | 69 ++++++++++++++++++++++++++++++++------------------------
2 files changed, 48 insertions(+), 29 deletions(-)
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 2978b9e2d57..0423e6abf30 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1727,8 +1727,9 @@ struct riscv_arg_info
will go. */
int c_length;
- /* The offset within CONTENTS for this part of the argument. Will
- always be 0 for the first part. For the second part of the
+ /* The offset within CONTENTS for this part of the argument. This can
+ be non-zero even for the first part (the first field of a struct can
+ have a non-zero offset due to padding). For the second part of the
argument, this might be the C_LENGTH value of the first part,
however, if we are passing a structure in two registers, and there's
is padding between the first and second field, then this offset
@@ -2417,6 +2418,26 @@ riscv_print_arg_location (ui_file *stream, struct gdbarch *gdbarch,
}
}
+/* Wrapper around REGCACHE->cooked_write. Places the LEN bytes of DATA
+ into a buffer that is at least as big as the register REGNUM, padding
+ out the DATA with either 0x00, or 0xff. For floating point registers
+ 0xff is used, for everyone else 0x00 is used. */
+
+static void
+riscv_regcache_cooked_write (int regnum, const gdb_byte *data, int len,
+ struct regcache *regcache, int flen)
+{
+ gdb_byte tmp [sizeof (ULONGEST)];
+
+ /* FP values in FP registers must be NaN-boxed. */
+ if (riscv_is_fp_regno_p (regnum) && len < flen)
+ memset (tmp, -1, sizeof (tmp));
+ else
+ memset (tmp, 0, sizeof (tmp));
+ memcpy (tmp, data, len);
+ regcache->cooked_write (regnum, tmp);
+}
+
/* Implement the push dummy call gdbarch callback. */
static CORE_ADDR
@@ -2526,18 +2547,13 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
{
case riscv_arg_info::location::in_reg:
{
- gdb_byte tmp [sizeof (ULONGEST)];
-
gdb_assert (info->argloc[0].c_length <= info->length);
- /* FP values in FP registers must be NaN-boxed. */
- if (riscv_is_fp_regno_p (info->argloc[0].loc_data.regno)
- && info->argloc[0].c_length < call_info.flen)
- memset (tmp, -1, sizeof (tmp));
- else
- memset (tmp, 0, sizeof (tmp));
- memcpy (tmp, (info->contents + info->argloc[0].c_offset),
- info->argloc[0].c_length);
- regcache->cooked_write (info->argloc[0].loc_data.regno, tmp);
+
+ riscv_regcache_cooked_write (info->argloc[0].loc_data.regno,
+ (info->contents
+ + info->argloc[0].c_offset),
+ info->argloc[0].c_length,
+ regcache, call_info.flen);
second_arg_length =
(((info->argloc[0].c_length + info->argloc[0].c_offset) < info->length)
? info->argloc[1].c_length : 0);
@@ -2569,19 +2585,13 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
{
case riscv_arg_info::location::in_reg:
{
- gdb_byte tmp [sizeof (ULONGEST)];
-
gdb_assert ((riscv_is_fp_regno_p (info->argloc[1].loc_data.regno)
&& second_arg_length <= call_info.flen)
|| second_arg_length <= call_info.xlen);
- /* FP values in FP registers must be NaN-boxed. */
- if (riscv_is_fp_regno_p (info->argloc[1].loc_data.regno)
- && second_arg_length < call_info.flen)
- memset (tmp, -1, sizeof (tmp));
- else
- memset (tmp, 0, sizeof (tmp));
- memcpy (tmp, second_arg_data, second_arg_length);
- regcache->cooked_write (info->argloc[1].loc_data.regno, tmp);
+ riscv_regcache_cooked_write (info->argloc[1].loc_data.regno,
+ second_arg_data,
+ second_arg_length,
+ regcache, call_info.flen);
}
break;
@@ -2701,9 +2711,9 @@ riscv_return_value (struct gdbarch *gdbarch,
if (writebuf)
{
const gdb_byte *ptr = writebuf + info.argloc[0].c_offset;
- regcache->cooked_write_part (regnum, 0,
+ riscv_regcache_cooked_write (regnum, ptr,
info.argloc[0].c_length,
- ptr);
+ regcache, call_info.flen);
}
/* A return value in register can have a second part in a
@@ -2730,10 +2740,11 @@ riscv_return_value (struct gdbarch *gdbarch,
if (writebuf)
{
- writebuf += info.argloc[1].c_offset;
- regcache->cooked_write_part (regnum, 0,
- info.argloc[1].c_length,
- writebuf);
+ const gdb_byte *ptr
+ = writebuf + info.argloc[1].c_offset;
+ riscv_regcache_cooked_write
+ (regnum, ptr, info.argloc[1].c_length,
+ regcache, call_info.flen);
}
break;
--
2.14.5
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] gdb/riscv: Apply NaN boxing when writing return values into registers
2020-03-20 23:25 [PATCH] gdb/riscv: Apply NaN boxing when writing return values into registers Andrew Burgess
@ 2020-03-25 11:36 ` Andrew Burgess
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Burgess @ 2020-03-25 11:36 UTC (permalink / raw)
To: gdb-patches
* Andrew Burgess <andrew.burgess@embecosm.com> [2020-03-20 23:25:59 +0000]:
> This patch was discussed on the bug report here:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=25489
>
> If there's no additional feedback on the list then I'll merge this in
> a few days.
>
> Thanks,
> Andrew
>
>
> ---
>
> When setting up function parameters we already perform NaN boxing, as
> required by the RISC-V ABI, however, we don't do this when writing
> values into registers as part of setting up a return value.
>
> This commit moves the NaN boxing code into a small helper function,
> and then makes use of this function when setting up function
> parameters, and also when setting up return values.
>
> This should resolve this failure:
>
> FAIL: gdb.base/return-nodebug.exp: float: full width of the returned result
>
> gdb/ChangeLog:
>
> PR gdb/25489
> * riscv-tdep.c (riscv_arg_info::c_offset): Update comment.
> (riscv_regcache_cooked_write): New function.
> (riscv_push_dummy_call): Use new function.
> (riscv_return_value): Likewise.
I've now pushed this patch.
Thanks,
Andrew
> ---
> gdb/ChangeLog | 8 +++++++
> gdb/riscv-tdep.c | 69 ++++++++++++++++++++++++++++++++------------------------
> 2 files changed, 48 insertions(+), 29 deletions(-)
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 2978b9e2d57..0423e6abf30 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -1727,8 +1727,9 @@ struct riscv_arg_info
> will go. */
> int c_length;
>
> - /* The offset within CONTENTS for this part of the argument. Will
> - always be 0 for the first part. For the second part of the
> + /* The offset within CONTENTS for this part of the argument. This can
> + be non-zero even for the first part (the first field of a struct can
> + have a non-zero offset due to padding). For the second part of the
> argument, this might be the C_LENGTH value of the first part,
> however, if we are passing a structure in two registers, and there's
> is padding between the first and second field, then this offset
> @@ -2417,6 +2418,26 @@ riscv_print_arg_location (ui_file *stream, struct gdbarch *gdbarch,
> }
> }
>
> +/* Wrapper around REGCACHE->cooked_write. Places the LEN bytes of DATA
> + into a buffer that is at least as big as the register REGNUM, padding
> + out the DATA with either 0x00, or 0xff. For floating point registers
> + 0xff is used, for everyone else 0x00 is used. */
> +
> +static void
> +riscv_regcache_cooked_write (int regnum, const gdb_byte *data, int len,
> + struct regcache *regcache, int flen)
> +{
> + gdb_byte tmp [sizeof (ULONGEST)];
> +
> + /* FP values in FP registers must be NaN-boxed. */
> + if (riscv_is_fp_regno_p (regnum) && len < flen)
> + memset (tmp, -1, sizeof (tmp));
> + else
> + memset (tmp, 0, sizeof (tmp));
> + memcpy (tmp, data, len);
> + regcache->cooked_write (regnum, tmp);
> +}
> +
> /* Implement the push dummy call gdbarch callback. */
>
> static CORE_ADDR
> @@ -2526,18 +2547,13 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
> {
> case riscv_arg_info::location::in_reg:
> {
> - gdb_byte tmp [sizeof (ULONGEST)];
> -
> gdb_assert (info->argloc[0].c_length <= info->length);
> - /* FP values in FP registers must be NaN-boxed. */
> - if (riscv_is_fp_regno_p (info->argloc[0].loc_data.regno)
> - && info->argloc[0].c_length < call_info.flen)
> - memset (tmp, -1, sizeof (tmp));
> - else
> - memset (tmp, 0, sizeof (tmp));
> - memcpy (tmp, (info->contents + info->argloc[0].c_offset),
> - info->argloc[0].c_length);
> - regcache->cooked_write (info->argloc[0].loc_data.regno, tmp);
> +
> + riscv_regcache_cooked_write (info->argloc[0].loc_data.regno,
> + (info->contents
> + + info->argloc[0].c_offset),
> + info->argloc[0].c_length,
> + regcache, call_info.flen);
> second_arg_length =
> (((info->argloc[0].c_length + info->argloc[0].c_offset) < info->length)
> ? info->argloc[1].c_length : 0);
> @@ -2569,19 +2585,13 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
> {
> case riscv_arg_info::location::in_reg:
> {
> - gdb_byte tmp [sizeof (ULONGEST)];
> -
> gdb_assert ((riscv_is_fp_regno_p (info->argloc[1].loc_data.regno)
> && second_arg_length <= call_info.flen)
> || second_arg_length <= call_info.xlen);
> - /* FP values in FP registers must be NaN-boxed. */
> - if (riscv_is_fp_regno_p (info->argloc[1].loc_data.regno)
> - && second_arg_length < call_info.flen)
> - memset (tmp, -1, sizeof (tmp));
> - else
> - memset (tmp, 0, sizeof (tmp));
> - memcpy (tmp, second_arg_data, second_arg_length);
> - regcache->cooked_write (info->argloc[1].loc_data.regno, tmp);
> + riscv_regcache_cooked_write (info->argloc[1].loc_data.regno,
> + second_arg_data,
> + second_arg_length,
> + regcache, call_info.flen);
> }
> break;
>
> @@ -2701,9 +2711,9 @@ riscv_return_value (struct gdbarch *gdbarch,
> if (writebuf)
> {
> const gdb_byte *ptr = writebuf + info.argloc[0].c_offset;
> - regcache->cooked_write_part (regnum, 0,
> + riscv_regcache_cooked_write (regnum, ptr,
> info.argloc[0].c_length,
> - ptr);
> + regcache, call_info.flen);
> }
>
> /* A return value in register can have a second part in a
> @@ -2730,10 +2740,11 @@ riscv_return_value (struct gdbarch *gdbarch,
>
> if (writebuf)
> {
> - writebuf += info.argloc[1].c_offset;
> - regcache->cooked_write_part (regnum, 0,
> - info.argloc[1].c_length,
> - writebuf);
> + const gdb_byte *ptr
> + = writebuf + info.argloc[1].c_offset;
> + riscv_regcache_cooked_write
> + (regnum, ptr, info.argloc[1].c_length,
> + regcache, call_info.flen);
> }
> break;
>
> --
> 2.14.5
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-03-25 11:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20 23:25 [PATCH] gdb/riscv: Apply NaN boxing when writing return values into registers Andrew Burgess
2020-03-25 11:36 ` Andrew Burgess
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).