From f41e81d9be59ab953f269df71daa31a207b9855f Mon Sep 17 00:00:00 2001
From: Arup Chakraborty <achakraborty@ventanamicro.com>
Date: Mon, 6 Mar 2023 23:08:39 -0800
Subject: [PATCH 01/10] A first take on Branch Prediction API (finally!)

---
 core/BranchPred.hpp | 59 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 core/BranchPred.hpp

diff --git a/core/BranchPred.hpp b/core/BranchPred.hpp
new file mode 100644
index 00000000..541e6a51
--- /dev/null
+++ b/core/BranchPred.hpp
@@ -0,0 +1,59 @@
+// <Branch.hpp> -*- C++ -*-
+// <Fetch.hpp> -*- C++ -*-
+
+//!
+//! \file BranchPred.hpp
+//! \brief Definition of Branch Prediction API
+//!
+
+/*
+ * This file defines the Branch Prediction API.
+ * The goal is to define an API that is generic and yet flexible enough to support various
+ * branch prediction microarchitecture.
+ * To the end, we envision a generic branch predictor as a black box with following inputs
+ * and outputs:
+ *   * A generic Prediction output
+ *   * A generic Prediction input
+ *   * A generic Update input
+ *
+ * The generic branch predictor may have two operations:
+ *   * getPrediction: produces Prediction output based on the Prediction input.
+ *   * updatePredictor: updates Predictor with Update input.
+ *
+ * It is intended that an implementation of branch predictor must also specify
+ * implementations of Prediction output, Prediction input and Update input, along with
+ * implementations of getPrediction and updatePredictor operations.
+ * */
+
+class Prediction; // Prediction output
+class Update;     // Update input
+class Input;      // Prediction input
+
+
+template <class PredictionT, UpdateT, InputT>
+class BranchPredictor
+{
+public:
+    PredictionT & getPrediction(const InputT &);
+    void updatePredictor(UpdateT &);
+};
+
+// following class definitions are example inputs & outputs for a very simple branch
+// predictor
+class Prediction
+{
+public:
+    uint64_t predictedPC;
+};
+
+class Update
+{
+public:
+    uint64_t correctedPC;
+};
+
+class Input
+{
+public:
+    uint64_t FetchPC;
+};

From 41be0f961e155d74fc6f8051ac8727ec72c8170c Mon Sep 17 00:00:00 2001
From: Arup Chakraborty <achakraborty@ventanamicro.com>
Date: Wed, 12 Apr 2023 07:53:56 -0700
Subject: [PATCH 02/10] A draft of branch prediction implemention:

    - Mostly intended to be pseduocode at this stage; compilation not
      attempted
---
 core/BranchPred.cpp | 65 +++++++++++++++++++++++++++++++++++++++++++++
 core/BranchPred.hpp | 11 ++++++--
 2 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 core/BranchPred.cpp

diff --git a/core/BranchPred.cpp b/core/BranchPred.cpp
new file mode 100644
index 00000000..19ce5aea
--- /dev/null
+++ b/core/BranchPred.cpp
@@ -0,0 +1,65 @@
+#include "BranchPred.hpp"
+
+/*
+ * The algorithm used for prediction / update is as follows:
+ * Update:
+ *   - if a branch is actually taken, increment its counter in branch history table
+ *   - if a branch is actually not taken, decrement its counter in branch history table
+ *   - max and min counter values are 3 and 0 respectively
+ *   - if taken, update BTB with target (?)
+ * Prediction:
+ *   - Lookup BHT using the input PC.
+ *      - If an entry exist, determine if it's taken or not-taken historically
+ *        - If not taken, predicted PC = input PC + X bytes, where X is the fixed
+ *          fetch bandwidth (for example, 4*4 = 16 bytes)
+ *        - If taken, look up BTB using input PC and output that as predicted PC
+ *      - If no entry exist, this branch is not seen before. Assume not taken and
+ *        predicted PC = input PC + X bytes
+ *
+ *  Notes:
+ *   - requires an update for each prediction
+ *      - possible optimization ?
+ *   - need to look up fetch PC from predicted PC: how?
+ */
+
+// TODO : must be function of fetch width param
+#define FETCH_WIDTH_BYTES 4 * 4
+
+template <class PredictionT, UpdateT, InputT>
+void BranchPredictor<PredictionT, UpdateT, InputT>::updatePredictor(UpdateT & update) {
+    // look up fetch pc for the predicted PC: how?
+    // uint64_t fetchPC = ...
+    bool isTaken = ((update.correctedPC - fetchPC) != FETCH_WIDTH_BYTES);
+    if (isTaken) {
+        branch_history_table_[fetchPC] =
+            (branch_history_table_[fetchPC] == 3) ? 3 :
+             branch_history_table_[fetchPC] + 1;
+    } else {
+        branch_history_table_[fetchPC] =
+            (branch_history_table_[fetchPC] == 0) ? 0 :
+             branch_history_table_[fetchPC] - 1;
+    }
+
+    if (isTaken) {
+        branch_target_buffer_[fetchPC] = update.correctedPC;
+    }
+}
+
+template <class PredictionT, UpdateT, InputT>
+PredictionT BranchPredictor<PredictionT, UpdateT, InputT>::getPrediction(const InputT & input) {
+    bool predictTaken;
+    if (branch_history_table_.count(input.fetchPC) > 0) {
+        predictTaken = (branch_history_table_[input.fetchPC] > 1);
+    } else {
+        predictTaken = false;
+    }
+
+    PredictionT prediction;
+    if (predictTaken) {
+        prediction.predictedPC = branch_target_buffer[input.fetchPC];
+    } else {
+        prediction.predictedPC = input.fetchPC + FETCH_WIDTH_BYTES;
+    }
+
+    return prediction;
+}
diff --git a/core/BranchPred.hpp b/core/BranchPred.hpp
index 541e6a51..7f5b3870 100644
--- a/core/BranchPred.hpp
+++ b/core/BranchPred.hpp
@@ -1,5 +1,4 @@
 // <Branch.hpp> -*- C++ -*-
-// <Fetch.hpp> -*- C++ -*-
 
 //!
 //! \file BranchPred.hpp
@@ -24,18 +23,25 @@
  * implementations of Prediction output, Prediction input and Update input, along with
  * implementations of getPrediction and updatePredictor operations.
  * */
+#include <map>
 
 class Prediction; // Prediction output
 class Update;     // Update input
 class Input;      // Prediction input
 
 
-template <class PredictionT, UpdateT, InputT>
+template <class PredictionT, class UpdateT, class InputT>
 class BranchPredictor
 {
 public:
+    BrandPredictor() = default;
     PredictionT & getPrediction(const InputT &);
     void updatePredictor(UpdateT &);
+private:
+    // a map of branch PC to 2 bit staurating counter tracking branch history
+    std::map <uint64_t, uint8_t> branch_history_table_; // BHT
+    // a map of branch PC to target of the branch
+    std::map <uint64_t, uint8_t> branch_target_buffer_; // BTB
 };
 
 // following class definitions are example inputs & outputs for a very simple branch
@@ -49,6 +55,7 @@ class Prediction
 class Update
 {
 public:
+    uint64_t predictedPC;
     uint64_t correctedPC;
 };
 

From 367eb841281f159f182fc48da9886a992d0c288d Mon Sep 17 00:00:00 2001
From: Arup Chakraborty <achakraborty@ventanamicro.com>
Date: Tue, 19 Dec 2023 22:55:24 -0800
Subject: [PATCH 03/10] Branch prediction API and an example implementation,
 per discussion #125

