Random branch misprediction flush fix (#154)

CC: @danbone closes #153 --------- Co-authored-by: Knute Lingaard <[email protected]>
riscv-software-src · Feb 13, 2024 · 6f2cbb7 · 6f2cbb7
1 parent 71e636b
commit 6f2cbb7
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 49 deletions.
diff --git a/core/CPUTopology.cpp b/core/CPUTopology.cpp
@@ -343,13 +343,6 @@ void olympia::CoreTopologySimple::bindTree(sparta::RootTreeNode* root_node)
             ;
             const std::string flush_manager = flushmanager_ports + ".out_flush_upper";
             bind_ports(exe_flush_in, flush_manager);
-
-            // Bind flush requests
-            const std::string exe_flush_out =
-                core_node + ".execute." + unit_name + ".ports.out_execute_flush";
-            ;
-            const std::string flush_manager_in = flushmanager_ports + ".in_flush_request";
-            bind_ports(exe_flush_out, flush_manager_in);
         }
 
         const auto issue_queue_to_pipe_map = olympia::coreutils::getPipeTopology(
@@ -430,4 +423,4 @@ olympia::CPUTopology::allocateTopology(const std::string & topology)
     }
     sparta_assert(nullptr != new_topology);
     return new_topology;
-}
+}
diff --git a/core/ExecutePipe.cpp b/core/ExecutePipe.cpp
@@ -83,9 +83,7 @@ namespace olympia
             if (ex_inst->isBranch() && (std::rand() % 20) == 0)
             {
                 ILOG("Randomly injecting a mispredicted branch: " << ex_inst);
-                FlushManager::FlushingCriteria criteria(FlushManager::FlushCause::MISPREDICTION,
-                                                        ex_inst);
-                out_execute_flush_.send(criteria);
+                ex_inst->setMispredicted();
             }
         }
 

diff --git a/core/ExecutePipe.hpp b/core/ExecutePipe.hpp
@@ -84,8 +84,6 @@ namespace olympia
         sparta::DataOutPort<uint32_t> out_execute_pipe_{&unit_port_set_, "out_execute_pipe"};
         sparta::DataInPort<FlushManager::FlushingCriteria> in_reorder_flush_{
             &unit_port_set_, "in_reorder_flush", sparta::SchedulingPhase::Flush, 1};
-        sparta::DataOutPort<FlushManager::FlushingCriteria> out_execute_flush_{&unit_port_set_,
-                                                                               "out_execute_flush"};
 
         // Scoreboards
         using ScoreboardViews =

diff --git a/core/FlushManager.hpp b/core/FlushManager.hpp
@@ -50,44 +50,44 @@ namespace olympia
             TARGET_MISPREDICTION,
             MISFETCH,
             POST_SYNC,
-            UKNOWN,
+            UNKNOWN,
             __LAST
         };
 
+        static bool determineInclusive(FlushCause cause)
+        {
+            static const std::map<FlushCause, bool> inclusive_flush_map = {
+                {FlushCause::TRAP,                 true},
+                {FlushCause::MISFETCH,             true},
+                {FlushCause::MISPREDICTION,        false},
+                {FlushCause::TARGET_MISPREDICTION, false},
+                {FlushCause::POST_SYNC,            false}
+            };
+
+            if(auto match = inclusive_flush_map.find(cause); match != inclusive_flush_map.end()) {
+                return match->second;
+            }
+            sparta_assert(false, "Unknown flush cause: " << static_cast<uint16_t>(cause));
+            return false;
+        }
+
         class FlushingCriteria
         {
         public:
-            FlushingCriteria(FlushCause cause, InstPtr inst_ptr) :
+            FlushingCriteria(FlushCause cause, const InstPtr & inst_ptr) :
                 cause_(cause),
-                inst_ptr_(inst_ptr) {}
+                is_inclusive_(determineInclusive(cause_)),
+                inst_ptr_(inst_ptr)
+            {}
 
             FlushingCriteria() = default;
             FlushingCriteria(const FlushingCriteria &rhs) = default;
             FlushingCriteria &operator=(const FlushingCriteria &rhs) = default;
 
             FlushCause getCause() const        { return cause_; }
             const InstPtr & getInstPtr() const { return inst_ptr_; }
-
-            bool isInclusiveFlush() const
-            {
-                static const std::map<FlushCause, bool> inclusive_flush_map = {
-                    {FlushCause::TRAP,                 true},
-                    {FlushCause::MISFETCH,             true},
-                    {FlushCause::MISPREDICTION,        false},
-                    {FlushCause::TARGET_MISPREDICTION, false},
-                    {FlushCause::POST_SYNC,            false}
-                };
-                if(auto match = inclusive_flush_map.find(cause_); match != inclusive_flush_map.end()) {
-                    return match->second;
-                }
-                sparta_assert(false, "Unknown flush cause: " << static_cast<uint16_t>(cause_));
-                return true;
-            }
-
-            bool isLowerPipeFlush() const
-            {
-                return cause_ == FlushCause::MISFETCH;
-            }
+            bool isInclusiveFlush() const { return is_inclusive_; }
+            bool isLowerPipeFlush() const { return cause_ == FlushCause::MISFETCH; }
 
             bool includedInFlush(const InstPtr& other) const
             {
@@ -97,11 +97,12 @@ namespace olympia
             }
 
         private:
-            FlushCause cause_ = FlushCause::UKNOWN;
+            // Cannot be const since these are copied into the PLE
+            FlushCause cause_ = FlushCause::UNKNOWN;
+            bool is_inclusive_ = false;
             InstPtr inst_ptr_;
         };
 
