cpu: modified after review feedback

src/cpu/simple/probes/LooppointAnalysis.py:
- remove default values for bb_valid_addr_range and
marker_valid_addr_range
- add more comments to explain parameter behaviors
- add citation to the LoopPoint paper

src/cpu/simple/probes/looppoint_analysis.cc:
- fix the incorrect styles
- remove updateBackwardBranch() function call
- match the style of checking if listeners vector is empty
- change the way of stopListening() to remove the listeners through the
manager instead of through the ProbeListener object's destructor.

src/cpu/simple/probes/looppoint_analysis.hh:
- removed backwardBranchPC and use the backwardBranchCounter to replace
its functionaility. Therefore, also removed updateBackwardBranch
function.

Change-Id: Id2430e2f04e61f72d5c4f1aad5cfd4d24a0fbc45

src/cpu/simple/probes/LooppointAnalysis.py

@@ -24,6 +24,7 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+from m5.citations import add_citation
 from m5.objects import SimObject
 from m5.objects.Probe import ProbeListenerObject
 from m5.params import *
@@ -54,16 +55,19 @@ class LooppointAnalysis(ProbeListenerObject):
         "the LooppointAnalysis manager"
     )
     bb_valid_addr_range = Param.AddrRange(
-        AddrRange(start=0, end=0), "the valid address range for basic blocks"
+        "the valid address range for basic blocks. If the address start and"
+        "end are both 0 (AddrRange(start=0, end=0)), it means every address is"
+        "valid."
     )
     marker_valid_addr_range = Param.AddrRange(
-        AddrRange(start=0, end=0), "the valid address range for markers"
+        "the valid address range for markers. If the address start and end are"
+        "both 0 (AddrRange(start=0, end=0)), it means every address is valid."
     )
     bb_excluded_addr_ranges = VectorParam.AddrRange(
         [], "the excluded address ranges for basic blocks"
     )
     if_listening = Param.Bool(
-        True, "if the LooppointAnalysis is listening to " "the probe point"
+        True, "if the LooppointAnalysis is listening to the probe point"
     )
 
 
@@ -87,4 +91,43 @@ class LooppointAnalysisManager(SimObject):
         PyBindMethod("getMostRecentBackwardBranchCount"),
     ]
 
-    region_length = Param.Int(100000000, "the length of the region")
+    region_length = Param.Int(100_000_000, "the length of the region")
+
+
+add_citation(
+    LooppointAnalysis,
+    """
+    @INPROCEEDINGS{9773236,
+        author={Sabu, Alen and Patil, Harish and Heirman, Wim and Carlson, Trevor E.},
+        booktitle={2022 IEEE International Symposium on High-Performance Computer Architecture (HPCA)},
+        title={LoopPoint: Checkpoint-driven Sampled Simulation for Multi-threaded Applications},
+        year={2022},
+        volume={},
+        number={},
+        pages={604-618},
+        keywords={Data centers;Codes;Multicore processing;Computational modeling;
+            Computer architecture;Parallel processing;
+            Benchmark testing;checkpointing;multi-threaded;
+            record-and-replay;sampling;simulation},
+        doi={10.1109/HPCA53966.2022.00051}}
+    """,
+)
+
+add_citation(
+    LooppointAnalysisManager,
+    """
+    @INPROCEEDINGS{9773236,
+        author={Sabu, Alen and Patil, Harish and Heirman, Wim and Carlson, Trevor E.},
+        booktitle={2022 IEEE International Symposium on High-Performance Computer Architecture (HPCA)},
+        title={LoopPoint: Checkpoint-driven Sampled Simulation for Multi-threaded Applications},
+        year={2022},
+        volume={},
+        number={},
+        pages={604-618},
+        keywords={Data centers;Codes;Multicore processing;Computational modeling;
+            Computer architecture;Parallel processing;
+            Benchmark testing;checkpointing;multi-threaded;
+            record-and-replay;sampling;simulation},
+        doi={10.1109/HPCA53966.2022.00051}}
+    """,
+)

src/cpu/simple/probes/looppoint_analysis.cc

@@ -42,8 +42,7 @@ LooppointAnalysis::LooppointAnalysis(const LooppointAnalysisParams &params)
     DPRINTF(LooppointAnalysis, "Start listening from the beginning of the "
                             "simulation? %s\n", ifListening ? "Yes" : "No");
 
