| Message ID | 20190303172343.13406-9-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | target/ppc: switch fpr/vsrl registers so all VSX registers are in host endian order | expand |
On 3/3/19 9:23 AM, Mark Cave-Ayland wrote: > static inline void get_cpu_vsrh(TCGv_i64 dst, int n) > { > - if (n < 32) { > - get_fpr(dst, n); > - } else { > - get_avr64(dst, n - 32, true); > - } > + tcg_gen_ld_i64(dst, cpu_env, vsrh_offset(n)); > } > > static inline void get_cpu_vsrl(TCGv_i64 dst, int n) > { > - if (n < 32) { > - get_vsrl(dst, n); > - } else { > - get_avr64(dst, n - 32, false); > - } > + tcg_gen_ld_i64(dst, cpu_env, vsrl_offset(n)); > } > > static inline void set_cpu_vsrh(int n, TCGv_i64 src) > { > - if (n < 32) { > - set_fpr(n, src); > - } else { > - set_avr64(n - 32, src, true); > - } > + tcg_gen_st_i64(src, cpu_env, vsrh_offset(n)); > } I think these ought to have a "high" parameter, like set/get_avr64. r~
On 03/03/2019 23:35, Richard Henderson wrote: > On 3/3/19 9:23 AM, Mark Cave-Ayland wrote: >> static inline void get_cpu_vsrh(TCGv_i64 dst, int n) >> { >> - if (n < 32) { >> - get_fpr(dst, n); >> - } else { >> - get_avr64(dst, n - 32, true); >> - } >> + tcg_gen_ld_i64(dst, cpu_env, vsrh_offset(n)); >> } >> >> static inline void get_cpu_vsrl(TCGv_i64 dst, int n) >> { >> - if (n < 32) { >> - get_vsrl(dst, n); >> - } else { >> - get_avr64(dst, n - 32, false); >> - } >> + tcg_gen_ld_i64(dst, cpu_env, vsrl_offset(n)); >> } >> >> static inline void set_cpu_vsrh(int n, TCGv_i64 src) >> { >> - if (n < 32) { >> - set_fpr(n, src); >> - } else { >> - set_avr64(n - 32, src, true); >> - } >> + tcg_gen_st_i64(src, cpu_env, vsrh_offset(n)); >> } > > I think these ought to have a "high" parameter, like set/get_avr64. Right, this is effectively the same discussion as in my previous email so I suggest we follow this up there. ATB, Mark.
On 05/03/2019 18:16, Mark Cave-Ayland wrote: > On 03/03/2019 23:35, Richard Henderson wrote: > >> On 3/3/19 9:23 AM, Mark Cave-Ayland wrote: >>> static inline void get_cpu_vsrh(TCGv_i64 dst, int n) >>> { >>> - if (n < 32) { >>> - get_fpr(dst, n); >>> - } else { >>> - get_avr64(dst, n - 32, true); >>> - } >>> + tcg_gen_ld_i64(dst, cpu_env, vsrh_offset(n)); >>> } >>> >>> static inline void get_cpu_vsrl(TCGv_i64 dst, int n) >>> { >>> - if (n < 32) { >>> - get_vsrl(dst, n); >>> - } else { >>> - get_avr64(dst, n - 32, false); >>> - } >>> + tcg_gen_ld_i64(dst, cpu_env, vsrl_offset(n)); >>> } >>> >>> static inline void set_cpu_vsrh(int n, TCGv_i64 src) >>> { >>> - if (n < 32) { >>> - set_fpr(n, src); >>> - } else { >>> - set_avr64(n - 32, src, true); >>> - } >>> + tcg_gen_st_i64(src, cpu_env, vsrh_offset(n)); >>> } >> >> I think these ought to have a "high" parameter, like set/get_avr64. > > Right, this is effectively the same discussion as in my previous email so I suggest > we follow this up there. I've reworked this patchset over the evening to keep avr64_offset() and looking over the result it's more readable than I thought, mostly thanks to its use of the VsrD macro. The only part I'm now not sure about is whether from the above you want to keep fpr_offset() and vsrh_offset(), or whether in the final patch in the series I can introduce vsr64_offset() similar to avr64_offset() and switch the callers over to use it? ATB, Mark.
On 3/6/19 1:48 PM, Mark Cave-Ayland wrote: > The only part I'm now not sure about is whether from the above you want to keep > fpr_offset() and vsrh_offset(), or whether in the final patch in the series I can > introduce vsr64_offset() similar to avr64_offset() and switch the callers over to use it? I would not keep vsrh_offset and would introduce vsr64_offset. But I would keep fpr_offset, specifically for fpr stuff. If you like, you can have it forward to vsr64_offset(r, high=true). r~
diff --git a/target/ppc/translate/vsx-impl.inc.c b/target/ppc/translate/vsx-impl.inc.c index 7d02a235e7..43e97756a2 100644 --- a/target/ppc/translate/vsx-impl.inc.c +++ b/target/ppc/translate/vsx-impl.inc.c @@ -1,49 +1,23 @@ /*** VSX extension ***/ -static inline void get_vsrl(TCGv_i64 dst, int n) -{ - tcg_gen_ld_i64(dst, cpu_env, vsrl_offset(n)); -} - -static inline void set_vsrl(int n, TCGv_i64 src) -{ - tcg_gen_st_i64(src, cpu_env, vsrl_offset(n)); -} - static inline void get_cpu_vsrh(TCGv_i64 dst, int n) { - if (n < 32) { - get_fpr(dst, n); - } else { - get_avr64(dst, n - 32, true); - } + tcg_gen_ld_i64(dst, cpu_env, vsrh_offset(n)); } static inline void get_cpu_vsrl(TCGv_i64 dst, int n) { - if (n < 32) { - get_vsrl(dst, n); - } else { - get_avr64(dst, n - 32, false); - } + tcg_gen_ld_i64(dst, cpu_env, vsrl_offset(n)); } static inline void set_cpu_vsrh(int n, TCGv_i64 src) { - if (n < 32) { - set_fpr(n, src); - } else { - set_avr64(n - 32, src, true); - } + tcg_gen_st_i64(src, cpu_env, vsrh_offset(n)); } static inline void set_cpu_vsrl(int n, TCGv_i64 src) { - if (n < 32) { - set_vsrl(n, src); - } else { - set_avr64(n - 32, src, false); - } + tcg_gen_st_i64(src, cpu_env, vsrl_offset(n)); } #define VSX_LOAD_SCALAR(name, operation) \
Now that the VSX registers are all in host endian order, there is no need to go via different accessors depending upon the register number. Instead the high and low parts can be accessed directly via vsrh_offset() and vsrl_offset() accordingly. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- target/ppc/translate/vsx-impl.inc.c | 34 ++++------------------------------ 1 file changed, 4 insertions(+), 30 deletions(-)