[3/3] RISC-V: gdb.base/gnu_vector fixes.
Commit Message
Unnamed arguments with 2*XLEN alignment are passed in aligned register pairs.
gdb/
* riscv-tdep.c (struct riscv_arg_info): New field is_unnamed.
(riscv_call_arg_scalar_int): If unnamed arg with twice xlen alignment,
then increment next_regnum if odd.
(riscv_arg_location): New arg is_unnamed. Set ainfo->is_unnamed.
(riscv_push_dummy_call): New local ftype. Call check_typedef to set
function type. Pass new arg to riscv_arg_location based on function
type.
(riscv_return_value): Pass new arg to riscv_arg_location.
---
gdb/riscv-tdep.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
Comments
* Jim Wilson <jimw@sifive.com> [2018-11-06 13:44:46 -0800]:
> Unnamed arguments with 2*XLEN alignment are passed in aligned register pairs.
>
> gdb/
> * riscv-tdep.c (struct riscv_arg_info): New field is_unnamed.
> (riscv_call_arg_scalar_int): If unnamed arg with twice xlen alignment,
> then increment next_regnum if odd.
> (riscv_arg_location): New arg is_unnamed. Set ainfo->is_unnamed.
> (riscv_push_dummy_call): New local ftype. Call check_typedef to set
> function type. Pass new arg to riscv_arg_location based on function
> type.
> (riscv_return_value): Pass new arg to riscv_arg_location.
Could you change the titles for the 3 commits in this series please to
make them more descriptive of the fix in each commit. This also
avoids having 3 commits in a row with the same title line, which can
look like a mistake in 'git log --format=oneline'.
> ---
> gdb/riscv-tdep.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 3d4f7e3dcc..93310c329f 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -1737,6 +1737,9 @@ struct riscv_arg_info
> then this offset will be set to 0. */
> int c_offset;
> } argloc[2];
> +
> + /* TRUE if this is an unnamed argument. */
> + bool is_unnamed;
> };
>
> /* Information about a set of registers being used for passing arguments as
> @@ -1932,6 +1935,12 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo,
> {
> int len = (ainfo->length > cinfo->xlen) ? cinfo->xlen : ainfo->length;
>
> + /* Unnamed arguments in registers that require 2*XLEN alignment are
> + passed in an aligned register pair. */
> + if (ainfo->is_unnamed && (ainfo->align == cinfo->xlen * 2)
> + && cinfo->int_regs.next_regnum & 1)
> + cinfo->int_regs.next_regnum++;
> +
> if (!riscv_assign_reg_location (&ainfo->argloc[0],
> &cinfo->int_regs, len, 0))
> riscv_assign_stack_location (&ainfo->argloc[0],
> @@ -2222,11 +2231,12 @@ static void
> riscv_arg_location (struct gdbarch *gdbarch,
> struct riscv_arg_info *ainfo,
> struct riscv_call_info *cinfo,
> - struct type *type)
> + struct type *type, bool is_unnamed)
Could you update the functions header comment to explain what impact
IS_UNNAMED will have on the behaviour of the function.
> {
> ainfo->type = type;
> ainfo->length = TYPE_LENGTH (ainfo->type);
> ainfo->align = riscv_type_alignment (ainfo->type);
> + ainfo->is_unnamed = is_unnamed;
> ainfo->contents = nullptr;
>
> switch (TYPE_CODE (ainfo->type))
> @@ -2375,6 +2385,11 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
>
> CORE_ADDR osp = sp;
>
> + struct type *ftype = check_typedef (value_type (function));
> +
> + if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
> + ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
> +
> /* We'll use register $a0 if we're returning a struct. */
> if (struct_return)
> ++call_info.int_regs.next_regnum;
> @@ -2388,7 +2403,8 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
> arg_value = args[i];
> arg_type = check_typedef (value_type (arg_value));
>
> - riscv_arg_location (gdbarch, info, &call_info, arg_type);
> + riscv_arg_location (gdbarch, info, &call_info, arg_type,
> + TYPE_VARARGS (ftype) && i >= TYPE_NFIELDS (ftype));
>
> if (info->type != arg_type)
> arg_value = value_cast (info->type, arg_value);
> @@ -2565,7 +2581,7 @@ riscv_return_value (struct gdbarch *gdbarch,
> struct type *arg_type;
>
> arg_type = check_typedef (type);
> - riscv_arg_location (gdbarch, &info, &call_info, arg_type);
> + riscv_arg_location (gdbarch, &info, &call_info, arg_type, false);
>
> if (riscv_debug_infcall > 0)
> {
> --
> 2.17.1
>
Otherwise looks fine.
Thanks,
Andrew
* Andrew Burgess <andrew.burgess@embecosm.com> [2018-11-08 14:34:11 +0000]:
> * Jim Wilson <jimw@sifive.com> [2018-11-06 13:44:46 -0800]:
>
> > Unnamed arguments with 2*XLEN alignment are passed in aligned register pairs.
> >
> > gdb/
> > * riscv-tdep.c (struct riscv_arg_info): New field is_unnamed.
> > (riscv_call_arg_scalar_int): If unnamed arg with twice xlen alignment,
> > then increment next_regnum if odd.
> > (riscv_arg_location): New arg is_unnamed. Set ainfo->is_unnamed.
> > (riscv_push_dummy_call): New local ftype. Call check_typedef to set
> > function type. Pass new arg to riscv_arg_location based on function
> > type.
> > (riscv_return_value): Pass new arg to riscv_arg_location.
>
> Could you change the titles for the 3 commits in this series please to
> make them more descriptive of the fix in each commit. This also
> avoids having 3 commits in a row with the same title line, which can
> look like a mistake in 'git log --format=oneline'.
Also can you make sure the commit message for each patch includes
details about which tests you expect to go from fail to pass, and on
which RISC-V variants you expect to see improvements.
Thanks,
Andrew
>
>
>
>
>
> > ---
> > gdb/riscv-tdep.c | 22 +++++++++++++++++++---
> > 1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> > index 3d4f7e3dcc..93310c329f 100644
> > --- a/gdb/riscv-tdep.c
> > +++ b/gdb/riscv-tdep.c
> > @@ -1737,6 +1737,9 @@ struct riscv_arg_info
> > then this offset will be set to 0. */
> > int c_offset;
> > } argloc[2];
> > +
> > + /* TRUE if this is an unnamed argument. */
> > + bool is_unnamed;
> > };
> >
> > /* Information about a set of registers being used for passing arguments as
> > @@ -1932,6 +1935,12 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo,
> > {
> > int len = (ainfo->length > cinfo->xlen) ? cinfo->xlen : ainfo->length;
> >
> > + /* Unnamed arguments in registers that require 2*XLEN alignment are
> > + passed in an aligned register pair. */
> > + if (ainfo->is_unnamed && (ainfo->align == cinfo->xlen * 2)
> > + && cinfo->int_regs.next_regnum & 1)
> > + cinfo->int_regs.next_regnum++;
> > +
> > if (!riscv_assign_reg_location (&ainfo->argloc[0],
> > &cinfo->int_regs, len, 0))
> > riscv_assign_stack_location (&ainfo->argloc[0],
> > @@ -2222,11 +2231,12 @@ static void
> > riscv_arg_location (struct gdbarch *gdbarch,
> > struct riscv_arg_info *ainfo,
> > struct riscv_call_info *cinfo,
> > - struct type *type)
> > + struct type *type, bool is_unnamed)
>
> Could you update the functions header comment to explain what impact
> IS_UNNAMED will have on the behaviour of the function.
>
> > {
> > ainfo->type = type;
> > ainfo->length = TYPE_LENGTH (ainfo->type);
> > ainfo->align = riscv_type_alignment (ainfo->type);
> > + ainfo->is_unnamed = is_unnamed;
> > ainfo->contents = nullptr;
> >
> > switch (TYPE_CODE (ainfo->type))
> > @@ -2375,6 +2385,11 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
> >
> > CORE_ADDR osp = sp;
> >
> > + struct type *ftype = check_typedef (value_type (function));
> > +
> > + if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
> > + ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
> > +
> > /* We'll use register $a0 if we're returning a struct. */
> > if (struct_return)
> > ++call_info.int_regs.next_regnum;
> > @@ -2388,7 +2403,8 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
> > arg_value = args[i];
> > arg_type = check_typedef (value_type (arg_value));
> >
> > - riscv_arg_location (gdbarch, info, &call_info, arg_type);
> > + riscv_arg_location (gdbarch, info, &call_info, arg_type,
> > + TYPE_VARARGS (ftype) && i >= TYPE_NFIELDS (ftype));
> >
> > if (info->type != arg_type)
> > arg_value = value_cast (info->type, arg_value);
> > @@ -2565,7 +2581,7 @@ riscv_return_value (struct gdbarch *gdbarch,
> > struct type *arg_type;
> >
> > arg_type = check_typedef (type);
> > - riscv_arg_location (gdbarch, &info, &call_info, arg_type);
> > + riscv_arg_location (gdbarch, &info, &call_info, arg_type, false);
> >
> > if (riscv_debug_infcall > 0)
> > {
> > --
> > 2.17.1
> >
>
> Otherwise looks fine.
>
> Thanks,
> Andrew
@@ -1737,6 +1737,9 @@ struct riscv_arg_info
then this offset will be set to 0. */
int c_offset;
} argloc[2];
+
+ /* TRUE if this is an unnamed argument. */
+ bool is_unnamed;
};
/* Information about a set of registers being used for passing arguments as
@@ -1932,6 +1935,12 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo,
{
int len = (ainfo->length > cinfo->xlen) ? cinfo->xlen : ainfo->length;
+ /* Unnamed arguments in registers that require 2*XLEN alignment are
+ passed in an aligned register pair. */
+ if (ainfo->is_unnamed && (ainfo->align == cinfo->xlen * 2)
+ && cinfo->int_regs.next_regnum & 1)
+ cinfo->int_regs.next_regnum++;
+
if (!riscv_assign_reg_location (&ainfo->argloc[0],
&cinfo->int_regs, len, 0))
riscv_assign_stack_location (&ainfo->argloc[0],
@@ -2222,11 +2231,12 @@ static void
riscv_arg_location (struct gdbarch *gdbarch,
struct riscv_arg_info *ainfo,
struct riscv_call_info *cinfo,
- struct type *type)
+ struct type *type, bool is_unnamed)
{
ainfo->type = type;
ainfo->length = TYPE_LENGTH (ainfo->type);
ainfo->align = riscv_type_alignment (ainfo->type);
+ ainfo->is_unnamed = is_unnamed;
ainfo->contents = nullptr;
switch (TYPE_CODE (ainfo->type))
@@ -2375,6 +2385,11 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
CORE_ADDR osp = sp;
+ struct type *ftype = check_typedef (value_type (function));
+
+ if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
+ ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
+
/* We'll use register $a0 if we're returning a struct. */
if (struct_return)
++call_info.int_regs.next_regnum;
@@ -2388,7 +2403,8 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
arg_value = args[i];
arg_type = check_typedef (value_type (arg_value));
- riscv_arg_location (gdbarch, info, &call_info, arg_type);
+ riscv_arg_location (gdbarch, info, &call_info, arg_type,
+ TYPE_VARARGS (ftype) && i >= TYPE_NFIELDS (ftype));
if (info->type != arg_type)
arg_value = value_cast (info->type, arg_value);
@@ -2565,7 +2581,7 @@ riscv_return_value (struct gdbarch *gdbarch,
struct type *arg_type;
arg_type = check_typedef (type);
- riscv_arg_location (gdbarch, &info, &call_info, arg_type);
+ riscv_arg_location (gdbarch, &info, &call_info, arg_type, false);
if (riscv_debug_infcall > 0)
{