-    for (int i = 0; i < params.bb_excluded_addr_ranges.size(); i++)
+    for (int i = 0; i < params.bb_excluded_addr_ranges.size(); i++) {
-    {
         bbExcludedAddrRanges.push_back(
             AddrRange(
                 params.bb_excluded_addr_ranges[i].start(),
@@ -65,24 +64,21 @@ LooppointAnalysis::LooppointAnalysis(const LooppointAnalysisParams &params)
 void
 LooppointAnalysis::updateLocalBBV(const Addr pc)
 {
-    if (localBBV.find(pc) == localBBV.end())
+    if (localBBV.find(pc) == localBBV.end()) {
-    {
         localBBV.insert(std::make_pair(pc, 1));
-    }
+    } else {
-    else
-    {
         localBBV.find(pc)->second++;
     }
 }
 
 void
-LooppointAnalysis::checkPc(const std::pair<SimpleThread*,
+LooppointAnalysis::checkPc(
-                                                     StaticInstPtr>& inst_pair)
+    const std::pair<SimpleThread*,StaticInstPtr>& inst_pair
+)
 {
     const StaticInstPtr &inst = inst_pair.second;
 
-    if (inst->isMicroop() && !inst->isLastMicroop())
+    if (inst->isMicroop() && !inst->isLastMicroop()) {
-    {
         // ignore this if it is a microop
         return;
     }
@@ -93,18 +89,14 @@ LooppointAnalysis::checkPc(const std::pair<SimpleThread*,
                 thread->getTC()->pcState().as<GenericISA::PCStateWithNext>();
     Addr pc = pcstate.pc();
 
-    if (lpaManager->ifEncountered(pc))
+    if (lpaManager->ifEncountered(pc)) {
-    {
         // if we have already encountered this pc, we should already
         // categorized it
-        if (lpaManager->ifValidNotControl(pc))
+        if (lpaManager->ifValidNotControl(pc)) {
-        {
             // if it is categorized as a valid not control instruction
             bbInstCounter++;
             lpaManager->incrementGlobalInstCounter();
-        }
+        } else if (lpaManager->ifValidControl(pc)) {
-        else if (lpaManager->ifValidControl(pc))
-        {
             // if it is categorized as a valid control instruction
             bbInstCounter ++;
             lpaManager->incrementGlobalInstCounter();
@@ -112,8 +104,7 @@ LooppointAnalysis::checkPc(const std::pair<SimpleThread*,
             updateLocalBBV(pc);
             lpaManager->updateGlobalBBV(pc);
             bbInstCounter = 0;
-            if (lpaManager->ifBackwardBranch(pc))
+            if (lpaManager->ifBackwardBranch(pc)) {
-            {
                 // if it is categorized as a backward branch
                 lpaManager->countBackwardBranch(pc);
             }
@@ -125,26 +116,19 @@ LooppointAnalysis::checkPc(const std::pair<SimpleThread*,
     // if we have not encountered this pc before, we should now update it to
     // the corresponding category
 
-    if (!thread->getIsaPtr()->inUserMode())
+    if (!thread->getIsaPtr()->inUserMode()) {
-    {
         // ignore this if it is not in user mode
         return;
     }
 
-    if (bbValidAddrRange.end() > 0 &&
+    if (bbValidAddrRange.end() > 0 && ! bbValidAddrRange.contains(pc)) {
-        (pc < bbValidAddrRange.start() || pc > bbValidAddrRange.end()))
-    {
         // ignore this if it is not in the valid address range
         return;
     }
 
-    if (bbExcludedAddrRanges.size() > 0)
+    if (bbExcludedAddrRanges.size() > 0) {
-    {
+        for (int i = 0; i < bbExcludedAddrRanges.size(); i++) {
-        for (int i = 0; i < bbExcludedAddrRanges.size(); i++)
+            if (bbExcludedAddrRanges[i].contains(pc)) {
-        {
-            if (pc >= bbExcludedAddrRanges[i].start() &&
-                pc <= bbExcludedAddrRanges[i].end())
-            {
                 // ignore this if it is in the excluded address range
                 return;
             }
@@ -154,8 +138,7 @@ LooppointAnalysis::checkPc(const std::pair<SimpleThread*,
     bbInstCounter++;
     lpaManager->incrementGlobalInstCounter();
 
-    if (inst->isControl())
+    if (inst->isControl()) {
-    {
         // if it is a control instruction, we see it as the end of a basic
         // block
         lpaManager->updateValidControl(pc);
@@ -164,30 +147,23 @@ LooppointAnalysis::checkPc(const std::pair<SimpleThread*,
         lpaManager->updateGlobalBBV(pc);
         bbInstCounter = 0;
 
-        if (markerValidAddrRange.end() > 0 &&
+        if (markerValidAddrRange.end() > 0
-         (pc < markerValidAddrRange.start() || pc > markerValidAddrRange.end())
+                && ! markerValidAddrRange.contains(pc)) {
-        )
-        {
             // if it is not in the marker valid address range, we do not
             // consider it as a possible marker used loop branch instruction
             return;
         }
 
-        if (inst->isDirectCtrl())
+        if (inst->isDirectCtrl()) {
-        {
             // We only consider direct control instructions as possible
             // loop branch instructions because it is PC-relative and it
             // excludes return instructions.
-            if (pcstate.npc() < pc)
+            if (pcstate.npc() < pc) {
-            {
-                lpaManager->updateBackwardBranch(pc);
                 lpaManager->countBackwardBranch(pc);
             }
 
         }
-    }
+    } else {
-    else
-    {
         lpaManager->updateValidNotControl(pc);
     }
 }
@@ -195,9 +171,8 @@ LooppointAnalysis::checkPc(const std::pair<SimpleThread*,
 void
 LooppointAnalysis::regProbeListeners()
 {
-    if (ifListening)
+    if (ifListening) {
-    {
+        if (listeners.empty()) {
-        if (listeners.size() == 0) {
             listeners.push_back(new looppointAnalysisListener(this,
                                 "Commit", &LooppointAnalysis::checkPc));
             DPRINTF(LooppointAnalysis,
@@ -218,11 +193,18 @@ void
 LooppointAnalysis::stopListening()
 {
     ifListening = false;
-
+    bool _ifRemoved;
-    for (auto l = listeners.begin(); l != listeners.end(); ++l) {
+    for (auto &_listener : listeners) {
-        delete (*l);
+        _ifRemoved = getProbeManager()->removeListener("Commit", *_listener);
+        panic_if(!_ifRemoved, "Failed to remove listener");
+        if (_listener != nullptr) {
+            delete(_listener);
+            DPRINTF(LooppointAnalysis,
+                "Stop listening to the RetiredInstsPC\n");
+        }
     }
     listeners.clear();
+    DPRINTF(LooppointAnalysis, "Stop listening to Commit\n");
 }
 
 LooppointAnalysisManager::LooppointAnalysisManager(const
@@ -238,19 +220,15 @@ LooppointAnalysisManager::LooppointAnalysisManager(const
 void
 LooppointAnalysisManager::countBackwardBranch(const Addr pc)
 {
-    if (backwardBranchCounter.find(pc) == backwardBranchCounter.end())
+    if (backwardBranchCounter.find(pc) == backwardBranchCounter.end()) {
-    {
         backwardBranchCounter.insert(std::make_pair(pc, 1));
-    }
+    } else {
-    else
-    {
         backwardBranchCounter.find(pc)->second++;
     }
 
     mostRecentBackwardBranchPC = pc;
 
-    if (globalInstCounter >= regionLength)
+    if (globalInstCounter >= regionLength) {
-    {
         // note that we do not reset any counter here but only raise an
         // exit event.
         // we can reset the counters through the simulation script using
@@ -267,12 +245,9 @@ LooppointAnalysisManager::countBackwardBranch(const Addr pc)
 void
 LooppointAnalysisManager::updateGlobalBBV(const Addr pc)
 {
-    if (globalBBV.find(pc) == globalBBV.end())
+    if (globalBBV.find(pc) == globalBBV.end()) {
-    {
         globalBBV.insert(std::make_pair(pc, 1));
-    }
+    } else {
-    else
-    {
         globalBBV.find(pc)->second++;
     }
 }

src/cpu/simple/probes/looppoint_analysis.hh

@@ -132,12 +132,14 @@ class LooppointAnalysis : public ProbeListenerObject
     void updateLocalBBV(const Addr pc);
 
   public:
-    std::unordered_map<Addr, uint64_t> getLocalBBV() const
+    std::unordered_map<Addr, uint64_t>
+    getLocalBBV() const
     {
         return localBBV;
     };
 
-    void clearLocalBBV()
+    void
+    clearLocalBBV()
     {
         localBBV.clear();
     };
@@ -209,11 +211,6 @@ class LooppointAnalysisManager: public SimObject
      */
     Addr mostRecentBackwardBranchPC;
 
-    /**
-     * This set stores the Program Counter addresses of the valid backward
-     * branches.
-     */
-    std::unordered_set<Addr> backwardBranchPC;
     /**
      * This set stores the Program Counter addresses of the valid not control
      * instructions.
@@ -231,47 +228,50 @@ class LooppointAnalysisManager: public SimObject
     std::unordered_set<Addr> encounteredPC;
 
   public:
-    bool ifBackwardBranch(const Addr pc) const
+    bool
+    ifBackwardBranch(const Addr pc) const
     {
-        return backwardBranchPC.find(pc) != backwardBranchPC.end();
+        return backwardBranchCounter.find(pc) != backwardBranchCounter.end();
     };
 
-    bool ifValidNotControl(const Addr pc) const
+    bool
+    ifValidNotControl(const Addr pc) const
     {
         return validNotControlPC.find(pc) != validNotControlPC.end();
     };
 
-    bool ifValidControl(const Addr pc) const
+    bool
+    ifValidControl(const Addr pc) const
     {
         return validControlPC.find(pc) != validControlPC.end();
     };
 
-    bool ifEncountered(const Addr pc) const
+    bool
+    ifEncountered(const Addr pc) const
     {
         return encounteredPC.find(pc) != encounteredPC.end();
     };
 
-    void updateBackwardBranch(const Addr pc)
+    void
-    {
+    updateValidNotControl(const Addr pc)
-        backwardBranchPC.insert(pc);
-    };
-
-    void updateValidNotControl(const Addr pc)
     {
         validNotControlPC.insert(pc);
     };
 
-    void updateValidControl(const Addr pc)
+    void
+    updateValidControl(const Addr pc)
     {
         validControlPC.insert(pc);
     };
 
-    void updateEncountered(const Addr pc)
+    void
+    updateEncountered(const Addr pc)
     {
         encounteredPC.insert(pc);
     };
 
-    void updateBBInstMap(Addr pc, uint64_t inst_ount)
+    void
+    updateBBInstMap(Addr pc, uint64_t inst_ount)
     {
         if (bbInstMap.find(pc) == bbInstMap.end())
         {
@@ -279,45 +279,53 @@ class LooppointAnalysisManager: public SimObject
         }
     };
 
-    std::unordered_map<Addr, uint64_t> getGlobalBBV() const
+    std::unordered_map<Addr, uint64_t>
+    getGlobalBBV() const
     {
         return globalBBV;
     };
 
-    void clearGlobalBBV()
+    void
+    clearGlobalBBV()
     {
         globalBBV.clear();
         DPRINTF(LooppointAnalysis,"globalBBV is cleared\n");
     };
 
-    uint64_t getGlobalInstCounter() const
+    uint64_t
+    getGlobalInstCounter() const
     {
         return globalInstCounter;
     };
 
-    void clearGlobalInstCounter()
+    void
+    clearGlobalInstCounter()
     {
         globalInstCounter = 0;
         DPRINTF(LooppointAnalysis,"globalInstCounter is cleared\n current "
             "globalInstCounter = %lu\n", globalInstCounter);
     };
 
-    void incrementGlobalInstCounter()
+    void
+    incrementGlobalInstCounter()
     {
         globalInstCounter++;
     };
 
-    Addr getMostRecentBackwardBranchPC() const
+    Addr
+    getMostRecentBackwardBranchPC() const
     {
         return mostRecentBackwardBranchPC;
     };
 
-    std::unordered_map<Addr, uint64_t> getBackwardBranchCounter() const
+    std::unordered_map<Addr, uint64_t>
+    getBackwardBranchCounter() const
     {
         return backwardBranchCounter;
     };
 
-    uint64_t getMostRecentBackwardBranchCount() const
+    uint64_t
+    getMostRecentBackwardBranchCount() const
     {
         return backwardBranchCounter.find(mostRecentBackwardBranchPC)->second;
     };