On 2020/2/21 下午4:58, Paolo Bonzini wrote: > On 21/02/20 04:45, cheng...@emindsoft.com.cn wrote: >> static inline void fpush(CPUX86State *env) >> { >> - env->fpstt = (env->fpstt - 1) & 7; >> - env->fptags[env->fpstt] = 0; /* validate stack entry */ >> + set_fpstt(env, env->fpstt - 1, false, true); > > On overflow fpstt is ~0, so this does: > > env->foverflow = true; > env->fpstt = 7; > env->fptags[7] = 0; /* validate stack entry */ > > Is this correct? You are going to set ST0 so the register should not be > marked empty. >
Originally, I wanted to add foverflow to mark the stack overflow only, and kept another things no touch. But I think what you said above is correct, for me, if fpush/f[i]ld*_STO are overflow, the env->fpstt, env->fpregs and env->fptags should be kept no touch, and foverflow is set true, so there is no negative effect. Welcome your idea. >> void helper_fdecstp(CPUX86State *env) >> { >> - env->fpstt = (env->fpstt - 1) & 7; >> + set_fpstt(env, env->fpstt - 1, false, false); > > This is clearing env->foverflow. But after 8 consecutive fdecstp or > fincstp the result of FXAM should not change. > >> env->fpus &= ~0x4700; >> } >> >> void helper_fincstp(CPUX86State *env) >> { >> - env->fpstt = (env->fpstt + 1) & 7; >> + set_fpstt(env, env->fpstt + 1, true, false); > > Same here. > OK. thanks. Now if foverflow is only for fpush/f[i]ld*_ST0, I guess fincstp/fdecstp can clear foverflow. The env->fptags are only for fpop, which keep no touch in fincstp/fdecstp. > The actual bug is hinted in helper_fxam_ST0: > > /* XXX: test fptags too */ > > I think the correct fix should be something like > > diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c > index 99f28f267f..792a128a6d 100644 > --- a/target/i386/fpu_helper.c > +++ b/target/i386/fpu_helper.c > @@ -991,7 +991,11 @@ void helper_fxam_ST0(CPUX86State *env) > env->fpus |= 0x200; /* C1 <-- 1 */ > } > > - /* XXX: test fptags too */ > + if (env->fptags[env->fpstt]) { > + env->fpus |= 0x4100; /* Empty */ > + return; > + } > + For fpop overflow, this fix is enough, but for me, we still need foverflow to check fpush/fld*_ST0 overflow. Don't you think we need check fpush/f[i]ld*_ST0 overflow? Thanks