From: Daniel R. Carvalho <odanrc@yahoo.com.br>
Date: Fri, 1 May 2020 15:15:42 +0000 (+0200)
Subject: mem-cache: Fixes to PIF prefetcher
X-Git-Tag: v20.1.0.0~669
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=c764e1b3dcadf4dea48db4831e67f360d46988dd;p=gem5.git

mem-cache: Fixes to PIF prefetcher

The temporal compactor was never initialized.

There were more possible indexes to the prec/succ vectors than
entries, so a block distance of zero would seg fault.

When checking for an address the wrong vector was being used.

From the original paper, "The prediction mechanism searches for
the PC of the accessed instruction in the index table"

Change-Id: I3c3aceac3c0adbbe8aef5c634c88cb35ba7487be
Signed-off-by: Daniel R. Carvalho <odanrc@yahoo.com.br>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/28487
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
---

diff --git a/src/mem/cache/prefetch/pif.cc b/src/mem/cache/prefetch/pif.cc
index 6a2983b1b..c557bd29f 100644
--- a/src/mem/cache/prefetch/pif.cc
+++ b/src/mem/cache/prefetch/pif.cc
@@ -75,12 +75,12 @@ PIF::CompactorEntry::inSameSpatialRegion(Addr pc,
     Addr blk_distance = distanceFromTrigger(pc, log_blk_size);
 
     bool hit = (pc > trigger) ?
-        (succ.size() >= blk_distance) : (prec.size() >= blk_distance);
+        (succ.size() > blk_distance) : (prec.size() > blk_distance);
     if (hit && update) {
         if (pc > trigger) {
-            succ[blk_distance - 1] = true;
+            succ[blk_distance] = true;
         } else if (pc < trigger) {
-            prec[blk_distance - 1] = true;
+            prec[blk_distance] = true;
         }
     }
     return hit;
@@ -93,9 +93,9 @@ PIF::CompactorEntry::hasAddress(Addr target,
     Addr blk_distance = distanceFromTrigger(target, log_blk_size);
     bool hit = false;
     if (target > trigger) {
-        hit = blk_distance <= succ.size() && succ[blk_distance - 1];
+        hit = blk_distance < succ.size() && succ[blk_distance];
     } else if (target < trigger) {
-        hit = blk_distance <= prec.size() && succ[blk_distance - 1];
+        hit = blk_distance < prec.size() && prec[blk_distance];
     } else {
         hit = true;
     }
@@ -134,6 +134,7 @@ PIF::notifyRetiredInst(const Addr pc)
     // First access to the prefetcher
     if (temporalCompactor.size() == 0) {
         spatialCompactor = CompactorEntry(pc, precSize, succSize);
+        temporalCompactor.push_back(spatialCompactor);
     } else {
         // If the PC of the instruction retired is in the same spatial region
         // than the last trigger address, update the bit vectors based on the
@@ -195,12 +196,16 @@ void
 PIF::calculatePrefetch(const PrefetchInfo &pfi,
     std::vector<AddrPriority> &addresses)
 {
-    const Addr addr = pfi.getAddr();
+    if (!pfi.hasPC()) {
+        return;
+    }
+
+    const Addr pc = pfi.getPC();
 
     // First check if the access has been prefetched, this is done by
     // comparing the access against the active Stream Address Buffers
     for (auto &sabEntry : streamAddressBuffer) {
-        if (sabEntry->hasAddress(addr, lBlkSize)) {
+        if (sabEntry->hasAddress(pc, lBlkSize)) {
             sabEntry++;
             sabEntry->getPredictedAddresses(lBlkSize, addresses);
             // We are done
@@ -210,7 +215,7 @@ PIF::calculatePrefetch(const PrefetchInfo &pfi,
 
     // Check if a valid entry in the 'index' table is found and allocate a new
     // active prediction stream
-    IndexEntry *idx_entry = index.findEntry(addr, /* unused */ false);
+    IndexEntry *idx_entry = index.findEntry(pc, /* unused */ false);
 
     if (idx_entry != nullptr) {
         index.accessEntry(idx_entry);