base: Rework bitunions so they can be more flexible.

They are now oriented around a class which makes it easy to provide
custom setter/getter functions which let you set or read bits in an
arbitrary way.

Future additions may add the ability to add custom bitfield methods,
and index-able bitfields.

Change-Id: Ibd6d4d9e49107490f6dad30a4379a8c93bda9333
Reviewed-on: https://gem5-review.googlesource.com/7201
Reviewed-by: Jason Lowe-Power <jason@lowepower.com>
Maintainer: Gabe Black <gabeblack@google.com>

6 files changed, 250 insertions, 193 deletions

diff --git a/src/arch/arm/types.hh b/src/arch/arm/types.hh
index eb9a9e801..0611232c4 100644
--- a/src/arch/arm/types.hh
+++ b/src/arch/arm/types.hh
@@ -741,10 +741,10 @@ namespace std {
 
 template<>
 struct hash<ArmISA::ExtMachInst> :
-        public hash<ArmISA::ExtMachInst::__DataType> {
+        public hash<ArmISA::ExtMachInst::__StorageType> {
 
     size_t operator()(const ArmISA::ExtMachInst &emi) const {
-        return hash<ArmISA::ExtMachInst::__DataType>::operator()(emi);
+        return hash<ArmISA::ExtMachInst::__StorageType>::operator()(emi);
     }
 
 };
diff --git a/src/base/bitunion.hh b/src/base/bitunion.hh
index 306899563..1715e34d6 100644
--- a/src/base/bitunion.hh
+++ b/src/base/bitunion.hh
@@ -31,6 +31,9 @@
 #ifndef __BASE_BITUNION_HH__
 #define __BASE_BITUNION_HH__
 
+#include <iostream>
+#include <type_traits>
+
 #include "base/bitfield.hh"
 
 //      The following implements the BitUnion system of defining bitfields
@@ -41,210 +44,230 @@
 //without having to have access to each other. More details are provided with
 //the individual components.
 
+//This class wraps around another which defines getter/setter functions which
+//manipulate the underlying data. The type of the underlying data and the type
+//of the bitfield itself are inferred from the argument types of the setter
+//function.
+template<class Base>
+class BitfieldTypeImpl : public Base
+{
+    static_assert(std::is_empty<Base>::value,
+                  "Bitfield base class must be empty.");
+
+  private:
+    using Base::setter;
+
+    template<typename T>
+    struct TypeDeducer;
+
+    template<typename T>
+    friend class TypeDeducer;
+
+    template<typename Type1, typename Type2>
+    struct TypeDeducer<void (Base::*)(Type1 &, Type2)>
+    {
+        typedef Type1 Storage;
+        typedef Type2 Type;
+    };
+
+  protected:
+    typedef typename TypeDeducer<
+            decltype(&BitfieldTypeImpl<Base>::setter)>::Storage Storage;
+    typedef typename TypeDeducer<
+            decltype(&BitfieldTypeImpl<Base>::setter)>::Type Type;
+
+    Type getter(const Storage &storage) const = delete;
+    void setter(Storage &storage, Type val) = delete;
+
+    Storage __storage;
+
+    operator Type () const
+    {
+        return Base::getter(__storage);
+    }
+
+    Type
+    operator=(const Type val)
+    {
+        Base::setter(__storage, val);
+        return val;
+    }
+
+    Type
+    operator=(BitfieldTypeImpl<Base> const & other)
+    {
+        return *this = (Type)other;
+    }
+};
+
+//A wrapper for the above class which allows setting and getting.
+template<class Base>
+class BitfieldType : public BitfieldTypeImpl<Base>
+{
+  protected:
+    using Impl = BitfieldTypeImpl<Base>;
+    using typename Impl::Type;
+
+  public:
+    operator Type () const { return Impl::operator Type(); }
+    Type operator=(const Type val) { return Impl::operator=(val); }
+    Type
+    operator=(BitfieldType<Base> const & other)
+    {
+        return Impl::operator=(other);
+    }
+};
+
+//A wrapper which only supports getting.
+template<class Base>
+class BitfieldROType : public BitfieldTypeImpl<Base>
+{
+  public:
+    using Impl = BitfieldTypeImpl<Base>;
+    using typename Impl::Type;
+
+    Type operator=(BitfieldROType<Base> const &other) = delete;
+    operator Type () const { return Impl::operator Type(); }
+};
+
+//A wrapper which only supports setting.
+template <class Base>
+class BitfieldWOType : public BitfieldTypeImpl<Base>
+{
+  protected:
+    using Impl = BitfieldTypeImpl<Base>;
+    using typename Impl::Type;
+
+  public:
+    Type operator=(const Type val) { return Impl::operator=(val); }
+    Type
+    operator=(BitfieldWOType<Base> const & other)
+    {
+        return Impl::operator=(other);
+    }
+};
+
 //This namespace is for classes which implement the backend of the BitUnion
-//stuff. Don't use any of these directly, except for the Bitfield classes in
-//the *BitfieldTypes class(es).
+//stuff. Don't use any of these directly.
 namespace BitfieldBackend
 {
-    //A base class for all bitfields. It instantiates the actual storage,
-    //and provides getBits and setBits functions for manipulating it. The
-    //Data template parameter is type of the underlying storage.
-    template<class Data>
-    class BitfieldBase
+    template<class Storage, int first, int last>
+    class Unsigned
     {
+        static_assert(first >= last,
+                      "Bitfield ranges must be specified as <msb, lsb>");
+
       protected:
-        Data __data;
-
-        //This function returns a range of bits from the underlying storage.
-        //It relies on the "bits" function above. It's the user's
-        //responsibility to make sure that there is a properly overloaded
-        //version of this function for whatever type they want to overlay.
-        inline uint64_t
-        getBits(int first, int last) const
+        uint64_t
+        getter(const Storage &storage) const
         {
-            return bits(__data, first, last);
+            return bits(storage, first, last);
         }
 
-        //Similar to the above, but for settings bits with replaceBits.
-        inline void
-        setBits(int first, int last, uint64_t val)
+        void
+        setter(Storage &storage, uint64_t val)
         {
-            replaceBits(__data, first, last, val);
+            replaceBits(storage, first, last, val);
         }
     };
 
-    //This class contains all the "regular" bitfield classes. It is inherited
-    //by all BitUnions which give them access to those types.
-    template<class Type>
-    class RegularBitfieldTypes
+    template<class Storage, int first, int last>
+    class Signed
     {
+        static_assert(first >= last,
+                      "Bitfield ranges must be specified as <msb, lsb>");
+
       protected:
-        //This class implements ordinary bitfields, that is a span of bits
-        //who's msb is "first", and who's lsb is "last".
-        template<int first, int last=first>
-        class Bitfield : public BitfieldBase<Type>
+        int64_t
+        getter(const Storage &storage) const
         {
-            static_assert(first >= last,
-                          "Bitfield ranges must be specified as <msb, lsb>");
-
-          public:
-            operator uint64_t () const
-            {
-                return this->getBits(first, last);
-            }
-
-            uint64_t
-            operator=(const uint64_t _data)
-            {
-                this->setBits(first, last, _data);
-                return _data;
-            }
-
-            uint64_t
-            operator=(Bitfield<first, last> const & other)
-            {
-                return *this = (uint64_t)other;
-            }
-        };
-
-        //A class which specializes the above so that it can only be read
-        //from. This is accomplished explicitly making sure the assignment
-        //operator is blocked. The conversion operator is carried through
-        //inheritance. This will unfortunately need to be copied into each
-        //bitfield type due to limitations with how templates work
-        template<int first, int last=first>
-        class BitfieldRO : public Bitfield<first, last>
-        {
-          private:
-            uint64_t
-            operator=(const uint64_t _data);
-
-            uint64_t
-            operator=(const Bitfield<first, last>& other);
-        };
+            return sext<first - last + 1>(bits(storage, first, last));
+        }
 
-        //Similar to the above, but only allows writing.
-        template<int first, int last=first>
-        class BitfieldWO : public Bitfield<first, last>
+        void
+        setter(Storage &storage, int64_t val)
         {
-          private:
-            operator uint64_t () const;
-
-          public:
-            using Bitfield<first, last>::operator=;
-        };
+            replaceBits(storage, first, last, val);
+        }
     };
 
-    //This class contains all the "regular" bitfield classes. It is inherited
-    //by all BitUnions which give them access to those types.
-    template<class Type>
-    class SignedBitfieldTypes
+    //This class contains the basic bitfield types which are automatically
+    //available within a BitUnion. They inherit their Storage type from the
+    //containing BitUnion.
+    template<class Storage>
+    class BitfieldTypes
     {
       protected:
-        //This class implements ordinary bitfields, that is a span of bits
-        //who's msb is "first", and who's lsb is "last".
+
         template<int first, int last=first>
-        class SignedBitfield : public BitfieldBase<Type>
-        {
-          public:
-            operator int64_t () const
-            {
-                return sext<first - last + 1>(this->getBits(first, last));
-            }
-
-            int64_t
-            operator=(const int64_t _data)
-            {
-                this->setBits(first, last, _data);
-                return _data;
-            }
-
-            int64_t
-            operator=(SignedBitfield<first, last> const & other)
-            {
-                return *this = (int64_t)other;
-            }
-        };
-
-        //A class which specializes the above so that it can only be read
-        //from. This is accomplished explicitly making sure the assignment
-        //operator is blocked. The conversion operator is carried through
-        //inheritance. This will unfortunately need to be copied into each
-        //bitfield type due to limitations with how templates work
+        using Bitfield = BitfieldType<Unsigned<Storage, first, last> >;
         template<int first, int last=first>
-        class SignedBitfieldRO : public SignedBitfield<first, last>
-        {
-          private:
-            int64_t
-            operator=(const int64_t _data);
-
-            int64_t
-            operator=(const SignedBitfield<first, last>& other);
-        };
-
-        //Similar to the above, but only allows writing.
+        using BitfieldRO =
+                BitfieldROType<Unsigned<Storage, first, last> >;
         template<int first, int last=first>
-        class SignedBitfieldWO : public SignedBitfield<first, last>
-        {
-          private:
-            operator int64_t () const;
+        using BitfieldWO =
+                BitfieldWOType<Unsigned<Storage, first, last> >;
 
-          public:
-            using SignedBitfield<first, last>::operator=;
-        };
+        template<int first, int last=first>
+        using SignedBitfield =
+                BitfieldType<Signed<Storage, first, last> >;
+        template<int first, int last=first>
+        using SignedBitfieldRO =
+                BitfieldROType<Signed<Storage, first, last> >;
+        template<int first, int last=first>
+        using SignedBitfieldWO =
+                BitfieldWOType<Signed<Storage, first, last> >;
     };
 
-    template<class Type>
-    class BitfieldTypes : public RegularBitfieldTypes<Type>,
-                          public SignedBitfieldTypes<Type>
-    {};
-
     //When a BitUnion is set up, an underlying class is created which holds
     //the actual union. This class then inherits from it, and provids the
     //implementations for various operators. Setting things up this way
     //prevents having to redefine these functions in every different BitUnion
     //type. More operators could be implemented in the future, as the need
     //arises.
-    template <class Type, class Base>
+    template <class Base>
     class BitUnionOperators : public Base
     {
+        static_assert(sizeof(Base) == sizeof(typename Base::__StorageType),
+                      "BitUnion larger than its storage type.");
+
       public:
-        BitUnionOperators(Type const & _data)
+        BitUnionOperators(typename Base::__StorageType const &val)
         {
-            Base::__data = _data;
+            Base::__storage = val;
         }
 
         BitUnionOperators() {}
 
-        operator const Type () const
+        operator const typename Base::__StorageType () const
         {
-            return Base::__data;
+            return Base::__storage;
         }
 
-        Type
-        operator=(Type const & _data)
+        typename Base::__StorageType
+        operator=(typename Base::__StorageType const &val)
         {
-            Base::__data = _data;
-            return _data;
+            Base::__storage = val;
+            return val;
         }
 
-        Type
-        operator=(BitUnionOperators const & other)
+        typename Base::__StorageType
+        operator=(BitUnionOperators const &other)
         {
-            Base::__data = other;
-            return Base::__data;
+            Base::__storage = other;
+            return Base::__storage;
         }
 
         bool
-        operator<(Base const & base) const
+        operator<(Base const &base) const
         {
-            return Base::__data < base.__data;
+            return Base::__storage < base.__storage;
         }
 
         bool
-        operator==(Base const & base) const
+        operator==(Base const &base) const
         {
-            return Base::__data == base.__data;
+            return Base::__storage == base.__storage;
         }
     };
 }
