Bobby R. Bruce has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/51790 )

Change subject: stdlib: Remove SimpleBoard as a superclass
......................................................................

stdlib: Remove SimpleBoard as a superclass

Previously SimpleBoard inherited from AbstractBoard and X86Board and
RiscvBoard inherited from the SimpleBoard. This has been shown to be a
needless level of abstraction. As such, this commit refactors the code
to have X86Board and RiscvBoard inherit directly from AbstractBoard.
Code common to the SimpleBoard, X86Board, and RiscvBoard has been moved
to the AbstractBoard.

Change-Id: I5a2c7404efeb4f8ddcb5d8006e3c163d10b88b2c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/51790
Reviewed-by: Bobby R. Bruce <[email protected]>
Maintainer: Bobby R. Bruce <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/python/gem5/components/boards/simple_board.py
M src/python/gem5/components/boards/riscv_board.py
M src/python/gem5/components/boards/test_board.py
M src/python/gem5/components/boards/x86_board.py
M src/python/gem5/components/boards/abstract_board.py
5 files changed, 96 insertions(+), 63 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/python/gem5/components/boards/abstract_board.py b/src/python/gem5/components/boards/abstract_board.py
index 165b0d9..845ccb2 100644
--- a/src/python/gem5/components/boards/abstract_board.py
+++ b/src/python/gem5/components/boards/abstract_board.py
@@ -26,9 +26,16 @@

 from abc import ABCMeta, abstractmethod

-from .mem_mode import MemMode
+from .mem_mode import MemMode, mem_mode_to_string

-from m5.objects import System, Port, IOXBar, ClockDomain
+from m5.objects import (
+    System,
+    Port,
+    IOXBar,
+    ClockDomain,
+    SrcClockDomain,
+    VoltageDomain,
+)

 from typing import List

@@ -56,17 +63,31 @@

     def __init__(
         self,
+        clk_freq: str,
         processor: "AbstractProcessor",
         memory: "AbstractMemory",
         cache_hierarchy: "AbstractCacheHierarchy",
+        exit_on_work_items: bool = False,
     ) -> None:
         super(AbstractBoard, self).__init__()
         """
+        :param clk_freq: The clock frequency for this board.
         :param processor: The processor for this board.
         :param memory: The memory for this board.
         :param cache_hierarchy: The Cachie Hierarchy for this board.
+        :param exit_on_work_items: Whether the simulation should exit
+        on work items.
         """

+        # Set up the clock domain and the voltage domain.
+        self.clk_domain = SrcClockDomain()
+        self.clk_domain.clock = clk_freq
+        self.clk_domain.voltage_domain = VoltageDomain()
+
+        # Set whether to exit on work items.
+        self.exit_on_work_items = exit_on_work_items
+
+        # Set the processor, memory, and cache hierarchy.
         self.processor = processor
         self.memory = memory
         self.cache_hierarchy = cache_hierarchy
@@ -101,6 +122,24 @@
         """
         return self.cache_line_size

+    def connect_system_port(self, port: Port) -> None:
+        self.system_port = port
+
+    def set_mem_mode(self, mem_mode: MemMode) -> None:
+        """
+        Set the memory mode of the board.
+
+        :param mem_mode: The memory mode the board is to be set to.
+        """
+        self.mem_mode = mem_mode_to_string(mem_mode=mem_mode)
+
+    def get_clock_domain(self) -> ClockDomain:
+        """Get the clock domain.
+
+        :returns: The clock domain.
+        """
+        return self.clk_domain
+
# Technically `get_dma_ports` returns a list. This list could be empty to
     # indicate the presense of dma ports. Though I quite like having this
     # boolean to quickly check a board.
@@ -165,27 +204,6 @@
         raise NotImplementedError

     @abstractmethod
-    def get_clock_domain(self) -> ClockDomain:
-        """Get the clock domain.
-
-        :returns: The clock domain.
-        """
-        raise NotImplementedError
-
-    @abstractmethod
-    def connect_system_port(self, port: Port) -> None:
-        raise NotImplementedError
-
-    @abstractmethod
-    def set_mem_mode(self, mem_mode: MemMode) -> None:
-        """
-        Set the memory mode of the board.
-
-        :param mem_mode: The memory mode the board is to be set to.
-        """
-        raise NotImplementedError
-
-    @abstractmethod
     def setup_memory_ranges(self) -> None:
         """
         Set the memory ranges for this board.
diff --git a/src/python/gem5/components/boards/riscv_board.py b/src/python/gem5/components/boards/riscv_board.py
index d42cfd5..43a5112 100644
--- a/src/python/gem5/components/boards/riscv_board.py
+++ b/src/python/gem5/components/boards/riscv_board.py
@@ -25,10 +25,9 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

 import os
-from typing import Optional
+from typing import Optional, List

 from ...utils.override import overrides
-from .simple_board import SimpleBoard
 from .abstract_board import AbstractBoard
 from ..processors.abstract_processor import AbstractProcessor
 from ..memory.abstract_memory_system import AbstractMemorySystem
@@ -66,7 +65,7 @@
 )


