Re: [Qemu-devel] [PATCH v2 01/15] target/ppc: remove getVSR()/putVSR() from fpu_helper.c

[Mark Cave-Ayland]

On 12/06/2019 20:45, Richard Henderson wrote:

> On 6/2/19 4:08 AM, Mark Cave-Ayland wrote:
>>  void helper_xvxsigsp(CPUPPCState *env, uint32_t opcode)
>>  {
>> -    ppc_vsr_t xt, xb;
>> +    ppc_vsr_t *xt = &env->vsr[xT(opcode)];
>> +    ppc_vsr_t *xb = &env->vsr[xB(opcode)];
>> +    ppc_vsr_t t = *xt;
>>      uint32_t exp, i, fraction;
>>  
>> -    getVSR(xB(opcode), &xb, env);
>> -    memset(&xt, 0, sizeof(xt));
> 
> Change in behaviour -- zero init to copy init.
> 
> Note for future cleanup: most of these initializations do not need to happen,
> because we overwrite all elements of T without consuming the previous value.
> 
> 
>> @@ -3410,23 +3382,22 @@ void helper_xsrqpi(CPUPPCState *env, uint32_t opcode)
>>          env->fp_status.float_exception_flags &= ~float_flag_inexact;
>>      }
>>  
>> -    helper_compute_fprf_float128(env, xt.f128);
>> +    helper_compute_fprf_float128(env, t.f128);
>> +    *xt = t;
>>      do_float_check_status(env, GETPC());
>> -    putVSR(rD(opcode) + 32, &xt, env);
> 
> Change in behaviour -- writeback happens before do_float_check_status instead
> of after.  This may well be a bug fix, but if so should happen separately.

I've now fixed both of these. FWIW this is probably going to be my last bit of 
work
on VSX for a while since as I don't have 64-bit reference hardware, and writing 
and
testing these patchsets takes a long time. At least I feel that things are now 
in a
place where people can start to take more advantage of the vector ops should 
they wish.


ATB,

Mark.