Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

[Petar Jovanovic]

> I have just posted a patch copying CP0_Config1 into DisasContext, and
> moving existing accesses to this variable accordingly. This can be used
> as an example how to do it.

Done in v4. Thanks.

Regards,
Petar
________________________________________
From: Aurelien Jarno address@hidden
Sent: Thursday, May 29, 2014 4:26 PM
To: Peter Maydell
Cc: Petar Jovanovic; address@hidden; Petar Jovanovic; address@hidden; 
address@hidden
Subject: Re: [Qemu-devel] [v3 PATCH] target-mips: implement UserLocal Register

On Thu, May 29, 2014 at 04:06:56PM +0200, Aurelien Jarno wrote:
> On Thu, May 29, 2014 at 02:48:28PM +0100, Peter Maydell wrote:
> > On 29 May 2014 14:35, Petar Jovanovic <address@hidden> wrote:
> > >> change as CP0_Config3 is a read-only register. If I am right, probably
> > >> the best is to check directly env->CP0_Config3.
> > >
> > > If you take a look at v1 of the patch, that's what was done.
> > > In the code review, this was marked as unacceptable because it
> > > required [1] passing env within the translator.
> > >
> > >
> > > [1] http://patchwork.ozlabs.org/patch/349709/
> >
> > The intention of that review comment is to say that the
> > bulk of the translator code should not have access directly
> > to env. This is because most of the fields of env are
> > not safe to use as a basis for code generation decisions.
> > Having direct acess to an env pointer makes it very easy
> > to accidentally use a field that's not safe to use.
> > Instead we prefer to set up a DisasContext which has only
> > the required information in it, and pass that around.
> > The DisasContext is initialised in
> > gen_intermediate_code_internal from the TB flags and in
> > some cases from env-> fields where this is safe (for
> > instancec ctx.insn_flags is a copy of env->insn_flags).
> > This makes it easy to see at a glance what parts of env are
> > being relied on by the code generation and when something
> > new is added it's easy to see and check in code review.
> >
> > In this case we might choose to copy CP0_Config3 (or
> > just the relevant flag from it) from env into ctx;
> > I have no particular opinion there.
>
> It looks like one bit of CP0_Config3 is RW when microMIPS is
> implemented, so it might be better to copy only the corresponding bit.

I have just posted a patch copying CP0_Config1 into DisasContext, and
moving existing accesses to this variable accordingly. This can be used
as an example how to do it.

--
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net