@@ -256,12 +279,12 @@ namespace BitfieldBackend
 //namespace ensures that there will be no collisions with other names as long
 //as the BitUnion names themselves are all distinct and nothing else uses
 //the BitfieldUnderlyingClasses namespace, which is unlikely. The class itself
-//creates a typedef of the "type" parameter called __DataType. This allows
+//creates a typedef of the "type" parameter called __StorageType. This allows
 //the type to propagate outside of the macro itself in a controlled way.
 //Finally, the base storage is defined which BitfieldOperators will refer to
 //in the operators it defines. This macro is intended to be followed by
 //bitfield definitions which will end up inside it's union. As explained
-//above, these is overlayed the __data member in its entirety by each of the
+//above, these is overlayed the __storage member in its entirety by each of the
 //bitfields which are defined in the union, creating shared storage with no
 //overhead.
 #define __BitUnion(type, name) \
@@ -269,9 +292,9 @@ namespace BitfieldBackend
         public BitfieldBackend::BitfieldTypes<type> \
     { \
       public: \
-        typedef type __DataType; \
+        typedef type __StorageType; \
         union { \
-            type __data;\
+            type __storage;
 
 //This closes off the class and union started by the above macro. It is
 //followed by a typedef which makes "name" refer to a BitfieldOperator
@@ -281,20 +304,19 @@ namespace BitfieldBackend
         }; \
     }; \
     typedef BitfieldBackend::BitUnionOperators< \
