> On 2011-06-11 09:56:39, Steve Reinhardt wrote: > > Shouldn't you get rid of the cache_unit.cc changes now? I thought that was > > the point. > > > > This is still a hack, in my opinion; note that the comment on the _pc field > > in mem/request.hh says "for tracing/debugging", i.e., it's not intended to > > be architectural. Also, it isn't always set (e.g., for device accesses), > > though for CPU accesses it should be. So I'd say (1) it should work and > > (2) it's a much less ugly hack than what you had before, so assuming you do > > get rid of the cache_unit.cc changes I'd say it's fine. > > > > I still think having a ProxyThreadContext wrapped around a DynInst is the > > "right" way to do it, but I can see where that also looks like a lot of > > mostly unnecessary overhead.
The hacky cache_unit.cc code will definitely be cleaned up in the final change. I was thinking that although the ProxyThreadContext<DynInst> seems like the right way to do this for DTB accesses, I think it may not work well for ITB accesses as it could be possible that the DynInst has not even been created yet. Typically, one might fetch the cache block and then create the DynInst (done in simple and o3) rather than vice versa (done in inorder). I'm thinking the right thing to do is to make the _pc field in Request more than a debugging feature, and then add a "isSet" flag there along with an assertion in "getPC()" to make sure that any reader of the PC will not get a uninitialized value. That way, you have one generalized way to access the TLB whether it be ITB or DTB access. - Korey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/743/#review1323 ----------------------------------------------------------- On 2011-06-10 22:52:04, Korey Sewell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/743/ > ----------------------------------------------------------- > > (Updated 2011-06-10 22:52:04) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > inorder/dtb: make sure DTB translate correct address > The DTB expects the correct PC in the ThreadContext > but how if the memory accesses are speculative? Shouldn't > we send along the requestor's PC to the translate functions? > > > Diffs > ----- > > src/arch/alpha/tlb.cc 77d12d8f7971 > src/cpu/inorder/resources/cache_unit.cc 77d12d8f7971 > > Diff: http://reviews.m5sim.org/r/743/diff > > > Testing > ------- > > > Thanks, > > Korey > > _______________________________________________ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
