On Thu, May 29, 2014 at 02:48:28PM +0100, Peter Maydell wrote: > On 29 May 2014 14:35, Petar Jovanovic <petar.jovano...@imgtec.com> 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. -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
