Re: [PATCH 01/20] target/sparc: Introduce cpu_put_psr_icc

[Richard Henderson]

On 10/19/23 07:11, Philippe Mathieu-Daudé wrote:
> >       uint32_t psr;
> >       __get_user(psr, &regs->psr);
> > -    env->psr = (psr & PSR_ICC) | (env->psr & ~PSR_ICC);
>
> This keeps the non-PSR_ICC fields from env->psr, ...
>
> > +    cpu_put_psr_icc(env, psr);
> >   #endif
> >       /* Note that pc and npc are handled in the caller. */
> > diff --git a/target/sparc/win_helper.c b/target/sparc/win_helper.c
> > index 3a7c0ff943..bf2c90c780 100644
> > --- a/target/sparc/win_helper.c
> > +++ b/target/sparc/win_helper.c
> > @@ -67,9 +67,14 @@ target_ulong cpu_get_psr(CPUSPARCState *env)
> >   #endif
> >   }
> > -void cpu_put_psr_raw(CPUSPARCState *env, target_ulong val)
> > +void cpu_put_psr_icc(CPUSPARCState *env, target_ulong val)
> >   {
> >       env->psr = val & PSR_ICC;
>
> ... while this zeroes the non-PSR_ICC fields. Is that expected?

The only field in env->psr is ICC.
The other fields are in env->psr{s,ps,et,pil,ef}.

This is a bit of old linux-user confusion, which apparently presumed more.
Anyway, the situation is improved with this patch, and further in the next by removing 
"env->psr" entirely.


r~


PS: The handling of PSR could probably be cleaned up.  I don't think there is a real need 
for the fields to be split apart like this.  A test of e.g. (env->psr & PSR_S) should be 
more or less identical in performance to (env->psrs != 0).  It's only the PSR_ICC 
subfields that get heavy use within translation.