---
 core/BranchPred.cpp | 85 +++++++++++++++++++++++----------------------
 core/BranchPred.hpp | 58 +++++++++++++++++++++----------
 2 files changed, 84 insertions(+), 59 deletions(-)

diff --git a/core/BranchPred.cpp b/core/BranchPred.cpp
index 19ce5aea..f0b81fc4 100644
--- a/core/BranchPred.cpp
+++ b/core/BranchPred.cpp
@@ -2,63 +2,66 @@
 
 /*
  * The algorithm used for prediction / update is as follows:
- * Update:
- *   - if a branch is actually taken, increment its counter in branch history table
- *   - if a branch is actually not taken, decrement its counter in branch history table
- *   - max and min counter values are 3 and 0 respectively
- *   - if taken, update BTB with target (?)
  * Prediction:
- *   - Lookup BHT using the input PC.
- *      - If an entry exist, determine if it's taken or not-taken historically
- *        - If not taken, predicted PC = input PC + X bytes, where X is the fixed
- *          fetch bandwidth (for example, 4*4 = 16 bytes)
- *        - If taken, look up BTB using input PC and output that as predicted PC
- *      - If no entry exist, this branch is not seen before. Assume not taken and
- *        predicted PC = input PC + X bytes
+ *    - look up BHT to determine if the branch is predicted taken or not
+ *    - look up BTB to see if an entry exists for the input fetch pc
+ *       - if present in BTB and predicted taken, BTB entry is used to determine
+ *         prediction branch idx and predictedPC
+ *       - if present in BTB but predicted not taken, BTB entry is used to determine
+ *         prediction branch idx, while predictedPC is the fall through addr
+ *       - if not present in BTB entry, prediction branch idx is the last instr of
+ *         the FetchPacket, while predicted PC is the fall through addr. Also, create
+ *         a new BTB entry 
+ * Update:
+ *    - a valid BTB entry must be present for fetch PC
+ *    - TBD 
  *
- *  Notes:
- *   - requires an update for each prediction
- *      - possible optimization ?
- *   - need to look up fetch PC from predicted PC: how?
  */
 
-// TODO : must be function of fetch width param
-#define FETCH_WIDTH_BYTES 4 * 4
+#define BYTES_PER_INST 4
 