-
         static constexpr char name[] = "flushmanager";
 
         class FlushManagerParameters : public sparta::ParameterSet
@@ -202,8 +203,8 @@ namespace olympia
             case FlushManager::FlushCause::POST_SYNC:
                 os << "POST_SYNC";
                 break;
-            case FlushManager::FlushCause::UKNOWN:
-                os << "UKNOWN";
+            case FlushManager::FlushCause::UNKNOWN:
+                os << "UNKNOWN";
                 break;
             case FlushManager::FlushCause::__LAST:
                 throw sparta::SpartaException("__LAST cannot be a valid enum state.");

diff --git a/core/Inst.hpp b/core/Inst.hpp
@@ -178,6 +178,10 @@ namespace olympia
         // Branch instruction was taken (always set for JAL/JALR)
         void setTakenBranch(bool taken) { is_taken_branch_ = taken; }
 
+        // Is this branch instruction mispredicted?
+        bool isMispredicted()  const { return is_mispredicted_; }
+        void setMispredicted()       { is_mispredicted_ = true; }
+
         // TBD -- add branch prediction
         void setSpeculative(bool spec) { is_speculative_ = spec; }
 
@@ -276,6 +280,9 @@ namespace olympia
         const bool is_condbranch_;
         const bool is_call_;
         const bool is_return_;
+
+        // Did this instruction mispredict?
+        bool is_mispredicted_ = false;
         bool is_taken_branch_ = false;
         sparta::Scheduleable* ev_retire_ = nullptr;
         Status status_state_;

diff --git a/core/ROB.cpp b/core/ROB.cpp
@@ -76,10 +76,6 @@ namespace olympia
         ev_ensure_forward_progress_.schedule(retire_timeout_interval_);
     }
 
-    void ROB::retireEvent_() {
-        retireInstructions_();
-    }
-
     // An illustration of the use of the callback -- instead of
     // getting a reference, you can pull the data from the port
     // directly, albeit inefficient and superfluous here...
@@ -94,6 +90,10 @@ namespace olympia
 
     void ROB::handleFlush_(const FlushManager::FlushingCriteria & criteria)
     {
+        sparta_assert(expect_flush_, "Received a flush, but didn't expect one");
+
+        expect_flush_ = false;
+
         uint32_t credits_to_send = 0;
 
         // Clean up internals and send new credit count
@@ -117,6 +117,11 @@ namespace olympia
 
     void ROB::retireInstructions_()
     {
+        // ROB is expecting a flush (back to itself)
+        if(expect_flush_) {
+            return;
+        }
+
         const uint32_t num_to_retire = std::min(reorder_buffer_.size(), num_to_retire_);
 
         ILOG("num to retire: " << num_to_retire);
@@ -171,6 +176,15 @@ namespace olympia
                     break;
                 }
 
+                // Is this a misprdicted branch requiring a refetch?
+                if(ex_inst.isMispredicted()) {
+                    FlushManager::FlushingCriteria criteria
+                        (FlushManager::FlushCause::MISPREDICTION, ex_inst_ptr);
+                    out_retire_flush_.send(criteria);
+                    expect_flush_ = true;
+                    break;
+                }
+
                 // This is rare for the example
                 if(SPARTA_EXPECT_FALSE(ex_inst.getPipe() == InstArchInfo::TargetPipe::SYS))
                 {

diff --git a/core/ROB.hpp b/core/ROB.hpp
@@ -108,9 +108,12 @@ namespace olympia
         sparta::DataInPort<FlushManager::FlushingCriteria> in_reorder_flush_
              {&unit_port_set_, "in_reorder_flush", sparta::SchedulingPhase::Flush, 1};
 
+        // Is the ROB expecting a flush?
+        bool expect_flush_ = false;
+
         // Events used by the ROB
         sparta::UniqueEvent<> ev_retire_ {&unit_event_set_, "retire_insts",
-                CREATE_SPARTA_HANDLER(ROB, retireEvent_)};
+                CREATE_SPARTA_HANDLER(ROB, retireInstructions_)};
 
         // For correlation activities
         sparta::pevents::PeventCollector<InstPEventPairs> retire_event_{"RETIRE", getContainer(), getClock()};
@@ -124,7 +127,6 @@ namespace olympia
         std::unique_ptr<sparta::NotificationSource<bool>> rob_stopped_notif_source_;
 
         void sendInitialCredits_();
-        void retireEvent_();
         void robAppended_(const InstGroup &);
         void retireInstructions_();
         void checkForwardProgress_();

diff --git a/test/sim/CMakeLists.txt b/test/sim/CMakeLists.txt
@@ -58,11 +58,11 @@ sparta_named_test(olympia_json_test_fp_transfers olympia
 
 # Test PEvent generation
 sparta_named_test(olympia_json_test_pevents olympia
-  --workload traces/dhry_riscv.zstf
+  --workload traces/dhry_riscv.zstf -i100k
   --pevents test_pevent.out RETIRE
   --pevents test_pevent.out COMPLETE)
 sparta_named_test(olympia_json_test_pevents_all olympia
-  --workload traces/dhry_riscv.zstf
+  --workload traces/dhry_riscv.zstf  -i100k
   --pevents test_pevent.out all)
 
 ## Test the arches