> On 2011-01-17 00:28:39, Gabe Black wrote: > > This change seems to have some dead functionality in it. The "delay" member > > added to translations is never used, the "delay()" pure virtual function is > > never used and not defined for any other ISAs (which I think will break > > them all), and the translationDelayed variable is never used. Unnecessary > > copyright changes should also be rolled back when removing that dead code. > > > > Are there any instructions that actually expect to get a valid fault when > > performing initiateAcc? I could imagine there might be since it used to be > > something you could always do for the most part, but getting rid of those > > instances would help simplify this code I think. The code that prepares a > > request for the memory system could follow from the finishTranslation > > function no matter if it happened immediately or after a table walk. Then > > an instruction would (hopefully) only require one pass since it's work > > would be done and it would either be ready to commit (a store) or ready for > > completeAcc (a load). The actual load/store queue would have to wait since > > the address might not be ready, but that might be a pretty simple extension > > on top of waiting for initiateAcc to happen.
I pointed out where delay() is used and it's not defined in BaseTLB, it's in Translation. All the regressions pass just fine. If you search for translationDelayed in the diff you'll see it is used too. Yes, Alpha still returns a fault on initiateAcc() since there is no table walk, the tlb lookup is known immediately. This is quite a substantial patch, so I would prefer to commit it close to as is and then work on patch on top of it to rework finishTranslation if it's possible. > On 2011-01-17 00:28:39, Gabe Black wrote: > > src/sim/tlb.hh, line 83 > > <http://reviews.m5sim.org/r/422/diff/1/?file=9524#file9524line83> > > > > Did you compile an ISAs other than ARM? I don't see this being defined > > for any of them, and since it's pure virtual I don't think they'll work any > > more. They will.. It's not the BaseTLB, it's translation which the timing cpus inherit, not the isas. - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/422/#review741 ----------------------------------------------------------- On 2011-01-12 09:12:18, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/422/ > ----------------------------------------------------------- > > (Updated 2011-01-12 09:12:18) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > O3: Enhance data address translation by supporting hardware page table > walkers. > > Some ISAs (like ARM) relies on hardware page table walkers. For those ISAs, > when a TLB miss occurs, initiateTranslation() can return with NoFault but with > the translation unfinished. > > Instructions experiencing a delayed translation due to a hardware page table > walk are deferred until the translation completes and kept into the IQ. In > order to keep track of them, the IQ has been augmented with a queue of the > outstanding delayed memory instructions. When their translation completes, > instructions are re-executed (only their initiateAccess() was already > executed; their DTB translation is now skipped). The IEW stage has been > modified to support such a 2-pass execution. > > > Diffs > ----- > > src/arch/arm/tlb.cc 5d0f62927d75 > src/cpu/base_dyn_inst.hh 5d0f62927d75 > src/cpu/base_dyn_inst_impl.hh 5d0f62927d75 > src/cpu/o3/fetch.hh 5d0f62927d75 > src/cpu/o3/iew_impl.hh 5d0f62927d75 > src/cpu/o3/inst_queue.hh 5d0f62927d75 > src/cpu/o3/inst_queue_impl.hh 5d0f62927d75 > src/cpu/o3/lsq_unit_impl.hh 5d0f62927d75 > src/cpu/simple/timing.hh 5d0f62927d75 > src/cpu/translation.hh 5d0f62927d75 > src/sim/tlb.hh 5d0f62927d75 > > Diff: http://reviews.m5sim.org/r/422/diff > > > Testing > ------- > > > Thanks, > > Ali > >
_______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
