On 10/12/2018 18:43, Richard Henderson wrote: > On 12/7/18 2:56 AM, Mark Cave-Ayland wrote: >> - gen_helper_f##op(cpu_fpr[rD(ctx->opcode)], cpu_env, >> \ >> - cpu_fpr[rA(ctx->opcode)], >> \ >> - cpu_fpr[rC(ctx->opcode)], cpu_fpr[rB(ctx->opcode)]); >> \ >> + get_fpr(t0, rA(ctx->opcode)); >> \ >> + get_fpr(t1, rC(ctx->opcode)); >> \ >> + get_fpr(t2, rB(ctx->opcode)); >> \ >> + gen_helper_f##op(t3, cpu_env, t0, t1, t2); >> \ >> + set_fpr(rD(ctx->opcode), t3); >> \ >> if (isfloat) { >> \ >> - gen_helper_frsp(cpu_fpr[rD(ctx->opcode)], cpu_env, >> \ >> - cpu_fpr[rD(ctx->opcode)]); >> \ >> + get_fpr(t0, rD(ctx->opcode)); >> \ >> + gen_helper_frsp(t3, cpu_env, t0); >> \ >> + set_fpr(rD(ctx->opcode), t3); >> \ >> } >> \ > > This is an accurate conversion, but the writeback to the rD register need not > happen until after helper_frsp. Just move it below the isfloat block.
Okay - I'll fix this up in the next iteration. > I do see that helper_frsp can raise an exception for invalid_op for SNaN. If > that code were actually reachable, this would have been an existing bug, in > that we should not have written back to rD after the first operation. > However, > any SNaN will already have been eliminated by the first operation (via > squashing to QNaN or by exiting via exception). > > Similarly in GEN_FLOAT_AB. Noted. >> + get_fpr(t0, rB(ctx->opcode)); >> + gen_helper_frsqrte(t1, cpu_env, t0); >> + set_fpr(rD(ctx->opcode), t1); >> + gen_helper_frsp(t1, cpu_env, t1); >> + gen_compute_fprf_float64(t1); > > gen_frsqrtes has the set_fpr in the wrong place. Likewise gen_fsqrts. Ooops. I remember having a think a bit harder about these two functions, so I'll go and take another look. ATB, Mark.