----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1089/#review2343 -----------------------------------------------------------
I just about follow what all is happening in this patch. I've made some suggestions to get it to compile. src/cpu/inorder/inorder_dyn_inst.hh <http://reviews.gem5.org/r/1089/#comment2768> need an #include "cpu/inorder/resources/inorder_translation.hh" src/cpu/inorder/resources/cache_unit.cc <http://reviews.gem5.org/r/1089/#comment2769> need to pass inst to InOrderTranslationState src/cpu/inorder/resources/cache_unit.cc <http://reviews.gem5.org/r/1089/#comment2770> should this be: DataTranslation<CacheUnit*> *trans = new DataTranslation<CacheUnit*>(this, inst->state, tid); ? src/cpu/inorder/resources/cache_unit.cc <http://reviews.gem5.org/r/1089/#comment2771> also need tid and size: ThreadID tid = inst->readTid(); int size = cache_req->dataPkt->getSize(); src/cpu/inorder/resources/cache_unit.cc <http://reviews.gem5.org/r/1089/#comment2772> should this be: memcpy(cache_req->reqData, cache_req->dataPkt->getPtr<uint8_t>(), size); ? src/cpu/inorder/resources/cache_unit.cc <http://reviews.gem5.org/r/1089/#comment2773> cache_req doesn't have a write_des. What should this do? src/cpu/inorder/resources/graduation_unit.cc <http://reviews.gem5.org/r/1089/#comment2774> Missing ( src/cpu/inorder/resources/inorder_translation.hh <http://reviews.gem5.org/r/1089/#comment2775> The two bools should be public or have accessor functions src/cpu/inorder/resources/inorder_translation.hh <http://reviews.gem5.org/r/1089/#comment2776> Also need a DynInstPtr and a CacheReqPtr and corresponding includes. If including CacheUnit, it needs a forward declaration. src/cpu/inorder/resources/inorder_translation.hh <http://reviews.gem5.org/r/1089/#comment2777> Constructors need to take a DynInstPtr src/cpu/inorder/resources/inorder_translation.hh <http://reviews.gem5.org/r/1089/#comment2778> also need methods something like: inline bool isRead() { return (mode == BaseTLB::Read); } inline bool isWrite() { return (mode == BaseTLB::Write); } src/cpu/translation.hh <http://reviews.gem5.org/r/1089/#comment2767> One of the methods here should be virtual to allow for dynamic_cast with InOrderTranslationState - Erik Tomusk On March 21, 2012, 12:42 a.m., Korey Sewell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1089/ > ----------------------------------------------------------- > > (Updated March 21, 2012, 12:42 a.m.) > > > Review request for Default. > > > Description > ------- > > inorder: add timing translation > This is Erik Tomusk's patch to add timing translation to InOrder. It's the > next step > in getting InOrder to work for ARM. > > > Diffs > ----- > > src/cpu/inorder/cpu.hh 6df06e5975c6 > src/cpu/inorder/cpu.cc 6df06e5975c6 > src/cpu/inorder/inorder_dyn_inst.hh 6df06e5975c6 > src/cpu/inorder/resources/cache_unit.hh 6df06e5975c6 > src/cpu/inorder/resources/cache_unit.cc 6df06e5975c6 > src/cpu/inorder/resources/graduation_unit.cc 6df06e5975c6 > src/cpu/inorder/resources/inorder_translation.hh PRE-CREATION > src/cpu/translation.hh 6df06e5975c6 > > Diff: http://reviews.gem5.org/r/1089/diff/ > > > Testing > ------- > > This is not fully tested yet but a work in progress. > > > Thanks, > > Korey Sewell > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
