Re: [PATCH 10/11] ELFv2 ABI: structure passing / return

[Joel Brobecker <brobecker at adacore dot com>]

> ChangeLog:
> 
> 	* ppc-sysv-tdep.c (ppc64_aggregate_candidate): New routine.
> 	(ppc64_elfv2_abi_homogeneous_aggregate): Likewise.
> 	(ppc64_sysv_abi_push_param): Handle ELFv2 homogeneous structs.
> 	(ppc64_sysv_abi_return_value): Likewise.  Also, handle small
> 	structures returned in GPRs.
> 
> Index: binutils-gdb/gdb/ppc-sysv-tdep.c
> ===================================================================
> --- binutils-gdb.orig/gdb/ppc-sysv-tdep.c
> +++ binutils-gdb/gdb/ppc-sysv-tdep.c
> @@ -1101,6 +1101,156 @@ convert_code_addr_to_desc_addr (CORE_ADD
>    return 1;
>  }
>  
> +/* Walk down the type tree of TYPE counting consecutive base elements.
> +   If *FIELD_TYPE is NULL, then set it to the first valid floating point
> +   or vector type.  If a non-floating point or vector type is found, or
> +   if a floating point or vector type that doesn't match a non-NULL
> +   *FIELD_TYPE is found, then return -1, otherwise return the count in the
> +   sub-tree.  */
> +
> +static LONGEST
> +ppc64_aggregate_candidate (struct type *type,
> +			   struct type **field_type)

I think we can reasonably change the return type to int, here?
That would avoid the cast you're making at:

> +      struct type *field_type = NULL;
> +      LONGEST field_count = ppc64_aggregate_candidate (type, &field_type);
[...]
> +	      if (n_elts)
> +		*n_elts = (int) field_count;

The other option is to make everything use LONGEST for this count.
That way, everything would be consistent and we'd be avoiding a cast
as well.

> +  if (tdep->elf_abi == POWERPC_ELF_V2
> +      && TYPE_LENGTH (valtype) <= 16
> +      && (TYPE_CODE (valtype) == TYPE_CODE_STRUCT
> +	  || TYPE_CODE (valtype) == TYPE_CODE_UNION
> +	  || (TYPE_CODE (valtype) == TYPE_CODE_ARRAY && !TYPE_VECTOR (valtype))))

The last line looks too long. Can you fold it?

> +    {
> +      int n_regs = (TYPE_LENGTH (valtype) + tdep->wordsize - 1) / tdep->wordsize;

Same here.

> +      int i;
> +
> +      for (i = 0; i < n_regs; i++)
> +	{
> +	  gdb_byte regval[MAX_REGISTER_SIZE];
> +	  int regnum = tdep->ppc_gp0_regnum + 3 + i;
> +	  int offset = i * tdep->wordsize;
> +	  int len = TYPE_LENGTH (valtype) - offset;
> +	  if (len > tdep->wordsize)
> +	    len = tdep->wordsize;

Missing empty line after local variable declarations.

-- 
Joel