-        BitfieldUnderlyingClasses##name::__DataType, \
         BitfieldUnderlyingClasses##name> name;
 
 //This sets up a bitfield which has other bitfields nested inside of it. The
-//__data member functions like the "underlying storage" of the top level
+//__storage member functions like the "underlying storage" of the top level
 //BitUnion. Like everything else, it overlays with the top level storage, so
 //making it a regular bitfield type makes the entire thing function as a
 //regular bitfield when referred to by itself.
-#define __SubBitUnion(fieldType, first, last, name) \
-    class : public BitfieldBackend::BitfieldTypes<__DataType> \
+#define __SubBitUnion(name, fieldType, ...) \
+    class \
     { \
       public: \
         union { \
-            fieldType<first, last> __data;
+            fieldType<__VA_ARGS__> __storage;
 
 //This closes off the union created above and gives it a name. Unlike the top
 //level BitUnion, we're interested in creating an object instead of a type.
@@ -303,22 +325,22 @@ namespace BitfieldBackend
 //do so.
 #define EndSubBitUnion(name) \
         }; \
-        inline operator __DataType () const \
-        { return __data; } \
+        inline operator __StorageType () const \
+        { return __storage; } \
         \
-        inline __DataType operator = (const __DataType & _data) \
-        { return __data = _data;} \
+        inline __StorageType operator = (const __StorageType & _storage) \
+        { return __storage = _storage;} \
     } name;
 
 //Regular bitfields
 //These define macros for read/write regular bitfield based subbitfields.
 #define SubBitUnion(name, first, last) \
