[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [Qemu-devel] [PATCH 4/8] target/ppc: introduce avrh_offse

From: Richard Henderson
Subject: Re: [Qemu-ppc] [Qemu-devel] [PATCH 4/8] target/ppc: introduce avrh_offset() and avrl_offset() functions
Date: Tue, 5 Mar 2019 13:24:29 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0

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?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]