On 3/5/19 9:38 AM, Mark Cave-Ayland wrote:
> It's really that this is a stepping stone to patch 7 where you end up with
> this:
>
> static inline int vsrh_offset(int i)
> {
> return offsetof(CPUPPCState, vsr[i].VsrD(0));
> }
>
> static inline int vsrl_offset(int i)
> {
> return offsetof(CPUPPCState, vsr[i].VsrD(1));
> }
>
> ...
>
> static inline int avrh_offset(int i)
> {
> return vsrh_offset(i + 32);
> }
>
> static inline int avrl_offset(int i)
> {
> return vsrl_offset(i + 32);
> }
Yes, but the only users look like...
> tcg_gen_ld_i64(dst, cpu_env, high ? avrh_offset(regno) :
> avrl_offset(regno));
... this. And to me that suggests that one helper instead of two would be
better, so that you can write
tcg_gen_ld_i64(dst, cpu_env, avr64_offset(regno, high));
and propagate the "high" into the VsrD argument.
> which looks a bit easier on the eye? I'm less keen on pushing the "high" bool
> all the
> way down into offset functions as I find the separate vsrh/vsrl functions
> much easier
> to read in the helpers than the get_avr64() version.
Ah. Hmm.
Will we not in the end require a function A64's
vec_reg_offset(regno, element, size)
I know DavidH did when writing the s390x vector support last week. Perhaps
that's more palatable than avr64_offset that only supports 64-bit elements?
r~
