[3/3] RISC-V: gdb.base/gnu_vector fixes.

  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(-)
  

* 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