-    __SubBitUnion(Bitfield, first, last, name)
+    __SubBitUnion(name, Bitfield, first, last)
 
 //Regular bitfields
 //These define macros for read/write regular bitfield based subbitfields.
 #define SignedSubBitUnion(name, first, last) \
-    __SubBitUnion(SignedBitfield, first, last, name)
+    __SubBitUnion(name, SignedBitfield, first, last)
 
 //Use this to define an arbitrary type overlayed with bitfields.
 #define BitUnion(type, name) __BitUnion(type, name)
diff --git a/src/base/bituniontest.cc b/src/base/bituniontest.cc
index 1f24e7e66..ed7b77b66 100644
--- a/src/base/bituniontest.cc
+++ b/src/base/bituniontest.cc
@@ -66,6 +66,44 @@ EndBitUnion(EmptySixteen)
 BitUnion8(EmptyEight)
 EndBitUnion(EmptyEight)
 
+class SplitField
+{
+  protected:
+    BitUnion64(In)
+        Bitfield<15, 12> high;
+        Bitfield<7, 4> low;
+    EndBitUnion(In)
+
+    BitUnion64(Out)
+        Bitfield<7, 4> high;
+        Bitfield<3, 0> low;
+    EndBitUnion(Out)
+  public:
+    uint64_t
+    getter(const uint64_t &storage) const
+    {
+        Out out = 0;
+        In in = storage;
+        out.high = in.high;
+        out.low = in.low;
+        return out;
+    }
+
+    void
+    setter(uint64_t &storage, uint64_t val)
+    {
+        Out out = val;
+        In in = 0;
+        in.high = out.high;
+        in.low = out.low;
+        storage = in;
+    }
+};
+
+BitUnion64(Split)
+    BitfieldType<SplitField> split;
+EndBitUnion(Split)
+
 struct ContainingStruct
 {
     BitUnion64(Contained)
@@ -99,8 +137,9 @@ EmptyEight emptyEight(0);
 class BitUnionData : public testing::Test {
   protected:
     SixtyFour sixtyFour;
+    Split split;
 
-    void SetUp() override { sixtyFour = 0; }
+    void SetUp() override { sixtyFour = 0; split = 0; }
 };
 
 TEST_F(BitUnionData, NormalBitfield)
@@ -192,3 +231,11 @@ TEST_F(BitUnionData, Operators)
     sixtyFour = otherSixtyFour;
     EXPECT_TRUE(sixtyFour == otherSixtyFour);
 }
