Re: [RFA] Altivec ABI patches

Elena Zannoni
2002-04-27T01:45:12Z

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