On Tue, Dec 17, 2013 at 12:54:19AM +0000, Peter Maydell wrote: > On 17 December 2013 00:34, Edgar E. Iglesias <edgar.igles...@gmail.com> wrote: > > On Mon, Dec 16, 2013 at 01:11:47PM +0100, Andreas Färber wrote: > >> Why are you adding this field here rather than in CPUState alongside the > >> other field? This being a pointer I can't imagine problems for > >> non-softmmu, and I had posted patches a while ago to move the > >> surrounding fields there. Having it in CPUState would avoid some of the > >> env_ptr accesses above, which were supposed to be an interim solution only. > > > This was discussed when I posted the RFC. My first try had this member in > > CPUState but some of the hot paths for mmio accesses needed to do > > ENV_GET_CPU() to get hold of the CS and AS. ENV_GET_CPU does a runtime type > > check that would slow things down. That's the reason I moved it to env. > > > > I'm not against moving the field back if it doesnt cost too much. Maybe we > > should consider removing the CPU() around ENV_GET_CPU()? > > > > See: > > http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02889.html > > I think that's the wrong way round. We should put the field in the right > place in the code, which is CPUState. To the extent that that means we > discover that our dynamic cast macros are not fast enough for hot > paths we should make the actual error checking be debug builds > only. (There's been pushback on dynamic-casts in fastpaths before > and the preferred approach so far has been "optimise the cast". > I think we should take "...and do it only in debug builds" over "do > the wrong thing because a purely-debug-purposes type check > isn't as fast as we'd like".)
Sounds reasonable to me, I'll move the AS back into CPUState for v2. Thanks, Edgar
