Bobby Bruce has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/64832?usp=email )

Change subject: stdlib: Update AbstractCore `set_simpoint` func
......................................................................

stdlib: Update AbstractCore `set_simpoint` func

This change:
- Makes this function private.
- Adds better documentation describing the usage.
- Changes the 'init' param to 'board_initialized'

This function really doesn't make much sense to set directly by an
stdlib user. It requires knowing whether or not the the board has been
initialized which is an annoying detail and will cause error if set
incorrectly.

The logic of the `init` parameter has been flipped to be
`board_initialized`. This makes it clearer what the parameter is
doing and what it's for.

The documentation for this function has been updated to make it clearer
on how the `board_initialized` parameter should be used correctly.

Change-Id: I567a48df06e6327b38673a2c510065d4334657e2
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/64832
Reviewed-by: Melissa Jost <mkj...@ucdavis.edu>
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Maintainer: Bobby Bruce <bbr...@ucdavis.edu>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/python/gem5/components/boards/se_binary_workload.py
M src/python/gem5/components/processors/abstract_core.py
M src/python/gem5/components/processors/base_cpu_core.py
M src/python/gem5/simulate/simulator.py
4 files changed, 49 insertions(+), 17 deletions(-)

Approvals:
  Melissa Jost: Looks good to me, approved
  kokoro: Regressions pass
  Jason Lowe-Power: Looks good to me, approved
  Bobby Bruce: Looks good to me, approved




diff --git a/src/python/gem5/components/boards/se_binary_workload.py b/src/python/gem5/components/boards/se_binary_workload.py
index b9ecc7f..8ec112e 100644
--- a/src/python/gem5/components/boards/se_binary_workload.py
+++ b/src/python/gem5/components/boards/se_binary_workload.py
@@ -149,9 +149,9 @@

         if self.get_processor().get_num_cores() > 1:
             warn("SimPoints only works with one core")
-        self.get_processor().get_cores()[0].set_simpoint(
+        self.get_processor().get_cores()[0]._set_simpoint(
             inst_starts=self._simpoint_object.get_simpoint_start_insts(),
-            init=True,
+            board_initialized=False,
         )

         # Call set_se_binary_workload after SimPoint setup is complete
diff --git a/src/python/gem5/components/processors/abstract_core.py b/src/python/gem5/components/processors/abstract_core.py
index 2f4cb79..ea2875e 100644
--- a/src/python/gem5/components/processors/abstract_core.py
+++ b/src/python/gem5/components/processors/abstract_core.py
@@ -122,17 +122,20 @@
         raise NotImplementedError

     @abstractmethod
-    def set_simpoint(self, inst_starts: List[int], init: bool) -> None:
+    def _set_simpoint(
+        self, inst_starts: List[int], board_initialized: bool
+    ) -> None:
         """Schedule simpoint exit events for the core.

This is used to raise SIMPOINT_BEGIN exit events in the gem5 standard - library. Duplicate instruction counts in the inst_starts list will not + library. This is called through the set_workload functions and should + not be called directly. Duplicate instruction counts in the inst_starts list will not
         be scheduled.

         :param inst_starts: a list of SimPoints starting instructions
- :param init: if it is True, the starting instructions will be scheduled - at the init stage of the core, else, the starting insructions will be
-        scheduled during the simulation
+        :param board_initialized: True if the board has already been
+ initialized, otherwise False. This parameter is necessary as simpoints
+        are setup differently dependent on this.
         """
raise NotImplementedError("This core type does not support simpoints")

diff --git a/src/python/gem5/components/processors/base_cpu_core.py b/src/python/gem5/components/processors/base_cpu_core.py
index db9a1a2..535a800 100644
--- a/src/python/gem5/components/processors/base_cpu_core.py
+++ b/src/python/gem5/components/processors/base_cpu_core.py
@@ -153,11 +153,13 @@
         return self.core.mmu

     @overrides(AbstractCore)
-    def set_simpoint(self, inst_starts: List[int], init: bool) -> None:
-        if init:
-            self.core.simpoint_start_insts = sorted(set(inst_starts))
-        else:
+    def _set_simpoint(
+        self, inst_starts: List[int], board_initialized: bool
+    ) -> None:
+        if board_initialized:
             self.core.scheduleSimpointsInstStop(sorted(set(inst_starts)))
+        else:
+            self.core.simpoint_start_insts = sorted(set(inst_starts))

     @overrides(AbstractCore)
     def set_inst_stop_any_thread(self, inst: int, init: bool) -> None:
diff --git a/src/python/gem5/simulate/simulator.py b/src/python/gem5/simulate/simulator.py
index 8be915e..d7e2141 100644
--- a/src/python/gem5/simulate/simulator.py
+++ b/src/python/gem5/simulate/simulator.py
@@ -236,9 +236,7 @@

         self._checkpoint_path = checkpoint_path

-    def schedule_simpoint(
- self, simpoint_start_insts: List[int], schedule_at_init: bool = False
-    ) -> None:
+    def schedule_simpoint(self, simpoint_start_insts: List[int]) -> None:
         """
         Schedule SIMPOINT_BEGIN exit events

@@ -246,13 +244,11 @@

         :param simpoint_start_insts: a list of number of instructions
         indicating the starting point of the simpoints
- :param schedule_at_init: if it is True, schedule the events in the init
-        stage of the core, else, schedule the events during the simulation
         """
         if self._board.get_processor().get_num_cores() > 1:
             warn("SimPoints only work with one core")
         self._board.get_processor().get_cores()[0].set_simpoint(
-            simpoint_start_insts, schedule_at_init
+            simpoint_start_insts, self._instantiated
         )

     def schedule_max_insts(

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/64832?usp=email To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I567a48df06e6327b38673a2c510065d4334657e2
Gerrit-Change-Number: 64832
Gerrit-PatchSet: 6
Gerrit-Owner: Bobby Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Bobby Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Melissa Jost <mkj...@ucdavis.edu>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

Reply via email to