+
+TEST_F(BitUnionData, Custom)
+{
+    EXPECT_EQ(split, 0);
+    split.split = 0xfff;
+    EXPECT_EQ(split, 0xf0f0);
+    EXPECT_EQ((uint64_t)split.split, 0xff);
+}
diff --git a/src/dev/x86/i8042.cc b/src/dev/x86/i8042.cc
index c5fca1b47..279c520c1 100644
--- a/src/dev/x86/i8042.cc
+++ b/src/dev/x86/i8042.cc
@@ -491,13 +491,10 @@ X86ISA::I8042::write(PacketPtr pkt)
 void
 X86ISA::I8042::serialize(CheckpointOut &cp) const
 {
-    uint8_t statusRegData = statusReg.__data;
-    uint8_t commandByteData = commandByte.__data;
-
     SERIALIZE_SCALAR(dataPort);
     SERIALIZE_SCALAR(commandPort);
-    SERIALIZE_SCALAR(statusRegData);
-    SERIALIZE_SCALAR(commandByteData);
+    SERIALIZE_SCALAR(statusReg);
+    SERIALIZE_SCALAR(commandByte);
     SERIALIZE_SCALAR(dataReg);
     SERIALIZE_SCALAR(lastCommand);
     mouse.serialize("mouse", cp);
@@ -507,20 +504,14 @@ X86ISA::I8042::serialize(CheckpointOut &cp) const
 void
 X86ISA::I8042::unserialize(CheckpointIn &cp)
 {
-    uint8_t statusRegData;
-    uint8_t commandByteData;
-
     UNSERIALIZE_SCALAR(dataPort);
     UNSERIALIZE_SCALAR(commandPort);
-    UNSERIALIZE_SCALAR(statusRegData);
-    UNSERIALIZE_SCALAR(commandByteData);
+    UNSERIALIZE_SCALAR(statusReg);
+    UNSERIALIZE_SCALAR(commandByte);
     UNSERIALIZE_SCALAR(dataReg);
     UNSERIALIZE_SCALAR(lastCommand);
     mouse.unserialize("mouse", cp);
     keyboard.unserialize("keyboard", cp);
-
-    statusReg.__data = statusRegData;
-    commandByte.__data = commandByteData;
 }
 
 void
diff --git a/src/dev/x86/speaker.cc b/src/dev/x86/speaker.cc
index 61a296719..4d39903e2 100644
--- a/src/dev/x86/speaker.cc
+++ b/src/dev/x86/speaker.cc
@@ -77,16 +77,13 @@ X86ISA::Speaker::write(PacketPtr pkt)
 void
 X86ISA::Speaker::serialize(CheckpointOut &cp) const
 {
-    uint8_t controlValData = controlVal.__data;
-    SERIALIZE_SCALAR(controlValData);
+    SERIALIZE_SCALAR(controlVal);
 }
 
 void
 X86ISA::Speaker::unserialize(CheckpointIn &cp)
 {
-    uint8_t controlValData;
-    UNSERIALIZE_SCALAR(controlValData);
-    controlVal.__data = controlValData;
+    UNSERIALIZE_SCALAR(controlVal);
 }
 
 X86ISA::Speaker *
diff --git a/src/sim/serialize.hh b/src/sim/serialize.hh
index 1005d5b97..b6c026394 100644
--- a/src/sim/serialize.hh
+++ b/src/sim/serialize.hh
@@ -72,33 +72,33 @@ typedef std::ostream CheckpointOut;
 template <class T>
 void paramOut(CheckpointOut &cp, const std::string &name, const T &param);
 
-template <typename DataType, typename BitUnion>
+template <typename BitUnion>
 void paramOut(CheckpointOut &cp, const std::string &name,
-              const BitfieldBackend::BitUnionOperators<DataType, BitUnion> &p)
+              const BitfieldBackend::BitUnionOperators<BitUnion> &p)
 {
-    paramOut(cp, name, p.__data);
+    paramOut(cp, name, p.__storage);
 }
 
 template <class T>
 void paramIn(CheckpointIn &cp, const std::string &name, T &param);
 
-template <typename DataType, typename BitUnion>
+template <typename BitUnion>
 void paramIn(CheckpointIn &cp, const std::string &name,
-             BitfieldBackend::BitUnionOperators<DataType, BitUnion> &p)
+             BitfieldBackend::BitUnionOperators<BitUnion> &p)
 {
-    paramIn(cp, name, p.__data);
+    paramIn(cp, name, p.__storage);
 }
 
 template <class T>
 bool optParamIn(CheckpointIn &cp, const std::string &name, T &param,
                 bool warn = true);
 
-template <typename DataType, typename BitUnion>
+template <typename BitUnion>
 bool optParamIn(CheckpointIn &cp, const std::string &name,
-                BitfieldBackend::BitUnionOperators<DataType, BitUnion> &p,
+                BitfieldBackend::BitUnionOperators<BitUnion> &p,
                 bool warn = true)
 {
-    return optParamIn(cp, name, p.__data, warn);
+    return optParamIn(cp, name, p.__storage, warn);
 }
 
 template <class T>