[PATCH 0/4] Aarch64: Correctly support args passed in float registers.

[PATCH 0/4] Aarch64: Correctly support args passed in float registers.

[Alan Hayward]

GDB currently does not adhere to the AAPCS spec when passing structures that
contain float arguments. It works for simple structures but does not correctly
detect nested structures.
This is shown by the failing tests in the recently added
infcall-nested-structs.exp. See PR gdb/22943.

Patch 1 contains a new function for correctly detecting float args.
I wrote this by taking the equivalent function in GCC and modifying it until
it fit into GDB, then simplified and cleaned up.
Patches 2 and 3 then use this function.

Patch 4 extends the infcall-nested-structs test to cover structures with 4
and 5 fields - 4 fields is the maximum number of fields that can be passed
via float args.

For the full spec see:
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042f/IHI0042F_aapcs.pdf


Alan Hayward (4):
  Aarch64: Add function to detect args passed in float registers
  Aarch64: Float register detection for _push_dummy_call
  Aarch64: Float register detection for return values.
  infcall-nested-structs: Test up to five fields

 gdb/aarch64-tdep.c                                | 444 ++++++++++++----------
 gdb/testsuite/gdb.base/infcall-nested-structs.c   | 168 +++++---
 gdb/testsuite/gdb.base/infcall-nested-structs.exp |   5 +-
 3 files changed, 366 insertions(+), 251 deletions(-)

-- 
2.15.2 (Apple Git-101.1)

[PATCH 1/4] Aarch64: Func to detect args passed in float regs

[Alan Hayward]

aapcs_is_vfp_call_or_return_candidate is as an eventual replacement
for is_hfa_or_hva.

This function is based on the GCC code
gcc/config/aarch64/aarch64.c:aarch64_vfp_is_call_or_return_candidate ()

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

	* aarch64-tdep.c (HA_MAX_NUM_FLDS): New macro.
	(aapcs_is_vfp_call_or_return_candidate_1): New function.
	(aapcs_is_vfp_call_or_return_candidate): Likewise.
---
 gdb/aarch64-tdep.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 5c6eb98545..d3ea13f6f6 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -71,6 +71,10 @@
 #define AARCH64_B0_REGNUM (AARCH64_H0_REGNUM + 32)
 #define AARCH64_SVE_V0_REGNUM (AARCH64_B0_REGNUM + 32)
 
+/* A Homogeneous Floating-Point or Short-Vector Aggregate may have at most
+   four members.  */
+#define HA_MAX_NUM_FLDS		4
+
 /* All possible aarch64 target descriptors.  */
 struct target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1];
 
@@ -1209,6 +1213,143 @@ is_hfa_or_hva (struct type *ty)
   return 0;
 }
 
