On Tue, Dec 17, 2013 at 10:54 AM, Peter Maydell <peter.mayd...@linaro.org> 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".) >
A levelled approach to debugging can help here too rather than having all-or-none with the QoM casts. Regards, Peter > thanks > -- PMM >
