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

Message ID 20180820092933.83224-3-alan.hayward@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Aug. 20, 2018, 9:29 a.m. UTC
  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.

2018-08-20  Alan Hayward  <alan.hayward@arm.com>

	* aarch64-tdep.c
	(aapcs_is_vfp_call_or_return_candidate): Make static
	(pass_in_v_or_stack): Remove function.
	(pass_in_v_vfp_candidate): New function.
	(aarch64_push_dummy_call): Check for float register candidates.
---
 gdb/aarch64-tdep.c | 149 +++++++++++++++++++++++++++--------------------------
 1 file changed, 75 insertions(+), 74 deletions(-)
  

Comments

Simon Marchi Aug. 28, 2018, 3:58 p.m. UTC | #1
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
  

Patch

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index d3ea13f6f6..45dde8a37e 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1329,7 +1329,7 @@  aapcs_is_vfp_call_or_return_candidate_1 (struct type *type,
 
    Note that HFAs and HVAs can include nested structures and arrays.  */
 
-bool
+static bool
 aapcs_is_vfp_call_or_return_candidate (struct type *type, int *count,
 				       struct type **fundamental_type)
 {
@@ -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;
+
+    default:
+      return false;
+    }
 }
 
 /* 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"));
+	    }
+	  else
+	    {
+	      info.nsrn = 8;
+	      pass_on_stack (&info, arg_type, arg);
+	    }
+	  continue;
+	}
+
       switch (TYPE_CODE (arg_type))
 	{
 	case TYPE_CODE_INT:
@@ -1648,68 +1707,10 @@  aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	  pass_in_x_or_stack (gdbarch, regcache, &info, arg_type, arg);
 	  break;
 
-	case TYPE_CODE_COMPLEX:
-	  if (info.nsrn <= 6)
-	    {
-	      const bfd_byte *buf = value_contents (arg);
-	      struct type *target_type =
-		check_typedef (TYPE_TARGET_TYPE (arg_type));
-
-	      pass_in_v (gdbarch, regcache, &info,
-			 TYPE_LENGTH (target_type), buf);
-	      pass_in_v (gdbarch, regcache, &info,
-			 TYPE_LENGTH (target_type),
-			 buf + TYPE_LENGTH (target_type));
-	    }
-	  else
-	    {
-	      info.nsrn = 8;
-	      pass_on_stack (&info, arg_type, arg);
-	    }
-	  break;
-	case TYPE_CODE_FLT:
-	  pass_in_v_or_stack (gdbarch, regcache, &info, arg_type, arg);
-	  break;
-
 	case TYPE_CODE_STRUCT:
 	case TYPE_CODE_ARRAY:
 	case TYPE_CODE_UNION:
-	  if (is_hfa_or_hva (arg_type))
-	    {
-	      int elements = TYPE_NFIELDS (arg_type);
-
-	      /* Homogeneous Aggregates */
-	      if (info.nsrn + elements < 8)
-		{
-		  int i;
-
-		  for (i = 0; i < elements; i++)
-		    {
-		      /* We know that we have sufficient registers
-			 available therefore this will never fallback
-			 to the stack.  */
-		      struct value *field =
-			value_primitive_field (arg, 0, i, arg_type);
-		      struct type *field_type =
-			check_typedef (value_type (field));
-
-		      pass_in_v_or_stack (gdbarch, regcache, &info,
-					  field_type, field);
-		    }
-		}
-	      else
-		{
-		  info.nsrn = 8;
-		  pass_on_stack (&info, arg_type, arg);
-		}
-	    }
-	  else if (TYPE_CODE (arg_type) == TYPE_CODE_ARRAY
-		   && TYPE_VECTOR (arg_type) && (len == 16 || len == 8))
-	    {
-	      /* Short vector types are passed in V registers.  */
-	      pass_in_v_or_stack (gdbarch, regcache, &info, arg_type, arg);
-	    }
-	  else if (len > 16)
+	  if (len > 16)
 	    {
 	      /* PCS B.7 Aggregates larger than 16 bytes are passed by
 		 invisible reference.  */