Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/67203?usp=email )

Change subject: dev: Add a "resetter" callback to the typed register class.
......................................................................

dev: Add a "resetter" callback to the typed register class.

When using the typed register template, most functionality of the class
can be controlled using callbacks. For instance, callbacks can be
installed to handle reads or writes to a register without having to
subclass the template and override those methods using inheritance.

The recently added reset() method did not follow this pattern though,
which has two problems. First, it's inconsistent with how the class is
normally used. Second, once you've defined a subclass, the reader,
writer, etc, callbacks still expect the type of the original class.
That means these have to either awkwardly use a type different from the
actual real type of the register, or use awkward, inefficient, and/or
dangerous casting to get back to the true type.

To address these problems, this change adds a resetter(...) method
which works like the reader(...) or writer(...) methods to optionally
install a callback to implement any special reset behavior.

Change-Id: Ia74b36616fd459c1dbed9304568903a76a4b55de
---
M src/dev/reg_bank.hh
M src/dev/reg_bank.test.cc
2 files changed, 116 insertions(+), 2 deletions(-)



diff --git a/src/dev/reg_bank.hh b/src/dev/reg_bank.hh
index 32d9058..dd68554 100644
--- a/src/dev/reg_bank.hh
+++ b/src/dev/reg_bank.hh
@@ -270,6 +270,12 @@
* is an alternative form of update which also takes a custom bitmask, if you
  * need to update bits other than the normally writeable ones.
  *
+ * Similarly, you can set a "resetter" handler which is responsible for
+ * resetting the register. It takes a reference to the current Register, and + * no other parameters. The "initialValue" accessor can retrieve the value the
+ * register was constructed with. The register is simply set to this value
+ * in the default resetter implementation.
+ *
  * = Read only bits =
  *
  * Often registers have bits which are fixed and not affected by writes. To
@@ -547,13 +553,14 @@
       protected:
         using This = Register<Data, RegByteOrder>;

-      public:
+      public:&
         using ReadFunc = std::function<Data (This &reg)>;
         using PartialReadFunc = std::function<
             Data (This &reg, int first, int last)>;
using WriteFunc = std::function<void (This &reg, const Data &value)>;
         using PartialWriteFunc = std::function<
             void (This &reg, const Data &value, int first, int last)>;
+        using ResetFunc = std::function<void (This &reg)>;

       private:
         Data _data = {};
@@ -564,6 +571,7 @@
         WriteFunc _writer = defaultWriter;
         PartialWriteFunc _partialWriter = defaultPartialWriter;
         PartialReadFunc _partialReader = defaultPartialReader;
+        ResetFunc _resetter = defaultResetter;

       protected:
         static Data defaultReader(This &reg) { return reg.get(); }
@@ -587,6 +595,12 @@
                                                  mask(first, last)));
         }

+        static void
+        defaultResetter(This &reg)
+        {
+            reg.get() = reg.initialValue();
+        }
+
         constexpr Data
         htoreg(Data data)
         {
@@ -721,6 +735,30 @@
             return partialWriter(wrapper);
         }

+        // Set the callables which handle resetting.
+        //
+        // The default resetter restores the initial value used in the
+        // constructor.
+        constexpr This &
+        resetter(const ResetFunc &new_resetter)
+        {
+            _resetter = new_resetter;
+            return *this;
+        }
+        template <class Parent, class... Args>
+        constexpr This &
+        resetter(Parent *parent, void (Parent::*nr)(Args... args))
+        {
+            auto wrapper = [parent, nr](Args&&... args) {
+                return (parent->*nr)(std::forward<Args>(args)...);
+            };
+            return resetter(wrapper);
+        }
+
+        // An accessor which returns the initial value as set in the
+        // constructor. This is intended to be used in a resetter function.
+        const Data &initialValue() const { return _resetData; }
+

         /*
          * Interface for accessing the register's state, for use by the
@@ -817,7 +855,7 @@
         }

         // Reset our data to its initial value.
-        void reset() override { get() = _resetData; }
+        void reset() override { _resetter(*this); }
     };

   private:
diff --git a/src/dev/reg_bank.test.cc b/src/dev/reg_bank.test.cc
index b4bc969..4439526 100644
--- a/src/dev/reg_bank.test.cc
+++ b/src/dev/reg_bank.test.cc
@@ -868,6 +868,56 @@
     EXPECT_EQ(write_value, 0x0344);
 }

+// Use the default resetter for a register.
+TEST_F(TypedRegisterTest, DefaultResetter)
+{
+    BackingType initial_value = reg.get();
+
+    reg.get() = initial_value + 1;
+    EXPECT_EQ(reg.get(), initial_value + 1);
+
+    reg.reset();
+
+    EXPECT_EQ(reg.get(), initial_value);
+}
+
+// Set a custom resetter for a register.
+TEST_F(TypedRegisterTest, Resetter)
+{
+    RegisterBankLE::Register<BackingType> *reg_ptr = nullptr;
+
+    reg.resetter([&reg_ptr](auto &r) {
+        reg_ptr = &r;
+    });
+
+    reg.reset();
+
+    EXPECT_EQ(reg_ptr, &reg);
+}
+
+// Set a custom resetter for a register which is a class method.
+TEST_F(TypedRegisterTest, ResetterMF)
+{
+    using Reg = RegisterBankLE::Register<BackingType>;
+
+    struct ResetStruct
+    {
+        Reg *reg_ptr = nullptr;
+
+        void
+        resetter(Reg &r)
+        {
+            reg_ptr = &r;
+        }
+    } reset_struct;
+
+    reg.resetter(&reset_struct, &ResetStruct::resetter);
+
+    reg.reset();
+
+    EXPECT_EQ(reset_struct.reg_ptr, &reg);
+}
+
 TEST_F(TypedRegisterTest, Serialize)
 {
     std::ostringstream os;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/67203?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: Ia74b36616fd459c1dbed9304568903a76a4b55de
Gerrit-Change-Number: 67203
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-CC: Gabe Black <gabebl...@google.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

Reply via email to