Re: [PATCH 2/4] Aarch64: Float register detection for _push_dummy_call

[Simon Marchi <simon dot marchi at polymtl dot ca>]

On 2018-08-20 05:29, Alan Hayward wrote:
> Use aapcs_is_vfp_call_or_return_candidate to detect float register
> args, then pass in registers if there is room.
>
> pass_in_v_or_stack is no longer used. Remove it.

Hi Alan,

I didn't spot anything wrong, but then again I wouldn't trust myself to 
spot bugs in that code :).  I noted some nits:

> @@ -1522,19 +1522,57 @@ pass_in_x_or_stack (struct gdbarch *gdbarch,
> struct regcache *regcache,
>      }
>  }
>
> -/* Pass a value in a V register, or on the stack if insufficient are
> -   available.  */
> -
> -static void
> -pass_in_v_or_stack (struct gdbarch *gdbarch,
> -		    struct regcache *regcache,
> -		    struct aarch64_call_info *info,
> -		    struct type *type,
> -		    struct value *arg)
> +/* Pass a value, which is of type arg_type, in a V register.  Assumes
> value is a
> +   aapcs_is_vfp_call_or_return_candidate and there are enough spare V
> +   registers.  A return value of false is an error state as the value 
> will have
> +   been partially passed to the stack.  */
> +static bool
> +pass_in_v_vfp_candidate (struct gdbarch *gdbarch, struct regcache 
> *regcache,
> +			 struct aarch64_call_info *info, struct type *arg_type,
> +			 struct value *arg)
>  {
> -  if (!pass_in_v (gdbarch, regcache, info, TYPE_LENGTH (type),
> -		  value_contents (arg)))
> -    pass_on_stack (info, type, arg);
> +  switch (TYPE_CODE (arg_type))
> +    {
> +    case TYPE_CODE_FLT:
> +      return pass_in_v (gdbarch, regcache, info, TYPE_LENGTH 
> (arg_type),
> +			value_contents (arg));
> +      break;
> +
> +    case TYPE_CODE_COMPLEX:
> +      {
> +	const bfd_byte *buf = value_contents (arg);
> +	struct type *target_type = check_typedef (TYPE_TARGET_TYPE 
> (arg_type));
> +
> +	if (!pass_in_v (gdbarch, regcache, info, TYPE_LENGTH (target_type),
> +			buf))
> +	  return false;
> +
> +	return pass_in_v (gdbarch, regcache, info, TYPE_LENGTH (target_type),
> +			  buf + TYPE_LENGTH (target_type));
> +      }
> +
> +    case TYPE_CODE_ARRAY:
> +      if (TYPE_VECTOR (arg_type))
> +	return pass_in_v (gdbarch, regcache, info, TYPE_LENGTH (arg_type),
> +			  value_contents (arg));
> +      /* fall through.  */
> +
> +    case TYPE_CODE_STRUCT:
> +    case TYPE_CODE_UNION:
> +      for (int i = 0; i < TYPE_NFIELDS (arg_type); i++)
> +	{
> +	    struct value *field = value_primitive_field (arg, 0, i, 
> arg_type);
> +	    struct type *field_type = check_typedef (value_type (field));
> +
> +	    if (!pass_in_v_vfp_candidate (gdbarch, regcache, info, 
> field_type,
> +					  field))
> +	    return false;
> +	}
> +	return true;

I think the last line should have one fewer level of indentation.

>  /* Implement the "push_dummy_call" gdbarch method.  */
> @@ -1623,12 +1661,33 @@ aarch64_push_dummy_call (struct gdbarch
> *gdbarch, struct value *function,
>    for (argnum = 0; argnum < nargs; argnum++)
>      {
>        struct value *arg = args[argnum];
> -      struct type *arg_type;
> -      int len;
> +      struct type *arg_type, *fundamental_type;
> +      int len, elements;
>
>        arg_type = check_typedef (value_type (arg));
>        len = TYPE_LENGTH (arg_type);
>
> +      /* If arg can be passed in v registers as per the AAPCS64, then 
> do so if
> +	 if there are enough spare registers.  */
> +      if (aapcs_is_vfp_call_or_return_candidate (arg_type, &elements,
> +						 &fundamental_type))
> +	{
> +	  if (info.nsrn + elements <= 8)
> +	    {
> +	      /* We know that we have sufficient registers available 
> therefore
> +		 this will never need to fallback to the stack.  */
> +	      if (!pass_in_v_vfp_candidate (gdbarch, regcache, &info, 
> arg_type,
> +					    arg))
> +		error (_("Failed to push args"));

I'm wondering if this could even be a gdb_assert, since it would be a 
logic error in GDB.  We first checked that there are were enough spare 
registers to fit the argument.  So if we fail to pass the argument in 
registers, it means our check was wrong, or something like that.  In 
other words, this call failing can't result from bad input.

Simon