+/* Worker function for aapcs_is_vfp_call_or_return_candidate.
+
+   Return the number of register required, or -1 on failure.
+
+   When encountering a base element, if FUNDAMENTAL_TYPE is not set then set it
+   to the element, else fail if the type of this element does not match the
+   existing value.  */
+
+static int
+aapcs_is_vfp_call_or_return_candidate_1 (struct type *type,
+					 struct type **fundamental_type)
+{
+  if (type == nullptr)
+    return -1;
+
+  switch (TYPE_CODE (type))
+    {
+    case TYPE_CODE_FLT:
+      if (TYPE_LENGTH (type) > 16)
+	return -1;
+
+      if (*fundamental_type == nullptr)
+	*fundamental_type = type;
+      else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type)
+	       || TYPE_CODE (type) != TYPE_CODE (*fundamental_type))
+	return -1;
+
+      return 1;
+
+    case TYPE_CODE_COMPLEX:
+      {
+	struct type *target_type = check_typedef (TYPE_TARGET_TYPE (type));
+	if (TYPE_LENGTH (target_type) > 16)
+	  return -1;
+
+	if (*fundamental_type == nullptr)
+	  *fundamental_type = target_type;
+	else if (TYPE_LENGTH (target_type) != TYPE_LENGTH (*fundamental_type)
+		 || TYPE_CODE (target_type) != TYPE_CODE (*fundamental_type))
+	  return -1;
+
+	return 2;
+      }
+
+    case TYPE_CODE_ARRAY:
+      {
+	if (TYPE_VECTOR (type))
+	  {
+	    if (TYPE_LENGTH (type) != 8 && TYPE_LENGTH (type) != 16)
+	      return -1;
+
+	    if (*fundamental_type == nullptr)
+	      *fundamental_type = type;
+	    else if (TYPE_LENGTH (type) != TYPE_LENGTH (*fundamental_type)
+		     || TYPE_CODE (type) != TYPE_CODE (*fundamental_type))
+	      return -1;
+
+	    return 1;
+	  }
+	else
+	  {
+	    struct type *target_type = TYPE_TARGET_TYPE (type);
+	    int count = aapcs_is_vfp_call_or_return_candidate_1
+			  (target_type, fundamental_type);
+
+	    if (count == -1)
+	      return count;
+
+	    count *= TYPE_LENGTH (type);
+	      return count;
+	  }
+      }
+
+    case TYPE_CODE_STRUCT:
+    case TYPE_CODE_UNION:
+      {
+	int count = 0;
+
+	for (int i = 0; i < TYPE_NFIELDS (type); i++)
+	  {
+	    struct type *member = check_typedef (TYPE_FIELD_TYPE (type, i));
+
+	    int sub_count = aapcs_is_vfp_call_or_return_candidate_1
+			      (member, fundamental_type);
+	    if (sub_count == -1)
+	      return -1;
+	    count += sub_count;
+	  }
+	return count;
+      }
+
+    default:
+      break;
+    }
+
+  return -1;
+}
+
+/* Return true if an argument, whose type is described by TYPE, can be passed or
+   returned in simd/fp registers, providing enough parameter passing registers
+   are available.  This is as described in the AAPCS64.
+
+   Upon successful return, *COUNT returns the number of needed registers,
+   *FUNDAMENTAL_TYPE contains the type of those registers.
+
+   Candidate as per the AAPCS64 5.4.2.C is either a:
+   - float.
+   - short-vector.
+   - HFA (Homogeneous Floating-point Aggregate, 4.3.5.1). A Composite type where
+     all the members are floats and has at most 4 members.
+   - HVA (Homogeneous Short-vector Aggregate, 4.3.5.2). A Composite type where
+     all the members are short vectors and has at most 4 members.
+   - Complex (7.1.1)
+
+   Note that HFAs and HVAs can include nested structures and arrays.  */
+
+bool
+aapcs_is_vfp_call_or_return_candidate (struct type *type, int *count,
+				       struct type **fundamental_type)
+{
+  if (type == nullptr)
+    return false;
+
+  *fundamental_type = nullptr;
+
+  int ag_count = aapcs_is_vfp_call_or_return_candidate_1 (type,
+							  fundamental_type);
+
+  if (ag_count > 0 && ag_count <= HA_MAX_NUM_FLDS)
+    {
+      *count = ag_count;
+      return true;
+    }
+  else
+    return false;
+}
+
 /* AArch64 function call information structure.  */
 struct aarch64_call_info
 {
-- 
2.15.2 (Apple Git-101.1)

Re: [PATCH 1/4] Aarch64: Func to detect args passed in float regs

[Simon Marchi]

On 2018-08-20 05:29, Alan Hayward wrote:
> aapcs_is_vfp_call_or_return_candidate is as an eventual replacement
> for is_hfa_or_hva.
> 
> This function is based on the GCC code
> gcc/config/aarch64/aarch64.c:aarch64_vfp_is_call_or_return_candidate ()
> 
> 2018-08-20  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* aarch64-tdep.c (HA_MAX_NUM_FLDS): New macro.
> 	(aapcs_is_vfp_call_or_return_candidate_1): New function.
> 	(aapcs_is_vfp_call_or_return_candidate): Likewise.

I'm not an AArch64 expert, but I didn't spot anything suspicious.  The 
documentation
helps a lot to understand, thanks for that.


> +/* Return true if an argument, whose type is described by TYPE, can
> be passed or
> +   returned in simd/fp registers, providing enough parameter passing 
> registers
> +   are available.  This is as described in the AAPCS64.
> +
> +   Upon successful return, *COUNT returns the number of needed 
> registers,
> +   *FUNDAMENTAL_TYPE contains the type of those registers.
> +
> +   Candidate as per the AAPCS64 5.4.2.C is either a:
> +   - float.
> +   - short-vector.
> +   - HFA (Homogeneous Floating-point Aggregate, 4.3.5.1). A Composite
> type where
> +     all the members are floats and has at most 4 members.
> +   - HVA (Homogeneous Short-vector Aggregate, 4.3.5.2). A Composite 
> type where
> +     all the members are short vectors and has at most 4 members.
> +   - Complex (7.1.1)
> +
> +   Note that HFAs and HVAs can include nested structures and arrays.  
> */
> +
> +bool

static?

Simon

Re: [PATCH 1/4] Aarch64: Func to detect args passed in float regs

[Alan Hayward]

> On 28 Aug 2018, at 16:43, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> 
> On 2018-08-20 05:29, Alan Hayward wrote:
>> aapcs_is_vfp_call_or_return_candidate is as an eventual replacement
>> for is_hfa_or_hva.
>> This function is based on the GCC code
>> gcc/config/aarch64/aarch64.c:aarch64_vfp_is_call_or_return_candidate ()
>> 2018-08-20  Alan Hayward  <alan.hayward@arm.com>
>> 	* aarch64-tdep.c (HA_MAX_NUM_FLDS): New macro.
>> 	(aapcs_is_vfp_call_or_return_candidate_1): New function.
>> 	(aapcs_is_vfp_call_or_return_candidate): Likewise.
> 
> I'm not an AArch64 expert, but I didn't spot anything suspicious.  The documentation
> helps a lot to understand, thanks for that.
> 
> 
>> +/* Return true if an argument, whose type is described by TYPE, can
>> be passed or
>> +   returned in simd/fp registers, providing enough parameter passing registers
>> +   are available.  This is as described in the AAPCS64.
>> +
>> +   Upon successful return, *COUNT returns the number of needed registers,
>> +   *FUNDAMENTAL_TYPE contains the type of those registers.
>> +
>> +   Candidate as per the AAPCS64 5.4.2.C is either a:
>> +   - float.
>> +   - short-vector.
>> +   - HFA (Homogeneous Floating-point Aggregate, 4.3.5.1). A Composite
>> type where
>> +     all the members are floats and has at most 4 members.
>> +   - HVA (Homogeneous Short-vector Aggregate, 4.3.5.2). A Composite type where
>> +     all the members are short vectors and has at most 4 members.
>> +   - Complex (7.1.1)
>> +
>> +   Note that HFAs and HVAs can include nested structures and arrays.  */
>> +
>> +bool
> 
> static?
> 

Yes, it should be static. But, in this patch nothing calls the function, which
causes a compile failure. I make it static in patch 2.
A little awkward, but seemed the best way to do it.


Alan.

Re: [PATCH 1/4] Aarch64: Func to detect args passed in float regs

[Simon Marchi]

On 2018-08-28 11:49, Alan Hayward wrote:
>> On 28 Aug 2018, at 16:43, Simon Marchi <simon.marchi@polymtl.ca> 
>> wrote:
>> 
>> On 2018-08-20 05:29, Alan Hayward wrote:
>>> aapcs_is_vfp_call_or_return_candidate is as an eventual replacement
>>> for is_hfa_or_hva.
>>> This function is based on the GCC code
>>> gcc/config/aarch64/aarch64.c:aarch64_vfp_is_call_or_return_candidate 
>>> ()
>>> 2018-08-20  Alan Hayward  <alan.hayward@arm.com>
>>> 	* aarch64-tdep.c (HA_MAX_NUM_FLDS): New macro.
>>> 	(aapcs_is_vfp_call_or_return_candidate_1): New function.
>>> 	(aapcs_is_vfp_call_or_return_candidate): Likewise.
>> 
>> I'm not an AArch64 expert, but I didn't spot anything suspicious.  The 
>> documentation
>> helps a lot to understand, thanks for that.
>> 
>> 
>>> +/* Return true if an argument, whose type is described by TYPE, can
>>> be passed or
>>> +   returned in simd/fp registers, providing enough parameter passing 
>>> registers
>>> +   are available.  This is as described in the AAPCS64.
>>> +
>>> +   Upon successful return, *COUNT returns the number of needed 
>>> registers,
>>> +   *FUNDAMENTAL_TYPE contains the type of those registers.
>>> +
>>> +   Candidate as per the AAPCS64 5.4.2.C is either a:
>>> +   - float.
>>> +   - short-vector.
>>> +   - HFA (Homogeneous Floating-point Aggregate, 4.3.5.1). A 
>>> Composite
>>> type where
>>> +     all the members are floats and has at most 4 members.
>>> +   - HVA (Homogeneous Short-vector Aggregate, 4.3.5.2). A Composite 
>>> type where
>>> +     all the members are short vectors and has at most 4 members.
>>> +   - Complex (7.1.1)
>>> +
>>> +   Note that HFAs and HVAs can include nested structures and arrays. 
>>>  */
>>> +
>>> +bool
>> 
>> static?
>> 
> 
> Yes, it should be static. But, in this patch nothing calls the 
> function, which
> causes a compile failure. I make it static in patch 2.
> A little awkward, but seemed the best way to do it.
> 
> 
> Alan.

Ah, I see.  Another option would be to put __attribute__((unused)) 
temporarily.  But I'm fine with what you've done too.

Simon

[PATCH 4/4] infcall-nested-structs: Test up to five fields

[Alan Hayward]

Aarch64 can pass structures of up to four members of identical
types in float registers (See AAPCS 5.3 and 5.4). Expand test to
cover this.

Remove the need to specify an additional sets of structures if tB
is not defined.

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

gdb/testsuite/
	* gdb.base/infcall-nested-structs.c (struct struct01): Remove.
	(struct struct02): Likewise.
	(struct struct03): Likewise.
	(struct struct04): Likewise.
	(struct struct_01_01): New struct.
	(struct struct_01_02): Likewise.
	(struct struct_01_03): Likewise.
	(struct struct_01_04): Likewise.
	(struct struct_02_01): Likewise.
	(struct struct_02_02): Likewise.
	(struct struct_02_03): Likewise.
	(struct struct_02_04): Likewise.
	(struct struct_04_01): Likewise.
	(struct struct_04_02): Likewise.
	(struct struct_04_03): Likewise.
	(struct struct_04_04): Likewise.
	(struct struct_05_01): Likewise.
	(struct struct_05_02): Likewise.
	(struct struct_05_03): Likewise.
	(struct struct_05_04): Likewise.
	(cmp_struct01): Remove function.
	(cmp_struct02): Likewise.
	(cmp_struct03): Likewise.
	(cmp_struct04): Likewise.
	(cmp_struct_01_01): Add Function.
	(cmp_struct_01_02): Likewise.
	(cmp_struct_01_03): Likewise.
	(cmp_struct_01_04): Likewise.
	(cmp_struct_02_01): Likewise.
	(cmp_struct_02_02): Likewise.
	(cmp_struct_02_03): Likewise.
	(cmp_struct_02_04): Likewise.
	(cmp_struct_04_01): Likewise.
	(cmp_struct_04_02): Likewise.
	(cmp_struct_04_03): Likewise.
	(cmp_struct_04_04): Likewise.
	(cmp_struct_05_01): Likewise.
	(cmp_struct_05_02): Likewise.
	(cmp_struct_05_03): Likewise.
	(cmp_struct_05_04): Likewise.
	(call_all): Add new structs.
	* gdb.base/infcall-nested-structs.exp: Likewise.
---
 gdb/testsuite/gdb.base/infcall-nested-structs.c   | 168 ++++++++++++++++------
 gdb/testsuite/gdb.base/infcall-nested-structs.exp |   5 +-
 2 files changed, 126 insertions(+), 47 deletions(-)

diff --git a/gdb/testsuite/gdb.base/infcall-nested-structs.c b/gdb/testsuite/gdb.base/infcall-nested-structs.c
index f024ac6163..01863cca80 100644
--- a/gdb/testsuite/gdb.base/infcall-nested-structs.c
+++ b/gdb/testsuite/gdb.base/infcall-nested-structs.c
@@ -17,13 +17,13 @@
 
 /* This file is used for testing GDBs ability to pass structures to, and
    return structures from, functions.  All of the structures in this test
-   are special in that they are small structures containing only 1 or 2
+   are special in that they are small structures containing from 1 up to 5
    scalar fields, the fields can be inside nested structures, and there can
    be empty structures around too.
 
-   This test was originally written for RiscV which has some special ABI
-   rules for structures like these, however, there should be no harm in
-   running these tests on other targets, though in many cases the
+   This test is specifically written for RiscV and Aarch64, which both have
+   special ABI rules for structures like these, however, there should be no harm
+   in running these tests on other targets, though in many cases the
    structures will treated no differently to the structures already covered
    in the structs.exp test script.  */
 
@@ -61,69 +61,133 @@ typedef long double _Complex tldc;
 #define REF_VAL(NAME) struct NAME ref_val_ ## NAME
 #define ES(NAME) struct { } NAME
 
+/* Test is either for a single type or two differing types.  */
 #if defined tA && ! defined tB
+#define tB tA
+#endif
+#if ! defined tB
+#error "Incorrect configuration of tA and tB defines"
+#endif
 
 /* Structures with a single field nested to various depths, along with
    some empty structures.  */
-struct struct01 { ES(es1); struct { struct { tA a; } s1; } s2; };
-struct struct02 { tA a; struct { struct { ES(es1); } s1; } s2; };
-struct struct03 { struct { struct { ES(es1); } s1; } s2; ES(es1); struct { struct { tA a; } s3; } s4;};
-struct struct04 { ES(es1); ES(es2); tA a; ES(es3); };
+struct struct_01_01 { ES(es1); struct { struct { tA a; } s1; } s2; };
+struct struct_01_02 { tA a; struct { struct { ES(es1); } s1; } s2; };
+struct struct_01_03 { struct { struct { ES(es1); } s1; } s2; ES(es1); struct { struct { tA a; } s3; } s4;};
+struct struct_01_04 { ES(es1); ES(es2); tA a; ES(es3); };
+
+/* Structures with two fields nested to various depths, along with
+   some empty structures.  */
+struct struct_02_01 { ES(es1); struct { struct { tA a; tB b; } s1; } s2; };
+struct struct_02_02 { tA a; struct { struct { ES(es1); } s1; } s2; tB b; };
+struct struct_02_03 { struct { struct { ES(es1); } s1; } s2; ES(es1); struct { struct { tA a; } s3; } s4; struct { struct { tB b; } s5; } s6;};
+struct struct_02_04 { ES(es1); ES(es2); tA a; ES(es3); tB b; };
+
+/* Structures with four fields nested to various depths, along with
+   some empty structures.  */
+struct struct_04_01 { ES(es1); struct { struct { tA a; tB b; tA c; tB d; } s1; } s2; };
+struct struct_04_02 { tA a; struct { struct { ES(es1); } s1; } s2; tB b; struct { struct { ES(es1); } s2; } s3; tA c; struct { struct { ES(es2); } s4; } s5; tB d;};
+struct struct_04_03 { struct { struct { ES(es1); } s1; } s2; ES(es1); struct { struct { tA a; } s3; } s4; struct { struct { tB b; } s5; } s6; struct { struct { tA c; } s7; } s8; struct { struct { tB d; } s9; } s10;};
+struct struct_04_04 { ES(es1); ES(es2); tA a; ES(es3); tB b; ES(es4); tA c; ES(es5); tB d; };
+
+/* Structures with five fields nested to various depths, along with
+   some empty structures.  */
+struct struct_05_01 { ES(es1); struct { struct { tA a; tB b; tA c; tB d; tA e; } s1; } s2; };
+struct struct_05_02 { tA a; struct { struct { ES(es1); } s1; } s2; tB b; struct { struct { ES(es1); } s2; } s3; tA c; struct { struct { ES(es2); } s4; } s5; tB d; struct { struct { ES(es2); } s6; } s7; tB e;};
+struct struct_05_03 { struct { struct { ES(es1); } s1; } s2; ES(es1); struct { struct { tA a; } s3; } s4; struct { struct { tB b; } s5; } s6; struct { struct { tA c; } s7; } s8; struct { struct { tB d; } s9; } s10; struct { struct { tA e; } s11; } s12;};
+struct struct_05_04 { ES(es1); ES(es2); tA a; ES(es3); tB b; ES(es4); tA c; ES(es5); tB d; ES(es6); tA e; };
 
-int cmp_struct01 (struct struct01 a, struct struct01 b)
+int cmp_struct_01_01 (struct struct_01_01 a, struct struct_01_01 b)
 { return a.s2.s1.a == b.s2.s1.a; }
 
-int cmp_struct02 (struct struct02 a, struct struct02 b)
+int cmp_struct_01_02 (struct struct_01_02 a, struct struct_01_02 b)
 { return a.a == b.a; }
 
-int cmp_struct03 (struct struct03 a, struct struct03 b)
+int cmp_struct_01_03 (struct struct_01_03 a, struct struct_01_03 b)
 { return a.s4.s3.a == b.s4.s3.a; }
 
-int cmp_struct04 (struct struct04 a, struct struct04 b)
+int cmp_struct_01_04 (struct struct_01_04 a, struct struct_01_04 b)
 { return a.a == b.a; }
 
-REF_VAL(struct01) = { {}, { { 'a' } } };
-REF_VAL(struct02) = { 'a', { { {} } } };
-REF_VAL(struct03) = { { { {} } }, {}, { { 'a' } } };
-REF_VAL(struct04) = { {}, {}, 'a', {} };
+int cmp_struct_02_01 (struct struct_02_01 a, struct struct_02_01 b)
+{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == a.s2.s1.b; }
 
-#elif defined tA && defined tB
+int cmp_struct_02_02 (struct struct_02_02 a, struct struct_02_02 b)
+{ return a.a == b.a && a.b == b.b; }
 
-/* These structures all have 2 fields, nested to various depths, along
-   with some empty structures.  */
-struct struct01 { struct { tA a; } s1; ES (e1); struct { struct { tB b; } s2;} s3;};
-struct struct02 { struct { struct { tA a; } s1; ES(e1); } s2; struct { struct { tB b; } s3;} s4; ES(e2);};
-struct struct03 { ES(e1); tA a; ES (e2); struct { struct { tB b; } s2;} s3;};
-struct struct04 { ES(e1); ES (e2); struct { struct { struct { tA a; struct { ES(e3); } s1; tB b; } s2; } s3;} s4;};
+int cmp_struct_02_03 (struct struct_02_03 a, struct struct_02_03 b)
+{ return a.s4.s3.a == b.s4.s3.a && a.s6.s5.b == b.s6.s5.b; }
 
-int cmp_struct01 (struct struct01 a, struct struct01 b)
-{ return a.s1.a == b.s1.a && a.s3.s2.b == b.s3.s2.b; }
+int cmp_struct_02_04 (struct struct_02_04 a, struct struct_02_04 b)
+{ return a.a == b.a && a.b == b.b; }
 
-int cmp_struct02 (struct struct02 a, struct struct02 b)
-{ return a.s2.s1.a == b.s2.s1.a && a.s4.s3.b == b.s4.s3.b; }
+int cmp_struct_04_01 (struct struct_04_01 a, struct struct_04_01 b)
+{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == a.s2.s1.b
+	 && a.s2.s1.c == b.s2.s1.c && a.s2.s1.d == a.s2.s1.d; }
 
-int cmp_struct03 (struct struct03 a, struct struct03 b)
-{ return a.a == b.a && a.s3.s2.b == b.s3.s2.b; }
+int cmp_struct_04_02 (struct struct_04_02 a, struct struct_04_02 b)
+{ return a.a == b.a && a.b == b.b && a.c == b.c && a.d == b.d; }
 
-int cmp_struct04 (struct struct04 a, struct struct04 b)
-{ return a.s4.s3.s2.a == b.s4.s3.s2.a && a.s4.s3.s2.b == b.s4.s3.s2.b; }
+int cmp_struct_04_03 (struct struct_04_03 a, struct struct_04_03 b)
+{ return a.s4.s3.a == b.s4.s3.a && a.s6.s5.b == b.s6.s5.b
+	 && a.s8.s7.c == b.s8.s7.c && a.s10.s9.d == b.s10.s9.d; }
 
-REF_VAL(struct01) = { { 'a' }, {}, { { '1' } } };
-REF_VAL(struct02) = { { { 'a' }, {} }, { { '1' } }, {} };
-REF_VAL(struct03) = { {}, 'a', {}, { { '1' } } };
-REF_VAL(struct04) = { {}, {}, { { { 'a', {}, '1'} } } } ;
+int cmp_struct_04_04 (struct struct_04_04 a, struct struct_04_04 b)
+{ return a.a == b.a && a.b == b.b && a.c == b.c && a.d == b.d; }
 
-#else
+int cmp_struct_05_01 (struct struct_05_01 a, struct struct_05_01 b)
+{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == a.s2.s1.b
+	 && a.s2.s1.c == b.s2.s1.c && a.s2.s1.d == a.s2.s1.d
+	 && a.s2.s1.e == b.s2.s1.e; }
 
-#error "Incorrect configuration of tA and tB defines"
+int cmp_struct_05_02 (struct struct_05_02 a, struct struct_05_02 b)
+{ return a.a == b.a && a.b == b.b && a.c == b.c && a.d == b.d && a.e == b.e; }
 
-#endif
+int cmp_struct_05_03 (struct struct_05_03 a, struct struct_05_03 b)
+{ return a.s4.s3.a == b.s4.s3.a && a.s6.s5.b == b.s6.s5.b
+	 && a.s8.s7.c == b.s8.s7.c && a.s10.s9.d == b.s10.s9.d
+	 && a.s12.s11.e == b.s12.s11.e; }
+
+int cmp_struct_05_04 (struct struct_05_04 a, struct struct_05_04 b)
+{ return a.a == b.a && a.b == b.b && a.c == b.c && a.d == b.d && a.e == b.e; }
+
+REF_VAL(struct_01_01) = { {}, { { 'a' } } };
+REF_VAL(struct_01_02) = { 'a', { { {} } } };
+REF_VAL(struct_01_03) = { { { {} } }, {}, { { 'a' } } };
+REF_VAL(struct_01_04) = { {}, {}, 'a', {} };
+
+REF_VAL(struct_02_01) = { {}, { { 'a', 'b' } } };
+REF_VAL(struct_02_02) = { 'a', { { {} } }, 'b' };
+REF_VAL(struct_02_03) = { { { {} } }, {}, { { 'a' } }, { { 'b' } } };
+REF_VAL(struct_02_04) = { {}, {}, 'a', {}, 'b' };
+
+REF_VAL(struct_04_01) = { {}, { { 'a', 'b', 'c', 'd' } } };
+REF_VAL(struct_04_02) = { 'a', { { {} } }, 'b', { { {} } }, 'c', { { {} } }, 'd' };
+REF_VAL(struct_04_03) = { { { {} } }, {}, { { 'a' } }, { { 'b' } }, { { 'c' } }, { { 'd' } } };
+REF_VAL(struct_04_04) = { {}, {}, 'a', {}, 'b', {}, 'c', {}, 'd' };
+
+REF_VAL(struct_05_01) = { {}, { { 'a', 'b', 'c', 'd', 'e' } } };
+REF_VAL(struct_05_02) = { 'a', { { {} } }, 'b', { { {} } }, 'c', { { {} } }, 'd', { { {} } }, 'e' };
+REF_VAL(struct_05_03) = { { { {} } }, {}, { { 'a' } }, { { 'b' } }, { { 'c' } }, { { 'd' } }, { { 'e' } } };
+REF_VAL(struct_05_04) = { {}, {}, 'a', {}, 'b', {}, 'c', {}, 'd', {}, 'e' };
 
 /* Create all of the functions GDB will call to check functionality.  */
-MAKE_CHECK_FUNCS(struct01)
-MAKE_CHECK_FUNCS(struct02)
-MAKE_CHECK_FUNCS(struct03)
-MAKE_CHECK_FUNCS(struct04)
+MAKE_CHECK_FUNCS(struct_01_01)
+MAKE_CHECK_FUNCS(struct_01_02)
+MAKE_CHECK_FUNCS(struct_01_03)
+MAKE_CHECK_FUNCS(struct_01_04)
+MAKE_CHECK_FUNCS(struct_02_01)
+MAKE_CHECK_FUNCS(struct_02_02)
+MAKE_CHECK_FUNCS(struct_02_03)
+MAKE_CHECK_FUNCS(struct_02_04)
+MAKE_CHECK_FUNCS(struct_04_01)
+MAKE_CHECK_FUNCS(struct_04_02)
+MAKE_CHECK_FUNCS(struct_04_03)
+MAKE_CHECK_FUNCS(struct_04_04)
+MAKE_CHECK_FUNCS(struct_05_01)
+MAKE_CHECK_FUNCS(struct_05_02)
+MAKE_CHECK_FUNCS(struct_05_03)
+MAKE_CHECK_FUNCS(struct_05_04)
 
 #define CALL_LINE(NAME) val += check_arg_ ## NAME (rtn_str_ ## NAME ())
 
@@ -132,10 +196,22 @@ call_all ()
 {
   int val;
 
-  CALL_LINE(struct01);
-  CALL_LINE(struct02);
-  CALL_LINE(struct03);
-  CALL_LINE(struct04);
+  CALL_LINE(struct_01_01);
+  CALL_LINE(struct_01_02);
+  CALL_LINE(struct_01_03);
+  CALL_LINE(struct_01_04);
+  CALL_LINE(struct_02_01);
+  CALL_LINE(struct_02_02);
+  CALL_LINE(struct_02_03);
+  CALL_LINE(struct_02_04);
+  CALL_LINE(struct_04_01);
+  CALL_LINE(struct_04_02);
+  CALL_LINE(struct_04_03);
+  CALL_LINE(struct_04_04);
+  CALL_LINE(struct_05_01);
+  CALL_LINE(struct_05_02);
+  CALL_LINE(struct_05_03);
+  CALL_LINE(struct_05_04);
 
   return (val != 4);
 }
diff --git a/gdb/testsuite/gdb.base/infcall-nested-structs.exp b/gdb/testsuite/gdb.base/infcall-nested-structs.exp
index e4cee5afad..b10e6d21eb 100644
--- a/gdb/testsuite/gdb.base/infcall-nested-structs.exp
+++ b/gdb/testsuite/gdb.base/infcall-nested-structs.exp
@@ -102,7 +102,10 @@ proc start_nested_structs_test { types } {
 proc run_tests {} {
     global gdb_prompt
 
-    foreach {name} {struct01 struct02 struct03 struct04} {
+    foreach {name} {struct_01_01 struct_01_02 struct_01_03 struct_01_04
+                    struct_02_01 struct_02_02 struct_02_03 struct_02_04
+                    struct_04_01 struct_04_02 struct_04_03 struct_04_04
+                    struct_05_01 struct_05_02 struct_05_03 struct_05_04} {
 	gdb_test "p/d check_arg_${name} (ref_val_${name})" "= 1"
 
 	set refval [ get_valueof "" "ref_val_${name}" "" ]
-- 
2.15.2 (Apple Git-101.1)

Re: [PATCH 4/4] infcall-nested-structs: Test up to five fields

[Simon Marchi]

On 2018-08-20 05:29, Alan Hayward wrote:
> Aarch64 can pass structures of up to four members of identical
> types in float registers (See AAPCS 5.3 and 5.4). Expand test to
> cover this.
> 
> Remove the need to specify an additional sets of structures if tB
> is not defined.

Thanks, LGTM.

Simon

[PATCH 3/4] Aarch64: Float register detection for return values.

[Alan Hayward]

Use aapcs_is_vfp_call_or_return_candidate to detect float register
args.

Remove the no longer used is_hfa_or_hva().

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

	PR gdb/22943:
	* aarch64-tdep.c (is_hfa_or_hva): Remove function.
	(aarch64_extract_return_value): Use
	aapcs_is_vfp_call_or_return_candidate.
	(aarch64_return_in_memory): Likewise.
	(aarch64_store_return_value): Likewise.
---
 gdb/aarch64-tdep.c | 200 +++++++++++++----------------------------------------
 1 file changed, 47 insertions(+), 153 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 45dde8a37e..7ff97f9078 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1151,68 +1151,6 @@ aarch64_type_align (struct type *t)
     }
 }
 
-/* Return 1 if *TY is a homogeneous floating-point aggregate or
-   homogeneous short-vector aggregate as defined in the AAPCS64 ABI
-   document; otherwise return 0.  */
-
-static int
-is_hfa_or_hva (struct type *ty)
-{
-  switch (TYPE_CODE (ty))
-    {
-    case TYPE_CODE_ARRAY:
-      {
-	struct type *target_ty = TYPE_TARGET_TYPE (ty);
-
-	if (TYPE_VECTOR (ty))
-	  return 0;
-
-	if (TYPE_LENGTH (ty) <= 4 /* HFA or HVA has at most 4 members.  */
-	    && (TYPE_CODE (target_ty) == TYPE_CODE_FLT /* HFA */
-		|| (TYPE_CODE (target_ty) == TYPE_CODE_ARRAY /* HVA */
-		    && TYPE_VECTOR (target_ty))))
-	  return 1;
-	break;
-      }
-
-    case TYPE_CODE_UNION:
-    case TYPE_CODE_STRUCT:
-      {
-	/* HFA or HVA has at most four members.  */
-	if (TYPE_NFIELDS (ty) > 0 && TYPE_NFIELDS (ty) <= 4)
-	  {
-	    struct type *member0_type;
-
-	    member0_type = check_typedef (TYPE_FIELD_TYPE (ty, 0));
-	    if (TYPE_CODE (member0_type) == TYPE_CODE_FLT
-		|| (TYPE_CODE (member0_type) == TYPE_CODE_ARRAY
-		    && TYPE_VECTOR (member0_type)))
-	      {
-		int i;
-
-		for (i = 0; i < TYPE_NFIELDS (ty); i++)
-		  {
-		    struct type *member1_type;
-
-		    member1_type = check_typedef (TYPE_FIELD_TYPE (ty, i));
-		    if (TYPE_CODE (member0_type) != TYPE_CODE (member1_type)
-			|| (TYPE_LENGTH (member0_type)
-			    != TYPE_LENGTH (member1_type)))
-		      return 0;
-		  }
-		return 1;
-	      }
-	  }
-	return 0;
-      }
-
-    default:
-      break;
-    }
-
-  return 0;
-}
-
 /* Worker function for aapcs_is_vfp_call_or_return_candidate.
 
    Return the number of register required, or -1 on failure.
@@ -1988,14 +1926,30 @@ aarch64_extract_return_value (struct type *type, struct regcache *regs,
 {
   struct gdbarch *gdbarch = regs->arch ();
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  int elements;
+  struct type *fundamental_type;
 
-  if (TYPE_CODE (type) == TYPE_CODE_FLT)
+  if (aapcs_is_vfp_call_or_return_candidate (type, &elements,
+					     &fundamental_type))
     {
-      bfd_byte buf[V_REGISTER_SIZE];
-      int len = TYPE_LENGTH (type);
+      int len = TYPE_LENGTH (fundamental_type);
 
-      regs->cooked_read (AARCH64_V0_REGNUM, buf);
-      memcpy (valbuf, buf, len);
+      for (int i = 0; i < elements; i++)
+	{
+	  int regno = AARCH64_V0_REGNUM + i;
+	  bfd_byte buf[V_REGISTER_SIZE];
+
+	  if (aarch64_debug)
+	    {
+	      debug_printf ("read HFA or HVA return value element %d from %s\n",
+			    i + 1,
+			    gdbarch_register_name (gdbarch, regno));
+	    }
+	  regs->cooked_read (regno, buf);
+
+	  memcpy (valbuf, buf, len);
+	  valbuf += len;
+	}
     }
   else if (TYPE_CODE (type) == TYPE_CODE_INT
 	   || TYPE_CODE (type) == TYPE_CODE_CHAR
@@ -2023,53 +1977,6 @@ aarch64_extract_return_value (struct type *type, struct regcache *regs,
 	  valbuf += X_REGISTER_SIZE;
 	}
     }
-  else if (TYPE_CODE (type) == TYPE_CODE_COMPLEX)
-    {
-      int regno = AARCH64_V0_REGNUM;
-      bfd_byte buf[V_REGISTER_SIZE];
-      struct type *target_type = check_typedef (TYPE_TARGET_TYPE (type));
-      int len = TYPE_LENGTH (target_type);
-
-      regs->cooked_read (regno, buf);
-      memcpy (valbuf, buf, len);
-      valbuf += len;
-      regs->cooked_read (regno + 1, buf);
-      memcpy (valbuf, buf, len);
-      valbuf += len;
-    }
-  else if (is_hfa_or_hva (type))
-    {
-      int elements = TYPE_NFIELDS (type);
-      struct type *member_type = check_typedef (TYPE_FIELD_TYPE (type, 0));
-      int len = TYPE_LENGTH (member_type);
-      int i;
-
-      for (i = 0; i < elements; i++)
-	{
-	  int regno = AARCH64_V0_REGNUM + i;
-	  bfd_byte buf[V_REGISTER_SIZE];
-
-	  if (aarch64_debug)
-	    {
-	      debug_printf ("read HFA or HVA return value element %d from %s\n",
-			    i + 1,
-			    gdbarch_register_name (gdbarch, regno));
-	    }
-	  regs->cooked_read (regno, buf);
-
-	  memcpy (valbuf, buf, len);
-	  valbuf += len;
-	}
-    }
-  else if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type)
-	   && (TYPE_LENGTH (type) == 16 || TYPE_LENGTH (type) == 8))
-    {
-      /* Short vector is returned in V register.  */
-      gdb_byte buf[V_REGISTER_SIZE];
-
-      regs->cooked_read (AARCH64_V0_REGNUM, buf);
-      memcpy (valbuf, buf, TYPE_LENGTH (type));
-    }
   else
     {
       /* For a structure or union the behaviour is as if the value had
@@ -2098,8 +2005,11 @@ static int
 aarch64_return_in_memory (struct gdbarch *gdbarch, struct type *type)
 {
   type = check_typedef (type);
+  int elements;
+  struct type *fundamental_type;
 
-  if (is_hfa_or_hva (type))
+  if (aapcs_is_vfp_call_or_return_candidate (type, &elements,
+					     &fundamental_type))
     {
       /* v0-v7 are used to return values and one register is allocated
 	 for one member.  However, HFA or HVA has at most four members.  */
@@ -2126,14 +2036,31 @@ aarch64_store_return_value (struct type *type, struct regcache *regs,
 {
   struct gdbarch *gdbarch = regs->arch ();
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  int elements;
+  struct type *fundamental_type;
 
-  if (TYPE_CODE (type) == TYPE_CODE_FLT)
+  if (aapcs_is_vfp_call_or_return_candidate (type, &elements,
+					     &fundamental_type))
     {
-      bfd_byte buf[V_REGISTER_SIZE];
-      int len = TYPE_LENGTH (type);
+      int len = TYPE_LENGTH (fundamental_type);
+
+      for (int i = 0; i < elements; i++)
+	{
+	  int regno = AARCH64_V0_REGNUM + i;
+	  bfd_byte tmpbuf[V_REGISTER_SIZE];
+
+	  if (aarch64_debug)
+	    {
+	      debug_printf ("write HFA or HVA return value element %d to %s\n",
+			    i + 1,
+			    gdbarch_register_name (gdbarch, regno));
+	    }
 
-      memcpy (buf, valbuf, len > V_REGISTER_SIZE ? V_REGISTER_SIZE : len);
-      regs->cooked_write (AARCH64_V0_REGNUM, buf);
+	  memcpy (tmpbuf, valbuf,
+		  len > V_REGISTER_SIZE ? V_REGISTER_SIZE : len);
+	  regs->cooked_write (regno, tmpbuf);
+	  valbuf += len;
+	}
     }
   else if (TYPE_CODE (type) == TYPE_CODE_INT
 	   || TYPE_CODE (type) == TYPE_CODE_CHAR
@@ -2168,39 +2095,6 @@ aarch64_store_return_value (struct type *type, struct regcache *regs,
 	    }
 	}
     }
-  else if (is_hfa_or_hva (type))
-    {
-      int elements = TYPE_NFIELDS (type);
-      struct type *member_type = check_typedef (TYPE_FIELD_TYPE (type, 0));
-      int len = TYPE_LENGTH (member_type);
-      int i;
-
-      for (i = 0; i < elements; i++)
-	{
-	  int regno = AARCH64_V0_REGNUM + i;
-	  bfd_byte tmpbuf[V_REGISTER_SIZE];
-
-	  if (aarch64_debug)
-	    {
-	      debug_printf ("write HFA or HVA return value element %d to %s\n",
-			    i + 1,
-			    gdbarch_register_name (gdbarch, regno));
-	    }
-
-	  memcpy (tmpbuf, valbuf, len);
-	  regs->cooked_write (regno, tmpbuf);
-	  valbuf += len;
-	}
-    }
-  else if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type)
-	   && (TYPE_LENGTH (type) == 8 || TYPE_LENGTH (type) == 16))
-    {
-      /* Short vector.  */
-      gdb_byte buf[V_REGISTER_SIZE];
-
-      memcpy (buf, valbuf, TYPE_LENGTH (type));
-      regs->cooked_write (AARCH64_V0_REGNUM, buf);
-    }
   else
     {
       /* For a structure or union the behaviour is as if the value had
-- 
2.15.2 (Apple Git-101.1)

Re: [PATCH 3/4] Aarch64: Float register detection for return values.

[Simon Marchi]

On 2018-08-20 05:29, Alan Hayward wrote:
> Use aapcs_is_vfp_call_or_return_candidate to detect float register
> args.
> 
> Remove the no longer used is_hfa_or_hva().
> 
> 2018-08-20  Alan Hayward  <alan.hayward@arm.com>
> 
> 	PR gdb/22943:
> 	* aarch64-tdep.c (is_hfa_or_hva): Remove function.
> 	(aarch64_extract_return_value): Use
> 	aapcs_is_vfp_call_or_return_candidate.
> 	(aarch64_return_in_memory): Likewise.
> 	(aarch64_store_return_value): Likewise.

LGTM too, with the same disclaimer as the previous patches.

Simon

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

[Alan Hayward]

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

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.  */
-- 
2.15.2 (Apple Git-101.1)

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

[Simon Marchi]

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

[Ping][PATCH 0/4] Aarch64: Correctly support args passed in float registers.

[Alan Hayward]

Ping.


> On 20 Aug 2018, at 10:29, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
> GDB currently does not adhere to the AAPCS spec when passing structures that
> contain float arguments. It works for simple structures but does not correctly
> detect nested structures.
> This is shown by the failing tests in the recently added
> infcall-nested-structs.exp. See PR gdb/22943.
> 
> Patch 1 contains a new function for correctly detecting float args.
> I wrote this by taking the equivalent function in GCC and modifying it until
> it fit into GDB, then simplified and cleaned up.
> Patches 2 and 3 then use this function.
> 
> Patch 4 extends the infcall-nested-structs test to cover structures with 4
> and 5 fields - 4 fields is the maximum number of fields that can be passed
> via float args.
> 
> For the full spec see:
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042f/IHI0042F_aapcs.pdf
> 
> 
> Alan Hayward (4):
>  Aarch64: Add function to detect args passed in float registers
>  Aarch64: Float register detection for _push_dummy_call
>  Aarch64: Float register detection for return values.
>  infcall-nested-structs: Test up to five fields
> 
> gdb/aarch64-tdep.c                                | 444 ++++++++++++----------
> gdb/testsuite/gdb.base/infcall-nested-structs.c   | 168 +++++---
> gdb/testsuite/gdb.base/infcall-nested-structs.exp |   5 +-
> 3 files changed, 366 insertions(+), 251 deletions(-)
> 
> -- 
> 2.15.2 (Apple Git-101.1)
>