From 9779ba2e37a753df407b976fc4b299d936ea62b8 Mon Sep 17 00:00:00 2001 From: Andreas Hansson Date: Tue, 2 Dec 2014 06:07:36 -0500 Subject: [PATCH] mem: Add const getters for write packet data This patch takes a first step in tightening up how we use the data pointer in write packets. A const getter is added for the pointer itself (getConstPtr), and a number of member functions are also made const accordingly. In a range of places throughout the memory system the new member is used. The patch also removes the unused isReadWrite function. --- src/cpu/inorder/resources/cache_unit.cc | 12 +++++----- src/cpu/inorder/resources/fetch_unit.cc | 2 +- src/cpu/minor/execute.cc | 2 +- src/cpu/minor/lsq.cc | 2 +- src/cpu/o3/fetch_impl.hh | 2 +- src/cpu/simple/atomic.cc | 2 +- src/cpu/testers/memtest/memtest.cc | 1 + src/cpu/testers/rubytest/Check.cc | 2 +- src/mem/abstract_mem.cc | 9 ++++---- src/mem/cache/cache.hh | 2 +- src/mem/cache/cache_impl.hh | 12 +++++----- src/mem/external_slave.cc | 2 +- src/mem/packet.cc | 4 ++-- src/mem/packet.hh | 22 ++++++++++++------- src/mem/packet_access.hh | 2 +- src/mem/ruby/common/DataBlock.cc | 2 +- src/mem/ruby/common/DataBlock.hh | 2 +- src/mem/ruby/slicc_interface/RubyRequest.cc | 2 +- .../ruby/slicc_interface/RubySlicc_Util.hh | 2 +- src/mem/ruby/system/Sequencer.cc | 4 ++-- 20 files changed, 49 insertions(+), 41 deletions(-) diff --git a/src/cpu/inorder/resources/cache_unit.cc b/src/cpu/inorder/resources/cache_unit.cc index 251369e01..3a44986e2 100644 --- a/src/cpu/inorder/resources/cache_unit.cc +++ b/src/cpu/inorder/resources/cache_unit.cc @@ -68,7 +68,7 @@ using namespace ThePipeline; #if TRACING_ON static std::string -printMemData(uint8_t *data, unsigned size) +printMemData(const uint8_t *data, unsigned size) { std::stringstream dataStr; for (unsigned pos = 0; pos < size; pos++) { @@ -855,7 +855,7 @@ CacheUnit::doCacheAccess(DynInstPtr inst, uint64_t *write_res, DPRINTF(InOrderCachePort, "[tid:%u]: [sn:%i]: Storing data: %s\n", tid, inst->seqNum, - printMemData(cache_req->dataPkt->getPtr(), + printMemData(cache_req->dataPkt->getConstPtr(), cache_req->dataPkt->getSize())); if (mem_req->isCondSwap()) { @@ -1061,9 +1061,9 @@ CacheUnit::processCacheCompletion(PacketPtr pkt) DPRINTF(InOrderCachePort, "[tid:%u]: [sn:%i]: Bytes loaded were: %s\n", tid, inst->seqNum, - (split_pkt) ? printMemData(split_pkt->getPtr(), + (split_pkt) ? printMemData(split_pkt->getConstPtr(), split_pkt->getSize()) : - printMemData(cache_pkt->getPtr(), + printMemData(cache_pkt->getConstPtr(), cache_pkt->getSize())); } else if(inst->isStore()) { assert(cache_pkt->isWrite()); @@ -1071,9 +1071,9 @@ CacheUnit::processCacheCompletion(PacketPtr pkt) DPRINTF(InOrderCachePort, "[tid:%u]: [sn:%i]: Bytes stored were: %s\n", tid, inst->seqNum, - (split_pkt) ? printMemData(split_pkt->getPtr(), + (split_pkt) ? printMemData(split_pkt->getConstPtr(), split_pkt->getSize()) : - printMemData(cache_pkt->getPtr(), + printMemData(cache_pkt->getConstPtr(), cache_pkt->getSize())); } diff --git a/src/cpu/inorder/resources/fetch_unit.cc b/src/cpu/inorder/resources/fetch_unit.cc index 6892688b2..13864e589 100644 --- a/src/cpu/inorder/resources/fetch_unit.cc +++ b/src/cpu/inorder/resources/fetch_unit.cc @@ -503,7 +503,7 @@ FetchUnit::processCacheCompletion(PacketPtr pkt) // Copy Data to pendingFetch queue... (*pend_it)->block = new uint8_t[cacheBlkSize]; - memcpy((*pend_it)->block, cache_pkt->getPtr(), cacheBlkSize); + memcpy((*pend_it)->block, cache_pkt->getConstPtr(), cacheBlkSize); (*pend_it)->valid = true; cache_req->setMemAccPending(false); diff --git a/src/cpu/minor/execute.cc b/src/cpu/minor/execute.cc index 123128358..69cb9a239 100644 --- a/src/cpu/minor/execute.cc +++ b/src/cpu/minor/execute.cc @@ -355,7 +355,7 @@ Execute::handleMemResponse(MinorDynInstPtr inst, if (is_load && packet->getSize() > 0) { DPRINTF(MinorMem, "Memory data[0]: 0x%x\n", - static_cast(packet->getPtr()[0])); + static_cast(packet->getConstPtr()[0])); } /* Complete the memory access instruction */ diff --git a/src/cpu/minor/lsq.cc b/src/cpu/minor/lsq.cc index cae0d3666..fca580085 100644 --- a/src/cpu/minor/lsq.cc +++ b/src/cpu/minor/lsq.cc @@ -560,7 +560,7 @@ LSQ::SplitDataRequest::retireResponse(PacketPtr response) * by the response fragment */ std::memcpy( data + (response->req->getVaddr() - request.getVaddr()), - response->getPtr(), + response->getConstPtr(), response->req->getSize()); } } diff --git a/src/cpu/o3/fetch_impl.hh b/src/cpu/o3/fetch_impl.hh index 1c9799e41..47a64a9bf 100644 --- a/src/cpu/o3/fetch_impl.hh +++ b/src/cpu/o3/fetch_impl.hh @@ -388,7 +388,7 @@ DefaultFetch::processCacheCompletion(PacketPtr pkt) return; } - memcpy(fetchBuffer[tid], pkt->getPtr(), fetchBufferSize); + memcpy(fetchBuffer[tid], pkt->getConstPtr(), fetchBufferSize); fetchBufferValid[tid] = true; // Wake up the CPU (if it went to sleep and was waiting on diff --git a/src/cpu/simple/atomic.cc b/src/cpu/simple/atomic.cc index 06969f3e3..8dcae01c5 100644 --- a/src/cpu/simple/atomic.cc +++ b/src/cpu/simple/atomic.cc @@ -469,7 +469,7 @@ AtomicSimpleCPU::writeMem(uint8_t *data, unsigned size, if (req->isSwap()) { assert(res); - memcpy(res, pkt.getPtr(), fullSize); + memcpy(res, pkt.getConstPtr(), fullSize); } } diff --git a/src/cpu/testers/memtest/memtest.cc b/src/cpu/testers/memtest/memtest.cc index d949178c2..082737f8a 100644 --- a/src/cpu/testers/memtest/memtest.cc +++ b/src/cpu/testers/memtest/memtest.cc @@ -173,6 +173,7 @@ MemTest::completeRequest(PacketPtr pkt) safe_cast(pkt->senderState); uint8_t *data = state->data; + // @todo: This should really be a const pointer uint8_t *pkt_data = pkt->getPtr(); //Remove the address from the list of outstanding diff --git a/src/cpu/testers/rubytest/Check.cc b/src/cpu/testers/rubytest/Check.cc index 9de766077..19d0623c0 100644 --- a/src/cpu/testers/rubytest/Check.cc +++ b/src/cpu/testers/rubytest/Check.cc @@ -197,7 +197,7 @@ Check::initiateAction() pkt->dataDynamic(writeData); DPRINTF(RubyTest, "data 0x%x check 0x%x\n", - *(pkt->getPtr()), *writeData); + *(pkt->getConstPtr()), *writeData); // push the subblock onto the sender state. The sequencer will // update the subblock on the return diff --git a/src/mem/abstract_mem.cc b/src/mem/abstract_mem.cc index c819ce2fc..dca0403fb 100644 --- a/src/mem/abstract_mem.cc +++ b/src/mem/abstract_mem.cc @@ -309,7 +309,7 @@ AbstractMemory::checkLockedAddrList(PacketPtr pkt) A, system()->getMasterName(pkt->req->masterId()), \ pkt->getSize(), pkt->getAddr(), \ pkt->req->isUncacheable() ? 'U' : 'C'); \ - DDUMP(MemoryAccess, pkt->getPtr(), pkt->getSize()); \ + DDUMP(MemoryAccess, pkt->getConstPtr(), pkt->getSize()); \ } \ } while (0) @@ -344,7 +344,8 @@ AbstractMemory::access(PacketPtr pkt) bool overwrite_mem = true; // keep a copy of our possible write value, and copy what is at the // memory address into the packet - std::memcpy(&overwrite_val[0], pkt->getPtr(), pkt->getSize()); + std::memcpy(&overwrite_val[0], pkt->getConstPtr(), + pkt->getSize()); std::memcpy(pkt->getPtr(), hostAddr, pkt->getSize()); if (pkt->req->isCondSwap()) { @@ -381,7 +382,7 @@ AbstractMemory::access(PacketPtr pkt) } else if (pkt->isWrite()) { if (writeOK(pkt)) { if (pmemAddr) { - memcpy(hostAddr, pkt->getPtr(), pkt->getSize()); + memcpy(hostAddr, pkt->getConstPtr(), pkt->getSize()); DPRINTF(MemoryAccess, "%s wrote %x bytes to address %x\n", __func__, pkt->getSize(), pkt->getAddr()); } @@ -416,7 +417,7 @@ AbstractMemory::functionalAccess(PacketPtr pkt) pkt->makeResponse(); } else if (pkt->isWrite()) { if (pmemAddr) - memcpy(hostAddr, pkt->getPtr(), pkt->getSize()); + memcpy(hostAddr, pkt->getConstPtr(), pkt->getSize()); TRACE_PACKET("Write"); pkt->makeResponse(); } else if (pkt->isPrint()) { diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh index b9a9a7823..e0bd29752 100644 --- a/src/mem/cache/cache.hh +++ b/src/mem/cache/cache.hh @@ -287,7 +287,7 @@ class Cache : public BaseCache bool pending_downgrade = false); bool satisfyMSHR(MSHR *mshr, PacketPtr pkt, BlkType *blk); - void doTimingSupplyResponse(PacketPtr req_pkt, uint8_t *blk_data, + void doTimingSupplyResponse(PacketPtr req_pkt, const uint8_t *blk_data, bool already_copied, bool pending_inval); /** diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh index 66abf6eff..f4099c0ef 100644 --- a/src/mem/cache/cache_impl.hh +++ b/src/mem/cache/cache_impl.hh @@ -357,7 +357,7 @@ Cache::access(PacketPtr pkt, BlkType *&blk, blk->status &= ~BlkWritable; ++fastWrites; } - std::memcpy(blk->data, pkt->getPtr(), blkSize); + std::memcpy(blk->data, pkt->getConstPtr(), blkSize); DPRINTF(Cache, "%s new state is %s\n", __func__, blk->print()); incHitCount(pkt); return true; @@ -1211,7 +1211,7 @@ Cache::recvTimingResp(PacketPtr pkt) completion_time = clockEdge(responseLatency) + pkt->lastWordDelay; if (pkt->isRead() && !is_error) { - target->pkt->setData(pkt->getPtr()); + target->pkt->setData(pkt->getConstPtr()); } } target->pkt->makeTimingResponse(); @@ -1535,7 +1535,7 @@ Cache::handleFill(PacketPtr pkt, BlkType *blk, // if we got new data, copy it in if (pkt->isRead()) { - std::memcpy(blk->data, pkt->getPtr(), blkSize); + std::memcpy(blk->data, pkt->getConstPtr(), blkSize); } blk->whenReady = clockEdge() + responseLatency * clockPeriod() + @@ -1554,7 +1554,7 @@ Cache::handleFill(PacketPtr pkt, BlkType *blk, template void Cache:: -doTimingSupplyResponse(PacketPtr req_pkt, uint8_t *blk_data, +doTimingSupplyResponse(PacketPtr req_pkt, const uint8_t *blk_data, bool already_copied, bool pending_inval) { // sanity check @@ -1810,7 +1810,7 @@ Cache::recvTimingSnoopReq(PacketPtr pkt) // the packet's invalidate flag is set... assert(pkt->isInvalidate()); } - doTimingSupplyResponse(pkt, wb_pkt->getPtr(), + doTimingSupplyResponse(pkt, wb_pkt->getConstPtr(), false, false); if (pkt->isInvalidate()) { @@ -2020,7 +2020,7 @@ Cache::getTimingPacket() pkt = new Packet(tgt_pkt); pkt->allocate(); if (pkt->isWrite()) { - pkt->setData(tgt_pkt->getPtr()); + pkt->setData(tgt_pkt->getConstPtr()); } } } diff --git a/src/mem/external_slave.cc b/src/mem/external_slave.cc index c2ec8e2e4..67800b9a2 100644 --- a/src/mem/external_slave.cc +++ b/src/mem/external_slave.cc @@ -108,7 +108,7 @@ StubSlavePort::recvAtomic(PacketPtr packet) DPRINTF(ExternalPort, "StubSlavePort: recvAtomic a: 0x%x size: %d" " data: ...\n", packet->getAddr(), size); - DDUMP(ExternalPort, packet->getPtr(), size); + DDUMP(ExternalPort, packet->getConstPtr(), size); } return 0; diff --git a/src/mem/packet.cc b/src/mem/packet.cc index 8bbd7ff18..9dd67746b 100644 --- a/src/mem/packet.cc +++ b/src/mem/packet.cc @@ -303,11 +303,11 @@ Packet::checkFunctional(Printable *obj, Addr addr, bool is_secure, int size, } } else if (isWrite()) { if (offset >= 0) { - memcpy(data + offset, getPtr(), + memcpy(data + offset, getConstPtr(), (min(func_end, val_end) - func_start) + 1); } else { // val_start > func_start - memcpy(data, getPtr() - offset, + memcpy(data, getConstPtr() - offset, (min(func_end, val_end) - val_start) + 1); } } else { diff --git a/src/mem/packet.hh b/src/mem/packet.hh index 8d84a7ccb..fea9dbaae 100644 --- a/src/mem/packet.hh +++ b/src/mem/packet.hh @@ -186,7 +186,6 @@ class MemCmd bool needsResponse() const { return testCmdAttrib(NeedsResponse); } bool isInvalidate() const { return testCmdAttrib(IsInvalidate); } bool hasData() const { return testCmdAttrib(HasData); } - bool isReadWrite() const { return isRead() && isWrite(); } bool isLLSC() const { return testCmdAttrib(IsLlsc); } bool isSWPrefetch() const { return testCmdAttrib(IsSWPrefetch); } bool isHWPrefetch() const { return testCmdAttrib(IsHWPrefetch); } @@ -501,7 +500,6 @@ class Packet : public Printable bool needsResponse() const { return cmd.needsResponse(); } bool isInvalidate() const { return cmd.isInvalidate(); } bool hasData() const { return cmd.hasData(); } - bool isReadWrite() const { return cmd.isReadWrite(); } bool isLLSC() const { return cmd.isLLSC(); } bool isError() const { return cmd.isError(); } bool isPrint() const { return cmd.isPrint(); } @@ -852,11 +850,19 @@ class Packet : public Printable return (T*)data; } + template + const T* + getConstPtr() const + { + assert(flags.isSet(STATIC_DATA|DYNAMIC_DATA)); + return (const T*)data; + } + /** * return the value of what is pointed to in the packet. */ template - T get(); + T get() const; /** * set the value in the data pointer to v. @@ -868,7 +874,7 @@ class Packet : public Printable * Copy data into the packet from the provided pointer. */ void - setData(uint8_t *p) + setData(const uint8_t *p) { if (p != getPtr()) std::memcpy(getPtr(), p, getSize()); @@ -879,7 +885,7 @@ class Packet : public Printable * which is aligned to the given block size. */ void - setDataFromBlock(uint8_t *blk_data, int blkSize) + setDataFromBlock(const uint8_t *blk_data, int blkSize) { setData(blk_data + getOffset(blkSize)); } @@ -889,16 +895,16 @@ class Packet : public Printable * is aligned to the given block size. */ void - writeData(uint8_t *p) + writeData(uint8_t *p) const { - std::memcpy(p, getPtr(), getSize()); + std::memcpy(p, getConstPtr(), getSize()); } /** * Copy data from the packet to the memory at the provided pointer. */ void - writeDataToBlock(uint8_t *blk_data, int blkSize) + writeDataToBlock(uint8_t *blk_data, int blkSize) const { writeData(blk_data + getOffset(blkSize)); } diff --git a/src/mem/packet_access.hh b/src/mem/packet_access.hh index fca9606fc..9e6f1cbb1 100644 --- a/src/mem/packet_access.hh +++ b/src/mem/packet_access.hh @@ -45,7 +45,7 @@ /** return the value of what is pointed to in the packet. */ template inline T -Packet::get() +Packet::get() const { assert(flags.isSet(STATIC_DATA|DYNAMIC_DATA)); assert(sizeof(T) <= size); diff --git a/src/mem/ruby/common/DataBlock.cc b/src/mem/ruby/common/DataBlock.cc index c71449dd0..2a292444a 100644 --- a/src/mem/ruby/common/DataBlock.cc +++ b/src/mem/ruby/common/DataBlock.cc @@ -78,7 +78,7 @@ DataBlock::getData(int offset, int len) const } void -DataBlock::setData(uint8_t *data, int offset, int len) +DataBlock::setData(const uint8_t *data, int offset, int len) { assert(offset + len <= RubySystem::getBlockSizeBytes()); memcpy(&m_data[offset], data, len); diff --git a/src/mem/ruby/common/DataBlock.hh b/src/mem/ruby/common/DataBlock.hh index 56320523b..ac08fac82 100644 --- a/src/mem/ruby/common/DataBlock.hh +++ b/src/mem/ruby/common/DataBlock.hh @@ -59,7 +59,7 @@ class DataBlock uint8_t getByte(int whichByte) const; const uint8_t *getData(int offset, int len) const; void setByte(int whichByte, uint8_t data); - void setData(uint8_t *data, int offset, int len); + void setData(const uint8_t *data, int offset, int len); void copyPartial(const DataBlock & dblk, int offset, int len); bool equal(const DataBlock& obj) const; void print(std::ostream& out) const; diff --git a/src/mem/ruby/slicc_interface/RubyRequest.cc b/src/mem/ruby/slicc_interface/RubyRequest.cc index ff90e415e..e2f275006 100644 --- a/src/mem/ruby/slicc_interface/RubyRequest.cc +++ b/src/mem/ruby/slicc_interface/RubyRequest.cc @@ -72,7 +72,7 @@ RubyRequest::functionalWrite(Packet *pkt) Addr mBase = m_PhysicalAddress.getAddress(); Addr mTail = mBase + m_Size; - uint8_t * pktData = pkt->getPtr(); + const uint8_t * pktData = pkt->getConstPtr(); Addr cBase = std::max(wBase, mBase); Addr cTail = std::min(wTail, mTail); diff --git a/src/mem/ruby/slicc_interface/RubySlicc_Util.hh b/src/mem/ruby/slicc_interface/RubySlicc_Util.hh index 8e2a1c5b1..dd9a1f2a4 100644 --- a/src/mem/ruby/slicc_interface/RubySlicc_Util.hh +++ b/src/mem/ruby/slicc_interface/RubySlicc_Util.hh @@ -135,7 +135,7 @@ testAndWrite(Address addr, DataBlock& blk, Packet *pkt) lineAddr.makeLineAddress(); if (pktLineAddr == lineAddr) { - uint8_t *data = pkt->getPtr(); + const uint8_t *data = pkt->getConstPtr(); unsigned int size_in_bytes = pkt->getSize(); unsigned startByte = pkt->getAddr() - lineAddr.getAddress(); diff --git a/src/mem/ruby/system/Sequencer.cc b/src/mem/ruby/system/Sequencer.cc index 281ea22be..ef1b9676b 100644 --- a/src/mem/ruby/system/Sequencer.cc +++ b/src/mem/ruby/system/Sequencer.cc @@ -526,7 +526,7 @@ Sequencer::hitCallback(SequencerRequest* srequest, DataBlock& data, // update the data unless it is a non-data-carrying flush if (g_system_ptr->m_warmup_enabled) { - data.setData(pkt->getPtr(), + data.setData(pkt->getConstPtr(), request_address.getOffset(), pkt->getSize()); } else if (!pkt->isFlush()) { if ((type == RubyRequestType_LD) || @@ -538,7 +538,7 @@ Sequencer::hitCallback(SequencerRequest* srequest, DataBlock& data, data.getData(request_address.getOffset(), pkt->getSize()), pkt->getSize()); } else { - data.setData(pkt->getPtr(), + data.setData(pkt->getConstPtr(), request_address.getOffset(), pkt->getSize()); } }