Re: [RFA] Altivec ABI patches

[Kevin Buettner <kevinb at redhat dot com>]

On Apr 26,  4:44pm, Elena Zannoni wrote:

> As promised, this patch adds the last missing bits of Altivec support.
> It handles the new vector types for passing parameters and returning
> values.
> 
> Elena
> 
> 2002-04-26  Elena Zannoni  <ezannoni@redhat.com>
> 
> 	* rs6000-tdep.c (rs6000_extract_return_value,
> 	rs6000_store_return_value): Handle returning vectors.
> 	(rs6000_gdbarch_init): Use
> 	ppc_sysv_abi_broken_use_struct_convention for native sysv cases.
> 	* ppc-linux-tdep.c (ppc_sysv_abi_broken_use_struct_convention):
> 	New function.
> 	(ppc_sysv_abi_use_struct_convention): Deal with functions returning
> 	vectors.
> 	(ppc_sysv_abi_push_arguments): Handle vector parameters.
> 	* ppc-tdep.h (ppc_sysv_abi_broken_use_struct_convention): Export.

These changes look reasonable except for the following change to
ppc_sysv_abi_push_arguments():

> +	      if (vreg <= 13)
> +		{
> +		  *(int *) &registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
> +						     + vreg)] = 0;
> +		  memcpy (&registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
> +						    + vreg)],
> +			  v_val_buf, 16);
> +		  vreg++;
> +		}

Specifically, I'm having trouble understanding the point of this
assignment:

> +		  *(int *) &registers[REGISTER_BYTE (tdep->ppc_vr0_regnum
> +						     + vreg)] = 0;

Note that the subsequent memcpy() ends up overwriting the memory
zero'd by the assignment.  If the assignment does serve some useful
purpose, I'd prefer to see memset() used instead.

Your patch is approved so long as you address this point.

Thanks,

Kevin