-template <class PredictionT, UpdateT, InputT>
-void BranchPredictor<PredictionT, UpdateT, InputT>::updatePredictor(UpdateT & update) {
-    // look up fetch pc for the predicted PC: how?
-    // uint64_t fetchPC = ...
-    bool isTaken = ((update.correctedPC - fetchPC) != FETCH_WIDTH_BYTES);
-    if (isTaken) {
-        branch_history_table_[fetchPC] =
-            (branch_history_table_[fetchPC] == 3) ? 3 :
-             branch_history_table_[fetchPC] + 1;
-    } else {
-        branch_history_table_[fetchPC] =
-            (branch_history_table_[fetchPC] == 0) ? 0 :
-             branch_history_table_[fetchPC] - 1;
-    }
+void SimpleBranchPredictor::updatePredictor(DefaultUpdate & update) {
 
-    if (isTaken) {
-        branch_target_buffer_[fetchPC] = update.correctedPC;
+    sparta_assert(branch_target_buffer_.find(update.FetchPC) != branch_target_buffer_.end());
+    branch_target_buffer_[input.FetchPC].branch_idx = update.branch_idx;
+    if (update.actuallyTaken) {
+        branch_history_table_[update.FetchPC] =
+            (branch_history_table_[update.FetchPC] == 3) ? 3 :
+             branch_history_table_[update.FetchPC] + 1;
+        branch_target_buffer_[input.FetchPC].predictedPC = update.predictedPC;
+    } else {
+        branch_history_table_[update.FetchPC] =
+            (branch_history_table_[update.FetchPC] == 0) ? 0 :
+             branch_history_table_[update.FetchPC] - 1;
     }
 }
 
-template <class PredictionT, UpdateT, InputT>
-PredictionT BranchPredictor<PredictionT, UpdateT, InputT>::getPrediction(const InputT & input) {
+DefaultPrediction SimpleBranchPredictor::getPrediction(const DefaultInput & input) {
     bool predictTaken;
-    if (branch_history_table_.count(input.fetchPC) > 0) {
+    if (branch_history_table_.find(input.FetchPC) != branch_history_table_.end()) {
         predictTaken = (branch_history_table_[input.fetchPC] > 1);
     } else {
         predictTaken = false;
     }
 
-    PredictionT prediction;
-    if (predictTaken) {
-        prediction.predictedPC = branch_target_buffer[input.fetchPC];
+    DefaultPrediction prediction;
+    if (branch_target_buffer_.find(input.FetchPC) != branch_target_buffer_.end()) {
+        // BTB hit
+        BTBEntry btb_entry = branch_target_buffer_[input.FetchPC];
+        prediction.branch_idx = btb_entry.branch_idx;
+        if (predictTaken) {
+            prediction.predictedPC = btb_entry.predictedPC;
+        } else {
+            // fall through address
+            prediction.predictedPC = input.FetchPC + prediction.branch_idx + BYTES_PER_INST;
+        }
     } else {
-        prediction.predictedPC = input.fetchPC + FETCH_WIDTH_BYTES;
+        // BTB miss
+        prediction.branch_idx = max_fetch_insts_;
+        prediction.predictedPC = input.FetchPC + max_fetch_insts_ * BYTES_PER_INST;
+        // add new entry to BTB
+        branch_target_buffer_.insert(std::pair<uint64_t,BTBEntry>(
+            input.FetchPC, BTBEntry(prediction.branch_idx, prediction.predictedPC));
     }
 
     return prediction;
diff --git a/core/BranchPred.hpp b/core/BranchPred.hpp
index 7f5b3870..01a9959e 100644
--- a/core/BranchPred.hpp
+++ b/core/BranchPred.hpp
@@ -25,42 +25,64 @@
  * */
 #include <map>
 
-class Prediction; // Prediction output
-class Update;     // Update input
-class Input;      // Prediction input
-
-
 template <class PredictionT, class UpdateT, class InputT>
-class BranchPredictor
+class BranchPredictorIF
 {
 public:
-    BrandPredictor() = default;
-    PredictionT & getPrediction(const InputT &);
-    void updatePredictor(UpdateT &);
-private:
-    // a map of branch PC to 2 bit staurating counter tracking branch history
-    std::map <uint64_t, uint8_t> branch_history_table_; // BHT
-    // a map of branch PC to target of the branch
-    std::map <uint64_t, uint8_t> branch_target_buffer_; // BTB
+    virtual PredictionT & getPrediction(const InputT &) = 0;
+    virtual void updatePredictor(UpdateT &) = 0;
 };
 
 // following class definitions are example inputs & outputs for a very simple branch
 // predictor
-class Prediction
+class DefaultPrediction
 {
 public:
+    // index of branch instruction in the fetch packet
+    // branch_idx can vary from 0 to (FETCH_WIDTH - 1)
+    uint32_t branch_idx;
+    // predicted target PC 
     uint64_t predictedPC;
 };
 
-class Update
+class DefaultUpdate
 {
 public:
-    uint64_t predictedPC;
+    uint64_t FetchPC;
+    uint32_t branch_idx;
     uint64_t correctedPC;
+    bool actuallyTaken;
 };
 
-class Input
+class DefaultInput
 {
 public:
+    // PC of first instruction of fetch packet
     uint64_t FetchPC;
 };
+
+class BTBEntry 
+{
+public:
+    BTBEntry(uint32_t bidx, uint64_t predPC) :
+        branch_idx(bidx),
+        predictedPC(predPC)
+    {} 
+    uint32_t branch_idx;
+    uint64_t predictedPC; 
+};
+
+class SimpleBranchPredictor : public BranchPredictorIF<DefaultPrediction, DefaultUpdate, DefaultInput> 
+{
+    SimpleBranchPredictor(uint32_t max_fetch_insts) :
+        max_fetch_insts_(max_fetch_insts)
+    {}
+private:
+    // maximum number of instructions in a FetchPacket
+    uint32_t max_fetch_insts_; 
+    // a map of branch PC to 2 bit staurating counter tracking branch history
+    std::map <uint64_t, uint8_t> branch_history_table_; // BHT
+    // a map of branch PC to target of the branch
+    std::map <uint64_t, BTBEntry> branch_target_buffer_; // BTB
+    // 
+};

From ac6094934049e5cc2c2207a6bcde6dfb88193951 Mon Sep 17 00:00:00 2001
From: Arup Chakraborty <achakraborty@ventanamicro.com>
Date: Tue, 16 Jan 2024 23:17:45 -0800
Subject: [PATCH 04/10] Adding branch pred to CMake

---
 core/BranchPred.hpp | 1 +
 core/CMakeLists.txt | 1 +
 2 files changed, 2 insertions(+)

diff --git a/core/BranchPred.hpp b/core/BranchPred.hpp
index 01a9959e..393a9ddc 100644
--- a/core/BranchPred.hpp
+++ b/core/BranchPred.hpp
@@ -23,6 +23,7 @@
  * implementations of Prediction output, Prediction input and Update input, along with
  * implementations of getPrediction and updatePredictor operations.
  * */
+#include <cstdint>
 #include <map>
 
 template <class PredictionT, class UpdateT, class InputT>
diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt
index 886ebc05..297cbadd 100644
--- a/core/CMakeLists.txt
+++ b/core/CMakeLists.txt
@@ -1,6 +1,7 @@
 project (core)
 add_library(core
   Core.cpp
+  BranchPred.cpp
   Fetch.cpp
   Decode.cpp
   Rename.cpp

From 7db3d25a466d3c2fbb1f75094511212149ce427c Mon Sep 17 00:00:00 2001
From: Arup Chakraborty <achakraborty@ventanamicro.com>
Date: Sun, 21 Jan 2024 12:04:10 -0800
Subject: [PATCH 05/10] Fix compilation errors

---
 core/BranchPred.cpp |  8 ++++----
 core/BranchPred.hpp | 11 +++++++++--
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/core/BranchPred.cpp b/core/BranchPred.cpp
index f0b81fc4..28b6596a 100644
--- a/core/BranchPred.cpp
+++ b/core/BranchPred.cpp
@@ -23,12 +23,12 @@
 void SimpleBranchPredictor::updatePredictor(DefaultUpdate & update) {
 
     sparta_assert(branch_target_buffer_.find(update.FetchPC) != branch_target_buffer_.end());
-    branch_target_buffer_[input.FetchPC].branch_idx = update.branch_idx;
+    branch_target_buffer_[update.FetchPC].branch_idx = update.branch_idx;
     if (update.actuallyTaken) {
         branch_history_table_[update.FetchPC] =
             (branch_history_table_[update.FetchPC] == 3) ? 3 :
              branch_history_table_[update.FetchPC] + 1;
-        branch_target_buffer_[input.FetchPC].predictedPC = update.predictedPC;
+        branch_target_buffer_[update.FetchPC].predictedPC = update.correctedPC;
     } else {
         branch_history_table_[update.FetchPC] =
             (branch_history_table_[update.FetchPC] == 0) ? 0 :
@@ -39,7 +39,7 @@ void SimpleBranchPredictor::updatePredictor(DefaultUpdate & update) {
 DefaultPrediction SimpleBranchPredictor::getPrediction(const DefaultInput & input) {
     bool predictTaken;
     if (branch_history_table_.find(input.FetchPC) != branch_history_table_.end()) {
-        predictTaken = (branch_history_table_[input.fetchPC] > 1);
+        predictTaken = (branch_history_table_[input.FetchPC] > 1);
     } else {
         predictTaken = false;
     }
@@ -61,7 +61,7 @@ DefaultPrediction SimpleBranchPredictor::getPrediction(const DefaultInput & inpu
         prediction.predictedPC = input.FetchPC + max_fetch_insts_ * BYTES_PER_INST;
         // add new entry to BTB
         branch_target_buffer_.insert(std::pair<uint64_t,BTBEntry>(
-            input.FetchPC, BTBEntry(prediction.branch_idx, prediction.predictedPC));
+            input.FetchPC, BTBEntry(prediction.branch_idx, prediction.predictedPC)));
     }
 
     return prediction;
diff --git a/core/BranchPred.hpp b/core/BranchPred.hpp
index 393a9ddc..90618b8e 100644
--- a/core/BranchPred.hpp
+++ b/core/BranchPred.hpp
@@ -23,14 +23,17 @@
  * implementations of Prediction output, Prediction input and Update input, along with
  * implementations of getPrediction and updatePredictor operations.
  * */
+#pragma once
+
 #include <cstdint>
 #include <map>
+#include "sparta/utils/SpartaAssert.hpp"
 
 template <class PredictionT, class UpdateT, class InputT>
 class BranchPredictorIF
 {
 public:
-    virtual PredictionT & getPrediction(const InputT &) = 0;
+    virtual PredictionT getPrediction(const InputT &) = 0;
     virtual void updatePredictor(UpdateT &) = 0;
 };
 
@@ -65,7 +68,8 @@ class DefaultInput
 class BTBEntry 
 {
 public:
-    BTBEntry(uint32_t bidx, uint64_t predPC) :
+    // use of BTBEntry in std:map operator [] requires default constructor
+    BTBEntry(uint32_t bidx=0, uint64_t predPC=0) :
         branch_idx(bidx),
         predictedPC(predPC)
     {} 
@@ -75,9 +79,12 @@ class BTBEntry
 
 class SimpleBranchPredictor : public BranchPredictorIF<DefaultPrediction, DefaultUpdate, DefaultInput> 
 {
+public:
     SimpleBranchPredictor(uint32_t max_fetch_insts) :
         max_fetch_insts_(max_fetch_insts)
     {}
+    DefaultPrediction getPrediction(const DefaultInput &);
+    void updatePredictor(DefaultUpdate &);
 private:
     // maximum number of instructions in a FetchPacket
     uint32_t max_fetch_insts_; 

From 948131c3ebb01d3922fca5669c23aac18c46e5f2 Mon Sep 17 00:00:00 2001
From: Arup Chakraborty <achakraborty@ventanamicro.com>
Date: Tue, 6 Feb 2024 21:21:28 -0800
Subject: [PATCH 06/10] Added tests for branch prediction

---
 core/BranchPred.cpp                       |  2 ++
 test/CMakeLists.txt                       |  1 +
 test/core/branch_pred/BranchPred_test.cpp | 43 +++++++++++++++++++++++
 test/core/branch_pred/CMakeLists.txt      |  6 ++++
 4 files changed, 52 insertions(+)
 create mode 100644 test/core/branch_pred/BranchPred_test.cpp
 create mode 100644 test/core/branch_pred/CMakeLists.txt

diff --git a/core/BranchPred.cpp b/core/BranchPred.cpp
index 28b6596a..a56f4385 100644
--- a/core/BranchPred.cpp
+++ b/core/BranchPred.cpp
@@ -42,6 +42,8 @@ DefaultPrediction SimpleBranchPredictor::getPrediction(const DefaultInput & inpu
         predictTaken = (branch_history_table_[input.FetchPC] > 1);
     } else {
         predictTaken = false;
+        // add a new entry to BHT, biased towards not taken
+        branch_history_table_.insert(std::pair<uint64_t, uint8_t>(input.FetchPC, 1));       
     }
 
     DefaultPrediction prediction;
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 399ffaf1..1966aba0 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -31,3 +31,4 @@ add_subdirectory(core/dispatch)
 add_subdirectory(core/l2cache)
 add_subdirectory(core/rename)
 add_subdirectory(core/lsu)
+add_subdirectory(core/branch_pred)
diff --git a/test/core/branch_pred/BranchPred_test.cpp b/test/core/branch_pred/BranchPred_test.cpp
new file mode 100644
index 00000000..13ae0995
--- /dev/null
+++ b/test/core/branch_pred/BranchPred_test.cpp
@@ -0,0 +1,43 @@
+#include "BranchPred.hpp"
+#include "sparta/utils/SpartaTester.hpp"
+
+TEST_INIT
+
+void runTest(int argc, char **argv)
+{
+   SimpleBranchPredictor predictor(4); //specify max num insts to fetch
+
+   DefaultInput input;
+   input.FetchPC = 0x0;
+
+   // BTB miss
+   DefaultPrediction prediction = predictor.getPrediction(input);
+
+   EXPECT_EQUAL(prediction.branch_idx, 4);
+   EXPECT_EQUAL(prediction.predictedPC, 16);
+
+   // there was a taken branch at the 3rd instruction from fetchPC, with target 0x100
+   DefaultUpdate update;
+   update.FetchPC = 0x0;
+   update.branch_idx = 2;
+   update.correctedPC = 0x100;
+   update.actuallyTaken = true;
+   predictor.updatePredictor(update);
+
+   // try the same input with fetchPC 0x0 again
+   prediction = predictor.getPrediction(input);
+   
+   EXPECT_EQUAL(prediction.branch_idx, 2);
+   EXPECT_EQUAL(prediction.predictedPC, 0x100);
+
+   // TODO: add more tests
+            
+}
+
+int main(int argc, char **argv)
+{
+    runTest(argc, argv);
+
+    REPORT_ERROR;
+    return (int)ERROR_CODE;
+}
diff --git a/test/core/branch_pred/CMakeLists.txt b/test/core/branch_pred/CMakeLists.txt
new file mode 100644
index 00000000..a70c99c2
--- /dev/null
+++ b/test/core/branch_pred/CMakeLists.txt
@@ -0,0 +1,6 @@
+project(BranchPred_test)
+
+add_executable(BranchPred_test BranchPred_test.cpp)
+target_link_libraries(BranchPred_test core common_test SPARTA::sparta)
+
+sparta_named_test(BranchPred_test_Run  BranchPred_test)

From b8af1d52701e02e3a1b80b648758d06df508eec3 Mon Sep 17 00:00:00 2001
From: Arup Chakraborty <achakraborty@ventanamicro.com>
Date: Thu, 22 Feb 2024 22:03:06 -0800
Subject: [PATCH 07/10] Address review comments - part 1

---
 core/BranchPred.cpp                       | 56 ++++++++++++-----------
 core/BranchPred.hpp                       | 46 +++++++++++--------
 test/core/branch_pred/BranchPred_test.cpp | 12 ++---
 3 files changed, 63 insertions(+), 51 deletions(-)

diff --git a/core/BranchPred.cpp b/core/BranchPred.cpp
index a56f4385..1c95c190 100644
--- a/core/BranchPred.cpp
+++ b/core/BranchPred.cpp
@@ -4,66 +4,70 @@
  * The algorithm used for prediction / update is as follows:
  * Prediction:
  *    - look up BHT to determine if the branch is predicted taken or not
+ *      using 2-bit saturated counter
+ *       - value 3: strongly taken
+ *       - value 2: weakly taken
+ *       - value 1: weakly not taken
+ *       - value 0: strongly not taken
  *    - look up BTB to see if an entry exists for the input fetch pc
  *       - if present in BTB and predicted taken, BTB entry is used to determine
- *         prediction branch idx and predictedPC
+ *         prediction branch idx and predicted_PC
  *       - if present in BTB but predicted not taken, BTB entry is used to determine
- *         prediction branch idx, while predictedPC is the fall through addr
+ *         prediction branch idx, while predicted_PC is the fall through addr
  *       - if not present in BTB entry, prediction branch idx is the last instr of
  *         the FetchPacket, while predicted PC is the fall through addr. Also, create
- *         a new BTB entry 
+ *         a new BTB entry
  * Update:
  *    - a valid BTB entry must be present for fetch PC
- *    - TBD 
+ *    - TBD
  *
  */
 
 #define BYTES_PER_INST 4
 
-void SimpleBranchPredictor::updatePredictor(DefaultUpdate & update) {
+void SimpleBranchPredictor::updatePredictor(const DefaultUpdate & update) {
 
-    sparta_assert(branch_target_buffer_.find(update.FetchPC) != branch_target_buffer_.end());
-    branch_target_buffer_[update.FetchPC].branch_idx = update.branch_idx;
-    if (update.actuallyTaken) {
-        branch_history_table_[update.FetchPC] =
-            (branch_history_table_[update.FetchPC] == 3) ? 3 :
-             branch_history_table_[update.FetchPC] + 1;
-        branch_target_buffer_[update.FetchPC].predictedPC = update.correctedPC;
+    sparta_assert(branch_target_buffer_.find(update.fetch_PC) != branch_target_buffer_.end());
+    branch_target_buffer_[update.fetch_PC].branch_idx = update.branch_idx;
+    if (update.actually_taken) {
+        branch_history_table_[update.fetch_PC] =
+            (branch_history_table_[update.fetch_PC] == 3) ? 3 :
+             branch_history_table_[update.fetch_PC] + 1;
+        branch_target_buffer_[update.fetch_PC].predicted_PC = update.corrected_PC;
     } else {
-        branch_history_table_[update.FetchPC] =
-            (branch_history_table_[update.FetchPC] == 0) ? 0 :
-             branch_history_table_[update.FetchPC] - 1;
+        branch_history_table_[update.fetch_PC] =
+            (branch_history_table_[update.fetch_PC] == 0) ? 0 :
+             branch_history_table_[update.fetch_PC] - 1;
     }
 }
 
 DefaultPrediction SimpleBranchPredictor::getPrediction(const DefaultInput & input) {
-    bool predictTaken;
-    if (branch_history_table_.find(input.FetchPC) != branch_history_table_.end()) {
-        predictTaken = (branch_history_table_[input.FetchPC] > 1);
+    bool predictTaken = false;
+    if (branch_history_table_.find(input.fetch_PC) != branch_history_table_.end()) {
+        predictTaken = (branch_history_table_[input.fetch_PC] > 1);
     } else {
-        predictTaken = false;
         // add a new entry to BHT, biased towards not taken
-        branch_history_table_.insert(std::pair<uint64_t, uint8_t>(input.FetchPC, 1));       
+        branch_history_table_.insert(std::pair<uint64_t, uint8_t>(input.fetch_PC, 1));
     }
 
     DefaultPrediction prediction;
-    if (branch_target_buffer_.find(input.FetchPC) != branch_target_buffer_.end()) {
+    if (branch_target_buffer_.find(input.fetch_PC) != branch_target_buffer_.end()) {
         // BTB hit
-        BTBEntry btb_entry = branch_target_buffer_[input.FetchPC];
+        const BTBEntry & btb_entry = branch_target_buffer_[input.fetch_PC];
         prediction.branch_idx = btb_entry.branch_idx;
         if (predictTaken) {
-            prediction.predictedPC = btb_entry.predictedPC;
+            prediction.predicted_PC = btb_entry.predicted_PC;
         } else {
             // fall through address
-            prediction.predictedPC = input.FetchPC + prediction.branch_idx + BYTES_PER_INST;
+            prediction.predicted_PC = input.fetch_PC + prediction.branch_idx + BranchPredictorIF::bytes_per_inst;
         }
     } else {
         // BTB miss
         prediction.branch_idx = max_fetch_insts_;
-        prediction.predictedPC = input.FetchPC + max_fetch_insts_ * BYTES_PER_INST;
+        prediction.predicted_PC = input.fetch_PC + max_fetch_insts_ * BYTES_PER_INST;
         // add new entry to BTB
         branch_target_buffer_.insert(std::pair<uint64_t,BTBEntry>(
-            input.FetchPC, BTBEntry(prediction.branch_idx, prediction.predictedPC)));
+            input.fetch_PC, BTBEntry(prediction.branch_idx, prediction.predicted_PC)));
     }
 
     return prediction;
diff --git a/core/BranchPred.hpp b/core/BranchPred.hpp
index 90618b8e..644aae10 100644
--- a/core/BranchPred.hpp
+++ b/core/BranchPred.hpp
@@ -27,14 +27,18 @@
 
 #include <cstdint>
 #include <map>
+#include <limits>
 #include "sparta/utils/SpartaAssert.hpp"
 
 template <class PredictionT, class UpdateT, class InputT>
 class BranchPredictorIF
 {
 public:
+    // TODO: create constexpr for bytes per compressed and uncompressed inst
+    static constexpr uint8_t bytes_per_inst = 4;
+    virtual ~BranchPredictorIF() { };
     virtual PredictionT getPrediction(const InputT &) = 0;
-    virtual void updatePredictor(UpdateT &) = 0;
+    virtual void updatePredictor(const UpdateT &) = 0;
 };
 
 // following class definitions are example inputs & outputs for a very simple branch
@@ -44,53 +48,57 @@ class DefaultPrediction
 public:
     // index of branch instruction in the fetch packet
     // branch_idx can vary from 0 to (FETCH_WIDTH - 1)
-    uint32_t branch_idx;
-    // predicted target PC 
-    uint64_t predictedPC;
+    // initialized to default max to catch errors
+    uint32_t branch_idx = std::numeric_limits<uint32_t>::max();
+    // predicted target PC
+    uint64_t predicted_PC = std::numeric_limits<uint64_t>::max();
 };
 
 class DefaultUpdate
 {
 public:
-    uint64_t FetchPC;
-    uint32_t branch_idx;
-    uint64_t correctedPC;
-    bool actuallyTaken;
+    uint64_t fetch_PC = std::numeric_limits<uint64_t>::max();
+    uint32_t branch_idx = std::numeric_limits<uint32_t>::max();
+    uint64_t corrected_PC = std::numeric_limits<uint64_t>::max();
+    bool actually_taken = false;
 };
 
 class DefaultInput
 {
 public:
     // PC of first instruction of fetch packet
-    uint64_t FetchPC;
+    uint64_t fetch_PC = std::numeric_limits<uint64_t>::max();
 };
 
-class BTBEntry 
+class BTBEntry
 {
 public:
     // use of BTBEntry in std:map operator [] requires default constructor
-    BTBEntry(uint32_t bidx=0, uint64_t predPC=0) :
+    BTBEntry() = default;
+    BTBEntry(uint32_t bidx, uint64_t predPC) :
         branch_idx(bidx),
-        predictedPC(predPC)
-    {} 
-    uint32_t branch_idx;
-    uint64_t predictedPC; 
+        predicted_PC(predPC)
+    {}
+    uint32_t branch_idx {std::numeric_limits<uint32_t>::max()};
+    uint64_t predicted_PC {std::numeric_limits<uint64_t>::max()};
 };
 
-class SimpleBranchPredictor : public BranchPredictorIF<DefaultPrediction, DefaultUpdate, DefaultInput> 
+// Currently SimpleBranchPredictor works only with uncompressed instructions
+// TODO: generalize SimpleBranchPredictor for both compressed and uncompressed instructions
+class SimpleBranchPredictor : public BranchPredictorIF<DefaultPrediction, DefaultUpdate, DefaultInput>
 {
 public:
     SimpleBranchPredictor(uint32_t max_fetch_insts) :
         max_fetch_insts_(max_fetch_insts)
     {}
     DefaultPrediction getPrediction(const DefaultInput &);
-    void updatePredictor(DefaultUpdate &);
+    void updatePredictor(const DefaultUpdate &);
 private:
     // maximum number of instructions in a FetchPacket
-    uint32_t max_fetch_insts_; 
+    const uint32_t max_fetch_insts_;
+    // BHT and BTB of SimpleBranchPredictor is unlimited in size
     // a map of branch PC to 2 bit staurating counter tracking branch history
     std::map <uint64_t, uint8_t> branch_history_table_; // BHT
     // a map of branch PC to target of the branch
     std::map <uint64_t, BTBEntry> branch_target_buffer_; // BTB
-    // 
 };
diff --git a/test/core/branch_pred/BranchPred_test.cpp b/test/core/branch_pred/BranchPred_test.cpp
index 13ae0995..52e1b2b2 100644
--- a/test/core/branch_pred/BranchPred_test.cpp
+++ b/test/core/branch_pred/BranchPred_test.cpp
@@ -8,27 +8,27 @@ void runTest(int argc, char **argv)
    SimpleBranchPredictor predictor(4); //specify max num insts to fetch
 
    DefaultInput input;
-   input.FetchPC = 0x0;
+   input.fetch_PC = 0x0;
 
    // BTB miss
    DefaultPrediction prediction = predictor.getPrediction(input);
 
    EXPECT_EQUAL(prediction.branch_idx, 4);
-   EXPECT_EQUAL(prediction.predictedPC, 16);
+   EXPECT_EQUAL(prediction.predicted_PC, 16);
 
    // there was a taken branch at the 3rd instruction from fetchPC, with target 0x100
    DefaultUpdate update;
-   update.FetchPC = 0x0;
+   update.fetch_PC = 0x0;
    update.branch_idx = 2;
-   update.correctedPC = 0x100;
-   update.actuallyTaken = true;
+   update.corrected_PC = 0x100;
+   update.actually_taken = true;
    predictor.updatePredictor(update);
 
    // try the same input with fetchPC 0x0 again
    prediction = predictor.getPrediction(input);
    
    EXPECT_EQUAL(prediction.branch_idx, 2);
-   EXPECT_EQUAL(prediction.predictedPC, 0x100);
+   EXPECT_EQUAL(prediction.predicted_PC, 0x100);
 
    // TODO: add more tests
             

From 791e0fddf40ec4e278c3be3a9e1e90fd674dea55 Mon Sep 17 00:00:00 2001
From: Arup Chakraborty <achakraborty@ventanamicro.com>
Date: Sun, 25 Feb 2024 13:03:50 -0800
Subject: [PATCH 08/10] Address review comments -- add namespace

---
 core/BranchPred.cpp                       |  85 +++++++-------
 core/BranchPred.hpp                       | 137 +++++++++++-----------
 test/core/branch_pred/BranchPred_test.cpp |   8 +-
 3 files changed, 118 insertions(+), 112 deletions(-)

diff --git a/core/BranchPred.cpp b/core/BranchPred.cpp
index 1c95c190..47e33169 100644
--- a/core/BranchPred.cpp
+++ b/core/BranchPred.cpp
@@ -22,53 +22,54 @@
  *    - TBD
  *
  */
+namespace olympia
+{
 
-#define BYTES_PER_INST 4
+    void SimpleBranchPredictor::updatePredictor(const DefaultUpdate & update) {
 
-void SimpleBranchPredictor::updatePredictor(const DefaultUpdate & update) {
-
-    sparta_assert(branch_target_buffer_.find(update.fetch_PC) != branch_target_buffer_.end());
-    branch_target_buffer_[update.fetch_PC].branch_idx = update.branch_idx;
-    if (update.actually_taken) {
-        branch_history_table_[update.fetch_PC] =
-            (branch_history_table_[update.fetch_PC] == 3) ? 3 :
-             branch_history_table_[update.fetch_PC] + 1;
-        branch_target_buffer_[update.fetch_PC].predicted_PC = update.corrected_PC;
-    } else {
-        branch_history_table_[update.fetch_PC] =
-            (branch_history_table_[update.fetch_PC] == 0) ? 0 :
-             branch_history_table_[update.fetch_PC] - 1;
+        sparta_assert(branch_target_buffer_.find(update.fetch_PC) != branch_target_buffer_.end());
+        branch_target_buffer_[update.fetch_PC].branch_idx = update.branch_idx;
+        if (update.actually_taken) {
+            branch_history_table_[update.fetch_PC] =
+                (branch_history_table_[update.fetch_PC] == 3) ? 3 :
+                 branch_history_table_[update.fetch_PC] + 1;
+            branch_target_buffer_[update.fetch_PC].predicted_PC = update.corrected_PC;
+        } else {
+            branch_history_table_[update.fetch_PC] =
+                (branch_history_table_[update.fetch_PC] == 0) ? 0 :
+                 branch_history_table_[update.fetch_PC] - 1;
+        }
     }
-}
 
-DefaultPrediction SimpleBranchPredictor::getPrediction(const DefaultInput & input) {
-    bool predictTaken = false;
-    if (branch_history_table_.find(input.fetch_PC) != branch_history_table_.end()) {
-        predictTaken = (branch_history_table_[input.fetch_PC] > 1);
-    } else {
-        // add a new entry to BHT, biased towards not taken
-        branch_history_table_.insert(std::pair<uint64_t, uint8_t>(input.fetch_PC, 1));
-    }
+    DefaultPrediction SimpleBranchPredictor::getPrediction(const DefaultInput & input) {
+        bool predictTaken = false;
+        if (branch_history_table_.find(input.fetch_PC) != branch_history_table_.end()) {
+            predictTaken = (branch_history_table_[input.fetch_PC] > 1);
+        } else {
+            // add a new entry to BHT, biased towards not taken
+            branch_history_table_.insert(std::pair<uint64_t, uint8_t>(input.fetch_PC, 1));
+        }
 
-    DefaultPrediction prediction;
-    if (branch_target_buffer_.find(input.fetch_PC) != branch_target_buffer_.end()) {
-        // BTB hit
-        const BTBEntry & btb_entry = branch_target_buffer_[input.fetch_PC];
-        prediction.branch_idx = btb_entry.branch_idx;
-        if (predictTaken) {
-            prediction.predicted_PC = btb_entry.predicted_PC;
+        DefaultPrediction prediction;
+        if (branch_target_buffer_.find(input.fetch_PC) != branch_target_buffer_.end()) {
+            // BTB hit
+            const BTBEntry & btb_entry = branch_target_buffer_[input.fetch_PC];
+            prediction.branch_idx = btb_entry.branch_idx;
+            if (predictTaken) {
+                prediction.predicted_PC = btb_entry.predicted_PC;
+            } else {
+                // fall through address
+                prediction.predicted_PC = input.fetch_PC + prediction.branch_idx + BranchPredictorIF::bytes_per_inst;
+            }
         } else {
-            // fall through address
-            prediction.predicted_PC = input.fetch_PC + prediction.branch_idx + BranchPredictorIF::bytes_per_inst;
+            // BTB miss
+            prediction.branch_idx = max_fetch_insts_;
+            prediction.predicted_PC = input.fetch_PC + max_fetch_insts_ * bytes_per_inst;
+            // add new entry to BTB
+            branch_target_buffer_.insert(std::pair<uint64_t,BTBEntry>(
+                input.fetch_PC, BTBEntry(prediction.branch_idx, prediction.predicted_PC)));
         }
-    } else {
-        // BTB miss
-        prediction.branch_idx = max_fetch_insts_;
-        prediction.predicted_PC = input.fetch_PC + max_fetch_insts_ * BYTES_PER_INST;
-        // add new entry to BTB
-        branch_target_buffer_.insert(std::pair<uint64_t,BTBEntry>(
-            input.fetch_PC, BTBEntry(prediction.branch_idx, prediction.predicted_PC)));
-    }
 
-    return prediction;
-}
+        return prediction;
+    }
+} // namespace olympia
diff --git a/core/BranchPred.hpp b/core/BranchPred.hpp
index 644aae10..ebc444ac 100644
--- a/core/BranchPred.hpp
+++ b/core/BranchPred.hpp
@@ -30,75 +30,80 @@
 #include <limits>
 #include "sparta/utils/SpartaAssert.hpp"
 
-template <class PredictionT, class UpdateT, class InputT>
-class BranchPredictorIF
+namespace olympia
 {
-public:
-    // TODO: create constexpr for bytes per compressed and uncompressed inst
-    static constexpr uint8_t bytes_per_inst = 4;
-    virtual ~BranchPredictorIF() { };
-    virtual PredictionT getPrediction(const InputT &) = 0;
-    virtual void updatePredictor(const UpdateT &) = 0;
-};
 
-// following class definitions are example inputs & outputs for a very simple branch
-// predictor
-class DefaultPrediction
-{
-public:
-    // index of branch instruction in the fetch packet
-    // branch_idx can vary from 0 to (FETCH_WIDTH - 1)
-    // initialized to default max to catch errors
-    uint32_t branch_idx = std::numeric_limits<uint32_t>::max();
-    // predicted target PC
-    uint64_t predicted_PC = std::numeric_limits<uint64_t>::max();
-};
+    template <class PredictionT, class UpdateT, class InputT>
+    class BranchPredictorIF
+    {
+    public:
+        // TODO: create constexpr for bytes per compressed and uncompressed inst
+        static constexpr uint8_t bytes_per_inst = 4;
+        virtual ~BranchPredictorIF() { };
+        virtual PredictionT getPrediction(const InputT &) = 0;
+        virtual void updatePredictor(const UpdateT &) = 0;
+    };
 
-class DefaultUpdate
-{
-public:
-    uint64_t fetch_PC = std::numeric_limits<uint64_t>::max();
-    uint32_t branch_idx = std::numeric_limits<uint32_t>::max();
-    uint64_t corrected_PC = std::numeric_limits<uint64_t>::max();
-    bool actually_taken = false;
-};
+    // following class definitions are example inputs & outputs for a very simple branch
+    // predictor
+    class DefaultPrediction
+    {
+    public:
+        // index of branch instruction in the fetch packet
+        // branch_idx can vary from 0 to (FETCH_WIDTH - 1)
+        // initialized to default max to catch errors
+        uint32_t branch_idx = std::numeric_limits<uint32_t>::max();
+        // predicted target PC
+        uint64_t predicted_PC = std::numeric_limits<uint64_t>::max();
+    };
 
-class DefaultInput
-{
-public:
-    // PC of first instruction of fetch packet
-    uint64_t fetch_PC = std::numeric_limits<uint64_t>::max();
-};
+    class DefaultUpdate
+    {
+    public:
+        uint64_t fetch_PC = std::numeric_limits<uint64_t>::max();
+        uint32_t branch_idx = std::numeric_limits<uint32_t>::max();
+        uint64_t corrected_PC = std::numeric_limits<uint64_t>::max();
+        bool actually_taken = false;
+    };
 
-class BTBEntry
-{
-public:
-    // use of BTBEntry in std:map operator [] requires default constructor
-    BTBEntry() = default;
-    BTBEntry(uint32_t bidx, uint64_t predPC) :
-        branch_idx(bidx),
-        predicted_PC(predPC)
-    {}
-    uint32_t branch_idx {std::numeric_limits<uint32_t>::max()};
-    uint64_t predicted_PC {std::numeric_limits<uint64_t>::max()};
-};
+    class DefaultInput
+    {
+    public:
+        // PC of first instruction of fetch packet
+        uint64_t fetch_PC = std::numeric_limits<uint64_t>::max();
+    };
 
-// Currently SimpleBranchPredictor works only with uncompressed instructions
-// TODO: generalize SimpleBranchPredictor for both compressed and uncompressed instructions
-class SimpleBranchPredictor : public BranchPredictorIF<DefaultPrediction, DefaultUpdate, DefaultInput>
-{
-public:
-    SimpleBranchPredictor(uint32_t max_fetch_insts) :
-        max_fetch_insts_(max_fetch_insts)
-    {}
-    DefaultPrediction getPrediction(const DefaultInput &);
-    void updatePredictor(const DefaultUpdate &);
-private:
-    // maximum number of instructions in a FetchPacket
-    const uint32_t max_fetch_insts_;
-    // BHT and BTB of SimpleBranchPredictor is unlimited in size
-    // a map of branch PC to 2 bit staurating counter tracking branch history
-    std::map <uint64_t, uint8_t> branch_history_table_; // BHT
-    // a map of branch PC to target of the branch
-    std::map <uint64_t, BTBEntry> branch_target_buffer_; // BTB
-};
+    class BTBEntry
+    {
+    public:
+        // use of BTBEntry in std:map operator [] requires default constructor
+        BTBEntry() = default;
+        BTBEntry(uint32_t bidx, uint64_t predPC) :
+            branch_idx(bidx),
+            predicted_PC(predPC)
+        {}
+        uint32_t branch_idx {std::numeric_limits<uint32_t>::max()};
+        uint64_t predicted_PC {std::numeric_limits<uint64_t>::max()};
+    };
+
+    // Currently SimpleBranchPredictor works only with uncompressed instructions
+    // TODO: generalize SimpleBranchPredictor for both compressed and uncompressed instructions
+    class SimpleBranchPredictor : public BranchPredictorIF<DefaultPrediction, DefaultUpdate, DefaultInput>
+    {
+    public:
+        SimpleBranchPredictor(uint32_t max_fetch_insts) :
+            max_fetch_insts_(max_fetch_insts)
+        {}
+        DefaultPrediction getPrediction(const DefaultInput &);
+        void updatePredictor(const DefaultUpdate &);
+    private:
+        // maximum number of instructions in a FetchPacket
+        const uint32_t max_fetch_insts_;
+        // BHT and BTB of SimpleBranchPredictor is unlimited in size
+        // a map of branch PC to 2 bit staurating counter tracking branch history
+        std::map <uint64_t, uint8_t> branch_history_table_; // BHT
+        // a map of branch PC to target of the branch
+        std::map <uint64_t, BTBEntry> branch_target_buffer_; // BTB
+    };
+
+} // namespace olympia
diff --git a/test/core/branch_pred/BranchPred_test.cpp b/test/core/branch_pred/BranchPred_test.cpp
index 52e1b2b2..fd325526 100644
--- a/test/core/branch_pred/BranchPred_test.cpp
+++ b/test/core/branch_pred/BranchPred_test.cpp
@@ -5,19 +5,19 @@ TEST_INIT
 
 void runTest(int argc, char **argv)
 {
-   SimpleBranchPredictor predictor(4); //specify max num insts to fetch
+   olympia::SimpleBranchPredictor predictor(4); //specify max num insts to fetch
 
-   DefaultInput input;
+   olympia::DefaultInput input;
    input.fetch_PC = 0x0;
 
    // BTB miss
-   DefaultPrediction prediction = predictor.getPrediction(input);
+   olympia::DefaultPrediction prediction = predictor.getPrediction(input);
 
    EXPECT_EQUAL(prediction.branch_idx, 4);
    EXPECT_EQUAL(prediction.predicted_PC, 16);
 
    // there was a taken branch at the 3rd instruction from fetchPC, with target 0x100
-   DefaultUpdate update;
+   olympia::DefaultUpdate update;
    update.fetch_PC = 0x0;
    update.branch_idx = 2;
    update.corrected_PC = 0x100;

From 4e19e1c84d6e122b309bb062a06ee16c09b5fe0d Mon Sep 17 00:00:00 2001
From: Arup Chakraborty <achakraborty@ventanamicro.com>
Date: Sun, 25 Feb 2024 20:41:06 -0800
Subject: [PATCH 09/10] Separate Branch Prediction IF from Simple Predictor,
 per review comments

---
 core/BranchPredIF.hpp                         | 42 +++++++++++++++++++
 core/CMakeLists.txt                           |  2 +-
 core/{BranchPred.cpp => SimpleBranchPred.cpp} |  3 +-
 core/{BranchPred.hpp => SimpleBranchPred.hpp} | 37 ++++------------
 test/core/branch_pred/BranchPred_test.cpp     |  2 +-
 5 files changed, 53 insertions(+), 33 deletions(-)
 create mode 100644 core/BranchPredIF.hpp
 rename core/{BranchPred.cpp => SimpleBranchPred.cpp} (99%)
 rename core/{BranchPred.hpp => SimpleBranchPred.hpp} (65%)

diff --git a/core/BranchPredIF.hpp b/core/BranchPredIF.hpp
new file mode 100644
index 00000000..c67b1d0b
--- /dev/null
+++ b/core/BranchPredIF.hpp
@@ -0,0 +1,42 @@
+// <Branch.hpp> -*- C++ -*-
+
+//!
+//! \file BranchPred.hpp
+//! \brief Definition of Branch Prediction API
+//!
+
+/*
+ * This file defines the Branch Prediction API.
+ * The goal is to define an API that is generic and yet flexible enough to support various
+ * branch prediction microarchitecture.
+ * To the end, we envision a generic branch predictor as a black box with following inputs
+ * and outputs:
+ *   * A generic Prediction output
+ *   * A generic Prediction input
+ *   * A generic Update input
+ *
+ * The generic branch predictor may have two operations:
+ *   * getPrediction: produces Prediction output based on the Prediction input.
+ *   * updatePredictor: updates Predictor with Update input.
+ *
+ * It is intended that an implementation of branch predictor must also specify
+ * implementations of Prediction output, Prediction input and Update input, along with
+ * implementations of getPrediction and updatePredictor operations.
+ * */
+#pragma once
+
+namespace olympia
+{
+
+    template <class PredictionT, class UpdateT, class InputT>
+    class BranchPredictorIF
+    {
+    public:
+        // TODO: create constexpr for bytes per compressed and uncompressed inst
+        static constexpr uint8_t bytes_per_inst = 4;
+        virtual ~BranchPredictorIF() { };
+        virtual PredictionT getPrediction(const InputT &) = 0;
+        virtual void updatePredictor(const UpdateT &) = 0;
+    };
+
+} // namespace olympia
diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt
index 297cbadd..5142395c 100644
--- a/core/CMakeLists.txt
+++ b/core/CMakeLists.txt
@@ -1,7 +1,7 @@
 project (core)
 add_library(core
   Core.cpp
-  BranchPred.cpp
+  SimpleBranchPred.cpp
   Fetch.cpp
   Decode.cpp
   Rename.cpp
diff --git a/core/BranchPred.cpp b/core/SimpleBranchPred.cpp
similarity index 99%
rename from core/BranchPred.cpp
rename to core/SimpleBranchPred.cpp
index 47e33169..2dc1231d 100644
--- a/core/BranchPred.cpp
+++ b/core/SimpleBranchPred.cpp
@@ -1,4 +1,4 @@
-#include "BranchPred.hpp"
+#include "SimpleBranchPred.hpp"
 
 /*
  * The algorithm used for prediction / update is as follows:
@@ -72,4 +72,5 @@ namespace olympia
 
         return prediction;
     }
+
 } // namespace olympia
diff --git a/core/BranchPred.hpp b/core/SimpleBranchPred.hpp
similarity index 65%
rename from core/BranchPred.hpp
rename to core/SimpleBranchPred.hpp
index ebc444ac..55a17fde 100644
--- a/core/BranchPred.hpp
+++ b/core/SimpleBranchPred.hpp
@@ -1,27 +1,14 @@
-// <Branch.hpp> -*- C++ -*-
+// <SimpleBranchPred.hpp> -*- C++ -*-
 
 //!
-//! \file BranchPred.hpp
-//! \brief Definition of Branch Prediction API
+//! \file SimpleBranchPred.hpp
+//! \brief Class definition of a simple brranch prediction using branch prediction interface
 //!
 
 /*
- * This file defines the Branch Prediction API.
- * The goal is to define an API that is generic and yet flexible enough to support various
- * branch prediction microarchitecture.
- * To the end, we envision a generic branch predictor as a black box with following inputs
- * and outputs:
- *   * A generic Prediction output
- *   * A generic Prediction input
- *   * A generic Update input
- *
- * The generic branch predictor may have two operations:
- *   * getPrediction: produces Prediction output based on the Prediction input.
- *   * updatePredictor: updates Predictor with Update input.
- *
- * It is intended that an implementation of branch predictor must also specify
- * implementations of Prediction output, Prediction input and Update input, along with
- * implementations of getPrediction and updatePredictor operations.
+ * This file defines the class SimpleBranchPredictor, as well as, a default Prediction 
+ * output class, a default Prediction input class, a defeault Prediction input class
+ * as required by Olympia's Branch Prediction inteface
  * */
 #pragma once
 
@@ -29,21 +16,11 @@
 #include <map>
 #include <limits>
 #include "sparta/utils/SpartaAssert.hpp"
+#include "BranchPredIF.hpp"
 
 namespace olympia
 {
 
-    template <class PredictionT, class UpdateT, class InputT>
-    class BranchPredictorIF
-    {
-    public:
-        // TODO: create constexpr for bytes per compressed and uncompressed inst
-        static constexpr uint8_t bytes_per_inst = 4;
-        virtual ~BranchPredictorIF() { };
-        virtual PredictionT getPrediction(const InputT &) = 0;
-        virtual void updatePredictor(const UpdateT &) = 0;
-    };
-
     // following class definitions are example inputs & outputs for a very simple branch
     // predictor
     class DefaultPrediction
diff --git a/test/core/branch_pred/BranchPred_test.cpp b/test/core/branch_pred/BranchPred_test.cpp
index fd325526..31ec580d 100644
--- a/test/core/branch_pred/BranchPred_test.cpp
+++ b/test/core/branch_pred/BranchPred_test.cpp
@@ -1,4 +1,4 @@
-#include "BranchPred.hpp"
+#include "SimpleBranchPred.hpp"
 #include "sparta/utils/SpartaTester.hpp"
 
 TEST_INIT

From 1e61b06503ef202847884ac3d47e6bb3a2d99c15 Mon Sep 17 00:00:00 2001
From: Arup Chakraborty <achakraborty@ventanamicro.com>
Date: Sun, 3 Mar 2024 18:17:08 -0800
Subject: [PATCH 10/10] Address review comments: add separate namespace for
 Branch Predictor

---
 core/BranchPredIF.hpp                     | 3 +++
 core/SimpleBranchPred.cpp                 | 3 +++
 core/SimpleBranchPred.hpp                 | 3 +++
 test/core/branch_pred/BranchPred_test.cpp | 8 ++++----
 4 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/core/BranchPredIF.hpp b/core/BranchPredIF.hpp
index c67b1d0b..775521a1 100644
--- a/core/BranchPredIF.hpp
+++ b/core/BranchPredIF.hpp
@@ -26,6 +26,8 @@
 #pragma once
 
 namespace olympia
+{
+namespace BranchPredictor
 {
 
     template <class PredictionT, class UpdateT, class InputT>
@@ -39,4 +41,5 @@ namespace olympia
         virtual void updatePredictor(const UpdateT &) = 0;
     };
 
+} // namespace BranchPredictor
 } // namespace olympia
diff --git a/core/SimpleBranchPred.cpp b/core/SimpleBranchPred.cpp
index 2dc1231d..9c679ea5 100644
--- a/core/SimpleBranchPred.cpp
+++ b/core/SimpleBranchPred.cpp
@@ -23,6 +23,8 @@
  *
  */
 namespace olympia
+{
+namespace BranchPredictor
 {
 
     void SimpleBranchPredictor::updatePredictor(const DefaultUpdate & update) {
@@ -73,4 +75,5 @@ namespace olympia
         return prediction;
     }
 
+} // namespace BranchPredictor
 } // namespace olympia
diff --git a/core/SimpleBranchPred.hpp b/core/SimpleBranchPred.hpp
index 55a17fde..53f9bccc 100644
--- a/core/SimpleBranchPred.hpp
+++ b/core/SimpleBranchPred.hpp
@@ -19,6 +19,8 @@
 #include "BranchPredIF.hpp"
 
 namespace olympia
+{
+namespace BranchPredictor
 {
 
     // following class definitions are example inputs & outputs for a very simple branch
@@ -83,4 +85,5 @@ namespace olympia
         std::map <uint64_t, BTBEntry> branch_target_buffer_; // BTB
     };
 
+} // namespace BranchPredictor
 } // namespace olympia
diff --git a/test/core/branch_pred/BranchPred_test.cpp b/test/core/branch_pred/BranchPred_test.cpp
index 31ec580d..b37ffe15 100644
--- a/test/core/branch_pred/BranchPred_test.cpp
+++ b/test/core/branch_pred/BranchPred_test.cpp
@@ -5,19 +5,19 @@ TEST_INIT
 
 void runTest(int argc, char **argv)
 {
-   olympia::SimpleBranchPredictor predictor(4); //specify max num insts to fetch
+   olympia::BranchPredictor::SimpleBranchPredictor predictor(4); //specify max num insts to fetch
 
-   olympia::DefaultInput input;
+   olympia::BranchPredictor::DefaultInput input;
    input.fetch_PC = 0x0;
 
    // BTB miss
-   olympia::DefaultPrediction prediction = predictor.getPrediction(input);
+   olympia::BranchPredictor::DefaultPrediction prediction = predictor.getPrediction(input);
 
    EXPECT_EQUAL(prediction.branch_idx, 4);
    EXPECT_EQUAL(prediction.predicted_PC, 16);
 
    // there was a taken branch at the 3rd instruction from fetchPC, with target 0x100
-   olympia::DefaultUpdate update;
+   olympia::BranchPredictor::DefaultUpdate update;
    update.fetch_PC = 0x0;
    update.branch_idx = 2;
    update.corrected_PC = 0x100;