Re: [RFA] Altivec ABI patches

[Elena Zannoni <ezannoni at redhat dot com>]

Kevin Buettner writes:
 > 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.
 > 

It does seem odd, I actually just cut and pasted the code from a few
lines above. I thought there was some reason for it, after all.

	    {
	      *(int *) &registers[REGISTER_BYTE (greg)] = 0;
	      memcpy (&registers[REGISTER_BYTE (greg)], val_buf, 4);
	      greg++;
	    }

Should this go as well, then? Unless it always writes 4 bytes, but the
size of the general register can be bigger?


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

Ok.


 > Thanks,
 > 
 > Kevin

Elena