-class RiscvBoard(SimpleBoard):
+class RiscvBoard(AbstractBoard):
     """
     A board capable of full system simulation for RISC-V

@@ -84,8 +83,11 @@
         processor: AbstractProcessor,
         memory: AbstractMemorySystem,
         cache_hierarchy: AbstractCacheHierarchy,
+        exit_on_work_items: bool = False,
     ) -> None:
-        super().__init__(clk_freq, processor, memory, cache_hierarchy)
+        super().__init__(
+ clk_freq, processor, memory, cache_hierarchy, exit_on_work_items
+        )

         requires(isa_required=ISA.RISCV)

@@ -157,6 +159,17 @@
             )

     @overrides(AbstractBoard)
+    def has_dma_ports(self) -> bool:
+        return False
+
+    @overrides(AbstractBoard)
+    def get_dma_ports(self) -> List[Port]:
+        raise NotImplementedError(
+            "RISCVBoard does not have DMA Ports. "
+            "Use `has_dma_ports()` to check this."
+        )
+
+    @overrides(AbstractBoard)
     def has_io_bus(self) -> bool:
         return True

@@ -177,6 +190,20 @@
         self.mem_ranges = [AddrRange(start=0x80000000, size=mem_size)]
         memory.set_memory_range(self.mem_ranges)

+    @overrides(AbstractBoard)
+    def connect_things(self) -> None:
+        # Before incorporating the memory, set up the memory ranges
+        self.setup_memory_ranges()
+
+        # Incorporate the cache hierarchy for the motherboard.
+        self.get_cache_hierarchy().incorporate_cache(self)
+
+        # Incorporate the processor into the motherboard.
+        self.get_processor().incorporate_processor(self)
+
+        # Incorporate the memory into the motherboard.
+        self.get_memory().incorporate_memory(self)
+
     def set_workload(
         self, bootloader: AbstractResource, disk_image: AbstractResource,
         command: Optional[str] = None
diff --git a/src/python/gem5/components/boards/simple_board.py b/src/python/gem5/components/boards/simple_board.py
index 62c2d33..efe1736 100644
--- a/src/python/gem5/components/boards/simple_board.py
+++ b/src/python/gem5/components/boards/simple_board.py
@@ -37,7 +37,6 @@
 )

 from .abstract_board import AbstractBoard
-from .mem_mode import MemMode, mem_mode_to_string
 from ..processors.abstract_processor import AbstractProcessor
 from ..memory.abstract_memory_system import AbstractMemorySystem
from ..cachehierarchies.abstract_cache_hierarchy import AbstractCacheHierarchy
@@ -66,30 +65,13 @@
         exit_on_work_items: bool = False,
     ) -> None:
         super(SimpleBoard, self).__init__(
+            clk_freq=clk_freq,
             processor=processor,
             memory=memory,
             cache_hierarchy=cache_hierarchy,
+            exit_on_work_items=exit_on_work_items,
         )

-        # Set up the clock domain and the voltage domain.
-        self.clk_domain = SrcClockDomain()
-        self.clk_domain.clock = clk_freq
-        self.clk_domain.voltage_domain = VoltageDomain()
-
-        self.exit_on_work_items = exit_on_work_items
-
-    @overrides(AbstractBoard)
-    def get_clock_domain(self) -> ClockDomain:
-        return self.clk_domain
-
-    @overrides(AbstractBoard)
-    def connect_system_port(self, port: Port) -> None:
-        self.system_port = port
-
-    @overrides(AbstractBoard)
-    def set_mem_mode(self, mem_mode: MemMode) -> None:
-        self.mem_mode = mem_mode_to_string(mem_mode=mem_mode)
-
     @overrides(AbstractBoard)
     def connect_things(self) -> None:
         # Incorporate the cache hierarchy for the motherboard.
diff --git a/src/python/gem5/components/boards/test_board.py b/src/python/gem5/components/boards/test_board.py
index ea8efa8..861b5a8 100644
--- a/src/python/gem5/components/boards/test_board.py
+++ b/src/python/gem5/components/boards/test_board.py
@@ -60,16 +60,11 @@
         cache_hierarchy: AbstractCacheHierarchy,
     ):
         super(TestBoard, self).__init__(
+            clk_freq=clk_freq,
             processor=processor,
             memory=memory,
             cache_hierarchy=cache_hierarchy,
         )
-        self.clk_domain = SrcClockDomain(
-            clock=clk_freq, voltage_domain=VoltageDomain()
-        )
-
-    def connect_system_port(self, port: Port) -> None:
-        self.system_port = port

     def connect_things(self) -> None:
         self.get_processor().incorporate_processor(self)
@@ -78,9 +73,6 @@

         self.get_cache_hierarchy().incorporate_cache(self)

-    def get_clock_domain(self) -> ClockDomain:
-        return self.clk_domain
-
     @overrides(AbstractBoard)
     def has_io_bus(self) -> bool:
         return False
@@ -115,10 +107,6 @@
         )

     @overrides(AbstractBoard)
-    def set_mem_mode(self, mem_mode: MemMode) -> None:
-        self.mem_mode = mem_mode_to_string(mem_mode=mem_mode)
-
-    @overrides(AbstractBoard)
     def setup_memory_ranges(self) -> None:
         memory = self.get_memory()

diff --git a/src/python/gem5/components/boards/x86_board.py b/src/python/gem5/components/boards/x86_board.py
index df7e7fb..c6ce459 100644
--- a/src/python/gem5/components/boards/x86_board.py
+++ b/src/python/gem5/components/boards/x86_board.py
@@ -55,8 +55,6 @@

 from m5.util.convert import toMemorySize

-
-from .simple_board import SimpleBoard
 from ..processors.abstract_processor import AbstractProcessor
 from ..memory.abstract_memory_system import AbstractMemorySystem
from ..cachehierarchies.abstract_cache_hierarchy import AbstractCacheHierarchy
@@ -66,7 +64,7 @@
 from typing import List, Optional, Sequence


-class X86Board(SimpleBoard):
+class X86Board(AbstractBoard):
     """
     A board capable of full system simulation for X86.


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/51790
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: I5a2c7404efeb4f8ddcb5d8006e3c163d10b88b2c
Gerrit-Change-Number: 51790
Gerrit-PatchSet: 11
Gerrit-Owner: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to