Hello Nikos Nikoleris, Javier Bueno Hedo, Daniel Carvalho,
I'd like you to do a code review. Please visit
https://gem5-review.googlesource.com/c/public/gem5/+/17069
to review the following change.
Change subject: Revert "mem-cache: Added extra information to PrefetchInfo"
......................................................................
Revert "mem-cache: Added extra information to PrefetchInfo"
This reverts commit b0d1643ddf849d5e9b68a2e6535fa1ef561eac92.
Reason for revert: This commit broke building against NULL isa:
[ CXX] NULL/python/_m5/param_AMPMPrefetcher.cc -> .o
In file included from build/NULL/mem/cache/prefetch/queued.hh:49,
from
build/NULL/mem/cache/prefetch/access_map_pattern_matching.hh:45,
from build/NULL/python/_m5/param_AMPMPrefetcher.cc:9:
build/NULL/mem/cache/prefetch/base.hh:204:40: error: 'GuestByteOrder' is
not a member of 'NullISA'
get(ByteOrder endian = TheISA::GuestByteOrder) const
^~~~~~~~~~~~~~
build/NULL/mem/cache/prefetch/base.hh:204:40: note: suggested alternatives:
In file included from build/NULL/mem/port_proxy.hh:68,
from build/NULL/sim/system.hh:64,
from build/NULL/mem/cache/base.hh:81,
from
build/NULL/mem/cache/prefetch/access_map_pattern_matching.hh:43,
from build/NULL/python/_m5/param_AMPMPrefetcher.cc:9:
build/NULL/sim/byteswap.hh:173:21: note: 'BigEndianGuest::GuestByteOrder'
const ByteOrder GuestByteOrder = BigEndianByteOrder;
^~~~~~~~~~~~~~
build/NULL/sim/byteswap.hh:190:21:
note: 'LittleEndianGuest::GuestByteOrder'
const ByteOrder GuestByteOrder = LittleEndianByteOrder;
^~~~~~~~~~~~~~
scons: *** [build/NULL/python/_m5/param_AMPMPrefetcher.o] Error 1
scons: building terminated because of errors.
Change-Id: Ic7fa55c4b5937115e19110ad8cbd88e5679443e4
---
M src/mem/cache/prefetch/base.cc
M src/mem/cache/prefetch/base.hh
2 files changed, 26 insertions(+), 126 deletions(-)
diff --git a/src/mem/cache/prefetch/base.cc b/src/mem/cache/prefetch/base.cc
index 3664892..52f5d1a 100644
--- a/src/mem/cache/prefetch/base.cc
+++ b/src/mem/cache/prefetch/base.cc
@@ -56,26 +56,16 @@
#include "params/BasePrefetcher.hh"
#include "sim/system.hh"
-BasePrefetcher::PrefetchInfo::PrefetchInfo(PacketPtr pkt, Addr addr, bool
miss)
+BasePrefetcher::PrefetchInfo::PrefetchInfo(PacketPtr pkt, Addr addr)
: address(addr), pc(pkt->req->hasPC() ? pkt->req->getPC() : 0),
masterId(pkt->req->masterId()), validPC(pkt->req->hasPC()),
- secure(pkt->isSecure()), size(pkt->req->getSize()),
write(pkt->isWrite()),
- paddress(pkt->req->getPaddr()), cacheMiss(miss)
+ secure(pkt->isSecure())
{
- unsigned int req_size = pkt->req->getSize();
- if (!write && miss) {
- data = nullptr;
- } else {
- data = new uint8_t[req_size];
- Addr offset = pkt->req->getPaddr() - pkt->getAddr();
- std::memcpy(data, &(pkt->getConstPtr<uint8_t>()[offset]),
req_size);
- }
}
BasePrefetcher::PrefetchInfo::PrefetchInfo(PrefetchInfo const &pfi, Addr
addr)
: address(addr), pc(pfi.pc), masterId(pfi.masterId),
validPC(pfi.validPC),
- secure(pfi.secure), size(pfi.size), write(pfi.write),
- paddress(pfi.paddress), cacheMiss(pfi.cacheMiss), data(nullptr)
+ secure(pfi.secure)
{
}
@@ -85,7 +75,7 @@
if (isFill) {
parent.notifyFill(pkt);
} else {
- parent.probeNotify(pkt, miss);
+ parent.probeNotify(pkt);
}
}
@@ -124,11 +114,13 @@
}
bool
-BasePrefetcher::observeAccess(const PacketPtr &pkt, bool miss) const
+BasePrefetcher::observeAccess(const PacketPtr &pkt) const
{
+ Addr addr = pkt->getAddr();
bool fetch = pkt->req->isInstFetch();
bool read = pkt->isRead();
bool inv = pkt->isInvalidate();
+ bool is_secure = pkt->isSecure();
if (pkt->req->isUncacheable()) return false;
if (fetch && !onInst) return false;
@@ -139,7 +131,8 @@
if (pkt->cmd == MemCmd::CleanEvict) return false;
if (onMiss) {
- return miss;
+ return !inCache(addr, is_secure) &&
+ !inMissQueue(addr, is_secure);
}
return true;
@@ -200,28 +193,25 @@
}
void
-BasePrefetcher::probeNotify(const PacketPtr &pkt, bool miss)
+BasePrefetcher::probeNotify(const PacketPtr &pkt)
{
// Don't notify prefetcher on SWPrefetch, cache maintenance
// operations or for writes that we are coaslescing.
if (pkt->cmd.isSWPrefetch()) return;
if (pkt->req->isCacheMaintenance()) return;
if (pkt->isWrite() && cache != nullptr && cache->coalesce()) return;
- if (!pkt->req->hasPaddr()) {
- panic("Request must have a physical address");
- }
if (hasBeenPrefetched(pkt->getAddr(), pkt->isSecure())) {
usefulPrefetches += 1;
}
// Verify this access type is observed by prefetcher
- if (observeAccess(pkt, miss)) {
+ if (observeAccess(pkt)) {
if (useVirtualAddresses && pkt->req->hasVaddr()) {
- PrefetchInfo pfi(pkt, pkt->req->getVaddr(), miss);
+ PrefetchInfo pfi(pkt, pkt->req->getVaddr());
notify(pkt, pfi);
- } else if (!useVirtualAddresses) {
- PrefetchInfo pfi(pkt, pkt->req->getPaddr(), miss);
+ } else if (!useVirtualAddresses && pkt->req->hasPaddr()) {
+ PrefetchInfo pfi(pkt, pkt->req->getPaddr());
notify(pkt, pfi);
}
}
@@ -237,13 +227,10 @@
*/
if (listeners.empty() && cache != nullptr) {
ProbeManager *pm(cache->getProbeManager());
- listeners.push_back(new PrefetchListener(*this, pm, "Miss", false,
- true));
- listeners.push_back(new PrefetchListener(*this, pm, "Fill", true,
- false));
+ listeners.push_back(new PrefetchListener(*this, pm, "Miss"));
+ listeners.push_back(new PrefetchListener(*this, pm, "Fill", true));
if (prefetchOnAccess) {
- listeners.push_back(new PrefetchListener(*this, pm, "Hit",
false,
- false));
+ listeners.push_back(new PrefetchListener(*this, pm, "Hit"));
}
}
}
diff --git a/src/mem/cache/prefetch/base.hh b/src/mem/cache/prefetch/base.hh
index 05895f2..4df8428 100644
--- a/src/mem/cache/prefetch/base.hh
+++ b/src/mem/cache/prefetch/base.hh
@@ -51,12 +51,10 @@
#include <cstdint>
-#include "arch/isa_traits.hh"
#include "base/statistics.hh"
#include "base/types.hh"
#include "mem/packet.hh"
#include "mem/request.hh"
-#include "sim/byteswap.hh"
#include "sim/clocked_object.hh"
#include "sim/probe/probe.hh"
@@ -69,15 +67,13 @@
{
public:
PrefetchListener(BasePrefetcher &_parent, ProbeManager *pm,
- const std::string &name, bool _isFill = false,
- bool _miss = false)
+ const std::string &name, bool _isFill = false)
: ProbeListenerArgBase(pm, name),
- parent(_parent), isFill(_isFill), miss(_miss) {}
+ parent(_parent), isFill(_isFill) {}
void notify(const PacketPtr &pkt) override;
protected:
BasePrefetcher &parent;
- const bool isFill;
- const bool miss;
+ bool isFill;
};
std::vector<PrefetchListener *> listeners;
@@ -89,7 +85,7 @@
* generate new prefetch requests.
*/
class PrefetchInfo {
- /** The address used to train and generate prefetches */
+ /** The address. */
Addr address;
/** The program counter that generated this address. */
Addr pc;
@@ -99,16 +95,6 @@
bool validPC;
/** Whether this address targets the secure memory space. */
bool secure;
- /** Size in bytes of the request triggering this event */
- unsigned int size;
- /** Whether this event comes from a write request */
- bool write;
- /** Physical address, needed because address can be virtual */
- Addr paddress;
- /** Whether this event comes from a cache miss */
- bool cacheMiss;
- /** Pointer to the associated request data */
- uint8_t *data;
public:
/**
@@ -158,67 +144,6 @@
}
/**
- * Gets the size of the request triggering this event
- * @return the size in bytes of the request triggering this event
- */
- unsigned int getSize() const
- {
- return size;
- }
-
- /**
- * Checks if the request that caused this prefetch event was a
write
- * request
- * @return true if the request causing this event is a write
request
- */
- bool isWrite() const
- {
- return write;
- }
-
- /**
- * Gets the physical address of the request
- * @return physical address of the request
- */
- Addr getPaddr() const
- {
- return paddress;
- }
-
- /**
- * Check if this event comes from a cache miss
- * @result true if this event comes from a cache miss
- */
- bool isCacheMiss() const
- {
- return cacheMiss;
- }
-
- /**
- * Gets the associated data of the request triggering the event
- * @param Byte ordering of the stored data
- * @return the data
- */
- template <typename T>
- inline T
- get(ByteOrder endian = TheISA::GuestByteOrder) const
- {
- if (data == nullptr) {
- panic("PrefetchInfo::get called with a request with no
data.");
- }
- switch (endian) {
- case BigEndianByteOrder:
- return betoh(*(T*)data);
-
- case LittleEndianByteOrder:
- return letoh(*(T*)data);
-
- default:
- panic("Illegal byte order in PrefetchInfo::get()\n");
- };
- }
-
- /**
* Check for equality
* @param pfi PrefetchInfo to compare against
* @return True if this object and the provided one are equal
@@ -232,11 +157,9 @@
/**
* Constructs a PrefetchInfo using a PacketPtr.
* @param pkt PacketPtr used to generate the PrefetchInfo
- * @param addr the address value of the new object, this address is
- * used to train the prefetcher
- * @param miss whether this event comes from a cache miss
+ * @param addr the address value of the new object
*/
- PrefetchInfo(PacketPtr pkt, Addr addr, bool miss);
+ PrefetchInfo(PacketPtr pkt, Addr addr);
/**
* Constructs a PrefetchInfo using a new address value and
@@ -245,11 +168,6 @@
* @param addr the address value of the new object
*/
PrefetchInfo(PrefetchInfo const &pfi, Addr addr);
-
- ~PrefetchInfo()
- {
- delete data;
- }
};
protected:
@@ -291,12 +209,8 @@
/** Use Virtual Addresses for prefetching */
const bool useVirtualAddresses;
- /**
- * Determine if this access should be observed
- * @param pkt The memory request causing the event
- * @param miss whether this event comes from a cache miss
- */
- bool observeAccess(const PacketPtr &pkt, bool miss) const;
+ /** Determine if this access should be observed */
+ bool observeAccess(const PacketPtr &pkt) const;
/** Determine if address is in cache */
bool inCache(Addr addr, bool is_secure) const;
@@ -361,9 +275,8 @@
/**
* Process a notification event from the ProbeListener.
* @param pkt The memory request causing the event
- * @param miss whether this event comes from a cache miss
*/
- void probeNotify(const PacketPtr &pkt, bool miss);
+ void probeNotify(const PacketPtr &pkt);
/**
* Add a SimObject and a probe name to listen events from
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/17069
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: Ic7fa55c4b5937115e19110ad8cbd88e5679443e4
Gerrit-Change-Number: 17069
Gerrit-PatchSet: 1
Gerrit-Owner: Ryan Gambord <[email protected]>
Gerrit-Reviewer: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: Javier Bueno Hedo <[email protected]>
Gerrit-Reviewer: Nikos Nikoleris <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
