Review Board 2.0.15


inorder: add timing translation

Review Request #1089 - Created March 7, 2012 and discarded

Information
Korey Sewell
gem5
default
Reviewers
Default 
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.
This passes the alpha-hello world regression
- Latest update fixed memory leak bug
- It also fixes bug that did not correctly handle memory requests that were blocked (the doTLBAccess was always setting the request as completed, however this is not always the case).

Latest fixes deadlocks on some alpha, inorder, long regressions (twolf,bzip2,eon). Debug in progress...

Issue Summary

36 35 0 1
Description From Last Updated Status
should you still get the fault if it's a prefetch? Ali Saidi March 8, 2012, 1:14 a.m. Open
what about doing the access? Ali Saidi March 8, 2012, 1:14 a.m. Open
probably should delete it then Ali Saidi March 8, 2012, 1:14 a.m. Open
Can inst be of type InOrderDynInst? Is WholeTranslationState only used on CPUs that use InOrderDynInst? Erik Tomusk March 19, 2012, 3:08 a.m. Open
need an #include "cpu/inorder/resources/inorder_translation.hh" Erik Tomusk March 21, 2012, 1:57 a.m. Open
need to pass inst to InOrderTranslationState Erik Tomusk March 21, 2012, 1:57 a.m. Open
should this be: DataTranslation<CacheUnit*> *trans = new DataTranslation<CacheUnit*>(this, inst->state, tid); ? Erik Tomusk March 21, 2012, 1:57 a.m. Open
also need tid and size: ThreadID tid = inst->readTid(); int size = cache_req->dataPkt->getSize(); Erik Tomusk March 21, 2012, 1:57 a.m. Open
should this be: memcpy(cache_req->reqData, cache_req->dataPkt->getPtr<uint8_t>(), size); ? Erik Tomusk March 21, 2012, 1:57 a.m. Open
cache_req doesn't have a write_des. What should this do? Erik Tomusk March 21, 2012, 1:57 a.m. Open
Missing ( Erik Tomusk March 21, 2012, 1:57 a.m. Open
The two bools should be public or have accessor functions Erik Tomusk March 21, 2012, 1:57 a.m. Open
Also need a DynInstPtr and a CacheReqPtr and corresponding includes. If including CacheUnit, it needs a forward declaration. Erik Tomusk March 21, 2012, 1:57 a.m. Open
Constructors need to take a DynInstPtr Erik Tomusk March 21, 2012, 1:57 a.m. Open
also need methods something like: inline bool isRead() { return (mode == BaseTLB::Read); } inline bool isWrite() { return (mode ... Erik Tomusk March 21, 2012, 1:57 a.m. Open
One of the methods here should be virtual to allow for dynamic_cast with InOrderTranslationState Erik Tomusk March 21, 2012, 1:57 a.m. Open
This should be included where it's needed, not here. I'm assuming it isn't needed here because it wasn't here before ... Gabe Black April 3, 2012, 2:40 p.m. Open
This file isn't actually different. Please remove it from the change. Gabe Black April 3, 2012, 2:40 p.m. Open
Unnecessary white space change. Gabe Black April 3, 2012, 2:40 p.m. Open
Why is this part of the comment indented more than the earlier parts? Gabe Black April 3, 2012, 2:40 p.m. Open
Don't add a bunch of commented out code. Gabe Black April 3, 2012, 2:40 p.m. Open
There probably should be, but Korey is the expert and would know for sure. Gabe Black April 3, 2012, 2:40 p.m. Open
I'm pretty sure it's not. Gabe Black April 3, 2012, 2:40 p.m. Open
Which hack is that? Gabe Black April 3, 2012, 2:40 p.m. Open
Wrong indentation. Gabe Black April 3, 2012, 2:40 p.m. Open
Wrong indentation. Gabe Black April 3, 2012, 2:40 p.m. Open
Bracket goes on previous line Gabe Black April 3, 2012, 2:40 p.m. Open
Bracket on previous line. Fix below as well. Gabe Black April 3, 2012, 2:40 p.m. Open
Fixing these random bits of trailing whitespace is good, but it should be its own change since it doesn't have ... Gabe Black April 3, 2012, 2:40 p.m. Open
If CacheUnitEvent isn't used and shouldn't be called, get rid of it. Ideally in a separate change preceding this one. Gabe Black April 3, 2012, 2:40 p.m. Open
Bad indentation. Gabe Black April 3, 2012, 2:40 p.m. Open
Bad indentation. Gabe Black April 3, 2012, 2:40 p.m. Open
Don't comment out code, delete it. Gabe Black April 3, 2012, 2:40 p.m. Open
The indentation here is wrong, unless review board is confused somehow. Please fix up style problems. Gabe Black April 3, 2012, 2:40 p.m. Open
Where does data come from? Erik Tomusk April 3, 2012, 10:59 p.m. Open
Review request changed
Updated (June 25, 2015, 1:08 a.m.)

Status: Discarded