Generally this looks like a fairly straightforward port replacement of the original code with something essentially equivalent which is fine. One issue is the new virtual finishTranslation function. Since translation will happen very frequently (at least once a fetch), it's best to avoid a virtual function call and the slight performance overhead. It's small, but it adds up if you multiply it by several billion instructions. Steve can probably tell you the right way to weave that into the templating set up of the CPUs (it confuses me some times), so I'll leave that to him to comment on. Please let me know if there were any other interface/functional changes that I'm missing. I saw the setFault and isPrefetch functions, but those are going to be inlined anyway they're nothing to worry about.
Since changes to the CPUs can affect all ISAs in potentially subtle, obscure ways, please run all of the regressions once before submitting if you haven't already. Please also give Steve a chance to look this over if possible. Gabe Timothy M. Jones wrote: > # HG changeset patch > # User Timothy M. Jones <[email protected]> > # Date 1259064162 0 > # Node ID 77dffd16710c4ad8e308b7dfee75cce4219b30a0 > # Parent b431ed08a6b7d809df9bfed51365e4d3bdf81f93 > Cpu: Make TimingSimpleCPU use new DTB translation code. > > This patch removes the duplicate code from cpu/simple/timing.hh and makes > TimingSimpleCPU use the new code in cpu/translation.hh. I've had to make > a few additional changes to get it to work ok for both O3 and Timing. > > diff --git a/src/cpu/base.hh b/src/cpu/base.hh > --- a/src/cpu/base.hh > +++ b/src/cpu/base.hh > @@ -53,6 +53,7 @@ > class ThreadContext; > class System; > class Port; > +class WholeTranslationState; > > namespace TheISA > { > @@ -276,6 +277,12 @@ > > virtual Counter totalInstructions() const { return 0; } > > + /** > + * Finish a DTB translation. > + * @param state The DTB translation state. > + */ > + virtual void finishTranslation(WholeTranslationState *state) {} > + > // Function tracing > private: > bool functionTracingEnabled; > diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh > --- a/src/cpu/base_dyn_inst.hh > +++ b/src/cpu/base_dyn_inst.hh > @@ -979,9 +979,9 @@ > BaseTLB::Mode mode) > { > WholeTranslationState *state = > - new WholeTranslationState(req, res, mode); > - DataTranslation<Impl> *trans = > - new DataTranslation<Impl>(this, state); > + new WholeTranslationState(req, NULL, res, mode); > + DataTranslationInst<Impl> *trans = > + new DataTranslationInst<Impl>(this, state); > cpu->dtb->translateTiming(req, thread->getTC(), trans, mode); > } > > @@ -993,11 +993,11 @@ > { > // Set up the translation state. > WholeTranslationState *state = > - new WholeTranslationState(req, sreqLow, sreqHigh, res, mode); > - DataTranslation<Impl> *stransLow = > - new DataTranslation<Impl>(this, state, 0); > - DataTranslation<Impl> *stransHigh = > - new DataTranslation<Impl>(this, state, 1); > + new WholeTranslationState(req, sreqLow, sreqHigh, NULL, res, mode); > + DataTranslationInst<Impl> *stransLow = > + new DataTranslationInst<Impl>(this, state, 0); > + DataTranslationInst<Impl> *stransHigh = > + new DataTranslationInst<Impl>(this, state, 1); > > // Perform the translation. > cpu->dtb->translateTiming(sreqLow, thread->getTC(), stransLow, mode); > diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc > --- a/src/cpu/simple/timing.cc > +++ b/src/cpu/simple/timing.cc > @@ -268,19 +268,9 @@ > } > > void > -TimingSimpleCPU::sendData(Fault fault, RequestPtr req, > - uint8_t *data, uint64_t *res, bool read) > +TimingSimpleCPU::sendData(RequestPtr req, uint8_t *data, uint64_t *res, > + bool read) > { > - _status = Running; > - if (fault != NoFault) { > - if (req->isPrefetch()) > - fault = NoFault; > - delete data; > - delete req; > - > - translationFault(fault); > - return; > - } > PacketPtr pkt; > buildPacket(pkt, req, read); > pkt->dataDynamic<uint8_t>(data); > @@ -311,25 +301,9 @@ > } > > void > -TimingSimpleCPU::sendSplitData(Fault fault1, Fault fault2, > - RequestPtr req1, RequestPtr req2, RequestPtr req, > - uint8_t *data, bool read) > +TimingSimpleCPU::sendSplitData(RequestPtr req1, RequestPtr req2, > + RequestPtr req, uint8_t *data, bool read) > { > - _status = Running; > - if (fault1 != NoFault || fault2 != NoFault) { > - if (req1->isPrefetch()) > - fault1 = NoFault; > - if (req2->isPrefetch()) > - fault2 = NoFault; > - delete data; > - delete req1; > - delete req2; > - if (fault1 != NoFault) > - translationFault(fault1); > - else if (fault2 != NoFault) > - translationFault(fault2); > - return; > - } > PacketPtr pkt1, pkt2; > buildSplitPacket(pkt1, pkt2, req1, req2, req, data, read); > if (req->getFlags().isSet(Request::NO_ACCESS)) { > @@ -450,6 +424,7 @@ > const Addr pc = thread->readPC(); > unsigned block_size = dcachePort.peerBlockSize(); > int data_size = sizeof(T); > + BaseTLB::Mode mode = BaseTLB::Read; > > RequestPtr req = new Request(asid, addr, data_size, > flags, pc, _cpuId, tid); > @@ -457,24 +432,27 @@ > Addr split_addr = roundDown(addr + data_size - 1, block_size); > assert(split_addr <= addr || split_addr - addr < block_size); > > - > _status = DTBWaitResponse; > if (split_addr > addr) { > RequestPtr req1, req2; > assert(!req->isLLSC() && !req->isSwap()); > req->splitOnVaddr(split_addr, req1, req2); > > - typedef SplitDataTranslation::WholeTranslationState WholeState; > - WholeState *state = new WholeState(req1, req2, req, > - (uint8_t *)(new T), > BaseTLB::Read); > - thread->dtb->translateTiming(req1, tc, > - new SplitDataTranslation(this, 0, state), BaseTLB::Read); > - thread->dtb->translateTiming(req2, tc, > - new SplitDataTranslation(this, 1, state), BaseTLB::Read); > + WholeTranslationState *state = > + new WholeTranslationState(req, req1, req2, (uint8_t *)(new T), > + NULL, mode); > + DataTranslationCpu *trans1 = > + new DataTranslationCpu(this, state, 0); > + DataTranslationCpu *trans2 = > + new DataTranslationCpu(this, state, 1); > + > + thread->dtb->translateTiming(req1, tc, trans1, mode); > + thread->dtb->translateTiming(req2, tc, trans2, mode); > } else { > - DataTranslation *translation = > - new DataTranslation(this, (uint8_t *)(new T), NULL, > BaseTLB::Read); > - thread->dtb->translateTiming(req, tc, translation, BaseTLB::Read); > + WholeTranslationState *state = > + new WholeTranslationState(req, (uint8_t *)(new T), NULL, mode); > + DataTranslationCpu *translation = new DataTranslationCpu(this, > state); > + thread->dtb->translateTiming(req, tc, translation, mode); > } > > if (traceData) { > @@ -568,6 +546,7 @@ > const Addr pc = thread->readPC(); > unsigned block_size = dcachePort.peerBlockSize(); > int data_size = sizeof(T); > + BaseTLB::Mode mode = BaseTLB::Write; > > RequestPtr req = new Request(asid, addr, data_size, > flags, pc, _cpuId, tid); > @@ -583,17 +562,21 @@ > assert(!req->isLLSC() && !req->isSwap()); > req->splitOnVaddr(split_addr, req1, req2); > > - typedef SplitDataTranslation::WholeTranslationState WholeState; > - WholeState *state = new WholeState(req1, req2, req, > - (uint8_t *)dataP, BaseTLB::Write); > - thread->dtb->translateTiming(req1, tc, > - new SplitDataTranslation(this, 0, state), BaseTLB::Write); > - thread->dtb->translateTiming(req2, tc, > - new SplitDataTranslation(this, 1, state), BaseTLB::Write); > + WholeTranslationState *state = > + new WholeTranslationState(req, req1, req2, (uint8_t *)dataP, > + res, mode); > + DataTranslationCpu *trans1 = > + new DataTranslationCpu(this, state, 0); > + DataTranslationCpu *trans2 = > + new DataTranslationCpu(this, state, 1); > + > + thread->dtb->translateTiming(req1, tc, trans1, mode); > + thread->dtb->translateTiming(req2, tc, trans2, mode); > } else { > - DataTranslation *translation = > - new DataTranslation(this, (uint8_t *)dataP, res, BaseTLB::Write); > - thread->dtb->translateTiming(req, tc, translation, BaseTLB::Write); > + WholeTranslationState *state = > + new WholeTranslationState(req, (uint8_t *)dataP, res, mode); > + DataTranslationCpu *translation = new DataTranslationCpu(this, > state); > + thread->dtb->translateTiming(req, tc, translation, mode); > } > > if (traceData) { > @@ -668,6 +651,32 @@ > > > void > +TimingSimpleCPU::finishTranslation(WholeTranslationState *state) > +{ > + _status = Running; > + > + if (state->getFault() != NoFault) { > + if (state->isPrefetch()) { > + state->setNoFault(); > + } > + delete state->data; > + state->deleteReqs(); > + translationFault(state->getFault()); > + } else { > + if (!state->isSplit) { > + sendData(state->mainReq, state->data, state->res, > + state->mode == BaseTLB::Read); > + } else { > + sendSplitData(state->sreqLow, state->sreqHigh, state->mainReq, > + state->data, state->mode == BaseTLB::Read); > + } > + } > + > + delete state; > +} > + > + > +void > TimingSimpleCPU::fetch() > { > DPRINTF(SimpleCPU, "Fetch\n"); > diff --git a/src/cpu/simple/timing.hh b/src/cpu/simple/timing.hh > --- a/src/cpu/simple/timing.hh > +++ b/src/cpu/simple/timing.hh > @@ -32,6 +32,7 @@ > #define __CPU_SIMPLE_TIMING_HH__ > > #include "cpu/simple/base.hh" > +#include "cpu/translation.hh" > > #include "params/TimingSimpleCPU.hh" > > @@ -115,95 +116,9 @@ > }; > FetchTranslation fetchTranslation; > > - class DataTranslation : public BaseTLB::Translation > - { > - protected: > - TimingSimpleCPU *cpu; > - uint8_t *data; > - uint64_t *res; > - BaseTLB::Mode mode; > - > - public: > - DataTranslation(TimingSimpleCPU *_cpu, > - uint8_t *_data, uint64_t *_res, BaseTLB::Mode _mode) > - : cpu(_cpu), data(_data), res(_res), mode(_mode) > - { > - assert(mode == BaseTLB::Read || mode == BaseTLB::Write); > - } > - > - void > - finish(Fault fault, RequestPtr req, ThreadContext *tc, > - BaseTLB::Mode mode) > - { > - assert(mode == this->mode); > - cpu->sendData(fault, req, data, res, mode == BaseTLB::Read); > - delete this; > - } > - }; > - > - class SplitDataTranslation : public BaseTLB::Translation > - { > - public: > - struct WholeTranslationState > - { > - public: > - int outstanding; > - RequestPtr requests[2]; > - RequestPtr mainReq; > - Fault faults[2]; > - uint8_t *data; > - BaseTLB::Mode mode; > - > - WholeTranslationState(RequestPtr req1, RequestPtr req2, > - RequestPtr main, uint8_t *data, BaseTLB::Mode mode) > - { > - assert(mode == BaseTLB::Read || mode == BaseTLB::Write); > - > - outstanding = 2; > - requests[0] = req1; > - requests[1] = req2; > - mainReq = main; > - faults[0] = faults[1] = NoFault; > - this->data = data; > - this->mode = mode; > - } > - }; > - > - TimingSimpleCPU *cpu; > - int index; > - WholeTranslationState *state; > - > - SplitDataTranslation(TimingSimpleCPU *_cpu, int _index, > - WholeTranslationState *_state) > - : cpu(_cpu), index(_index), state(_state) > - {} > - > - void > - finish(Fault fault, RequestPtr req, ThreadContext *tc, > - BaseTLB::Mode mode) > - { > - assert(state); > - assert(state->outstanding); > - state->faults[index] = fault; > - if (--state->outstanding == 0) { > - cpu->sendSplitData(state->faults[0], > - state->faults[1], > - state->requests[0], > - state->requests[1], > - state->mainReq, > - state->data, > - state->mode == BaseTLB::Read); > - delete state; > - } > - delete this; > - } > - }; > - > - void sendData(Fault fault, RequestPtr req, > - uint8_t *data, uint64_t *res, bool read); > - void sendSplitData(Fault fault1, Fault fault2, > - RequestPtr req1, RequestPtr req2, RequestPtr req, > - uint8_t *data, bool read); > + void sendData(RequestPtr req, uint8_t *data, uint64_t *res, bool read); > + void sendSplitData(RequestPtr req1, RequestPtr req2, RequestPtr req, > + uint8_t *data, bool read); > > void translationFault(Fault fault); > > @@ -351,6 +266,12 @@ > */ > void printAddr(Addr a); > > + /** > + * Finish a DTB translation. > + * @param state The DTB translation state. > + */ > + virtual void finishTranslation(WholeTranslationState *state); > + > private: > > typedef EventWrapper<TimingSimpleCPU, &TimingSimpleCPU::fetch> > FetchEvent; > diff --git a/src/cpu/translation.hh b/src/cpu/translation.hh > --- a/src/cpu/translation.hh > +++ b/src/cpu/translation.hh > @@ -34,6 +34,7 @@ > #define __CPU_TRANSLATION_HH__ > > #include "sim/tlb.hh" > +#include "cpu/base.hh" > > // Forward declaration > template <class Impl> > @@ -50,13 +51,15 @@ > RequestPtr mainReq; > RequestPtr sreqLow; > RequestPtr sreqHigh; > + uint8_t *data; > uint64_t *res; > BaseTLB::Mode mode; > > /** Single translation state. */ > - WholeTranslationState(RequestPtr _req, uint64_t *_res, BaseTLB::Mode > _mode) > + WholeTranslationState(RequestPtr _req, uint8_t *_data, uint64_t *_res, > + BaseTLB::Mode _mode) > : outstanding(1), isSplit(false), mainReq(_req), sreqLow(NULL), > - sreqHigh(NULL), res(_res), mode(_mode) > + sreqHigh(NULL), data(_data), res(_res), mode(_mode) > { > faults[0] = faults[1] = NoFault; > assert(mode == BaseTLB::Read || mode == BaseTLB::Write); > @@ -64,10 +67,10 @@ > > /** Split translation state. */ > WholeTranslationState(RequestPtr _req, RequestPtr _sreqLow, > - RequestPtr _sreqHigh, uint64_t *_res, > + RequestPtr _sreqHigh, uint8_t *_data, uint64_t > *_res, > BaseTLB::Mode _mode) > : outstanding(2), isSplit(true), mainReq(_req), sreqLow(_sreqLow), > - sreqHigh(_sreqHigh), res(_res), mode(_mode) > + sreqHigh(_sreqHigh), data(_data), res(_res), mode(_mode) > { > faults[0] = faults[1] = NoFault; > assert(mode == BaseTLB::Read || mode == BaseTLB::Write); > @@ -104,12 +107,24 @@ > return NoFault; > } > > + void > + setNoFault() > + { > + faults[0] = faults[1] = NoFault; > + } > + > bool > isUncacheable() const > { > return mainReq->isUncacheable(); > } > > + bool > + isPrefetch() const > + { > + return mainReq->isPrefetch(); > + } > + > Addr > getPaddr() const > { > @@ -134,7 +149,7 @@ > }; > > template <class Impl> > -class DataTranslation : public BaseTLB::Translation > +class DataTranslationInst : public BaseTLB::Translation > { > protected: > typedef BaseDynInst<Impl> DynInst; > @@ -145,13 +160,13 @@ > int index; > > public: > - DataTranslation(DynInstPtr _inst, WholeTranslationState *_state) > + DataTranslationInst(DynInstPtr _inst, WholeTranslationState *_state) > : inst(_inst), state(_state), index(0) > { > } > > - DataTranslation(DynInstPtr _inst, WholeTranslationState *_state, > - int _index) > + DataTranslationInst(DynInstPtr _inst, WholeTranslationState *_state, > + int _index) > : inst(_inst), state(_state), index(_index) > { > } > @@ -163,10 +178,44 @@ > assert(state); > assert(mode == state->mode); > if (state->finish(fault, index)) { > + assert(inst); > inst->finishTranslation(state); > } > delete this; > } > }; > > +class DataTranslationCpu : public BaseTLB::Translation > +{ > + protected: > + BaseCPU *cpu; > + WholeTranslationState *state; > + int index; > + > + public: > + DataTranslationCpu(BaseCPU *_cpu, WholeTranslationState *_state) > + : cpu(_cpu), state(_state), index(0) > + { > + } > + > + DataTranslationCpu(BaseCPU *_cpu, WholeTranslationState *_state, > + int _index) > + : cpu(_cpu), state(_state), index(_index) > + { > + } > + > + virtual void > + finish(Fault fault, RequestPtr req, ThreadContext *tc, > + BaseTLB::Mode mode) > + { > + assert(state); > + assert(mode == state->mode); > + if (state->finish(fault, index)) { > + assert(cpu); > + cpu->finishTranslation(state); > + } > + delete this; > + } > +}; > + > #endif // __CPU_TRANSLATION_HH__ > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
