From cd9450c1d95d9494e2714ec84620c548b0eebbb1 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Sat, 6 Jan 2018 05:30:46 -0800 Subject: 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 Maintainer: Gabe Black --- src/arch/arm/types.hh | 4 +- src/base/bitunion.hh | 348 +++++++++++++++++++++++++---------------------- src/base/bituniontest.cc | 49 ++++++- src/dev/x86/i8042.cc | 17 +-- src/dev/x86/speaker.cc | 7 +- src/sim/serialize.hh | 18 +-- 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 : - public hash { + public hash { size_t operator()(const ArmISA::ExtMachInst &emi) const { - return hash::operator()(emi); + return hash::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 +#include + #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 BitfieldTypeImpl : public Base +{ + static_assert(std::is_empty::value, + "Bitfield base class must be empty."); + + private: + using Base::setter; + + template + struct TypeDeducer; + + template + friend class TypeDeducer; + + template + struct TypeDeducer + { + typedef Type1 Storage; + typedef Type2 Type; + }; + + protected: + typedef typename TypeDeducer< + decltype(&BitfieldTypeImpl::setter)>::Storage Storage; + typedef typename TypeDeducer< + decltype(&BitfieldTypeImpl::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 const & other) + { + return *this = (Type)other; + } +}; + +//A wrapper for the above class which allows setting and getting. +template +class BitfieldType : public BitfieldTypeImpl +{ + protected: + using Impl = BitfieldTypeImpl; + using typename Impl::Type; + + public: + operator Type () const { return Impl::operator Type(); } + Type operator=(const Type val) { return Impl::operator=(val); } + Type + operator=(BitfieldType const & other) + { + return Impl::operator=(other); + } +}; + +//A wrapper which only supports getting. +template +class BitfieldROType : public BitfieldTypeImpl +{ + public: + using Impl = BitfieldTypeImpl; + using typename Impl::Type; + + Type operator=(BitfieldROType const &other) = delete; + operator Type () const { return Impl::operator Type(); } +}; + +//A wrapper which only supports setting. +template +class BitfieldWOType : public BitfieldTypeImpl +{ + protected: + using Impl = BitfieldTypeImpl; + using typename Impl::Type; + + public: + Type operator=(const Type val) { return Impl::operator=(val); } + Type + operator=(BitfieldWOType 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 BitfieldBase + template + class Unsigned { + static_assert(first >= last, + "Bitfield ranges must be specified as "); + 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 RegularBitfieldTypes + template + class Signed { + static_assert(first >= last, + "Bitfield ranges must be specified as "); + protected: - //This class implements ordinary bitfields, that is a span of bits - //who's msb is "first", and who's lsb is "last". - template - class Bitfield : public BitfieldBase + int64_t + getter(const Storage &storage) const { - static_assert(first >= last, - "Bitfield ranges must be specified as "); - - 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 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 - class BitfieldRO : public Bitfield - { - private: - uint64_t - operator=(const uint64_t _data); - - uint64_t - operator=(const Bitfield& other); - }; + return sext(bits(storage, first, last)); + } - //Similar to the above, but only allows writing. - template - class BitfieldWO : public Bitfield + void + setter(Storage &storage, int64_t val) { - private: - operator uint64_t () const; - - public: - using Bitfield::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 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 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 - class SignedBitfield : public BitfieldBase - { - public: - operator int64_t () const - { - return sext(this->getBits(first, last)); - } - - int64_t - operator=(const int64_t _data) - { - this->setBits(first, last, _data); - return _data; - } - - int64_t - operator=(SignedBitfield 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 >; template - class SignedBitfieldRO : public SignedBitfield - { - private: - int64_t - operator=(const int64_t _data); - - int64_t - operator=(const SignedBitfield& other); - }; - - //Similar to the above, but only allows writing. + using BitfieldRO = + BitfieldROType >; template - class SignedBitfieldWO : public SignedBitfield - { - private: - operator int64_t () const; + using BitfieldWO = + BitfieldWOType >; - public: - using SignedBitfield::operator=; - }; + template + using SignedBitfield = + BitfieldType >; + template + using SignedBitfieldRO = + BitfieldROType >; + template + using SignedBitfieldWO = + BitfieldWOType >; }; - template - class BitfieldTypes : public RegularBitfieldTypes, - public SignedBitfieldTypes - {}; - //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 + template 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 \ { \ 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 __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 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 void paramOut(CheckpointOut &cp, const std::string &name, const T ¶m); -template +template void paramOut(CheckpointOut &cp, const std::string &name, - const BitfieldBackend::BitUnionOperators &p) + const BitfieldBackend::BitUnionOperators &p) { - paramOut(cp, name, p.__data); + paramOut(cp, name, p.__storage); } template void paramIn(CheckpointIn &cp, const std::string &name, T ¶m); -template +template void paramIn(CheckpointIn &cp, const std::string &name, - BitfieldBackend::BitUnionOperators &p) + BitfieldBackend::BitUnionOperators &p) { - paramIn(cp, name, p.__data); + paramIn(cp, name, p.__storage); } template bool optParamIn(CheckpointIn &cp, const std::string &name, T ¶m, bool warn = true); -template +template bool optParamIn(CheckpointIn &cp, const std::string &name, - BitfieldBackend::BitUnionOperators &p, + BitfieldBackend::BitUnionOperators &p, bool warn = true) { - return optParamIn(cp, name, p.__data, warn); + return optParamIn(cp, name, p.__storage, warn); } template -- cgit v1.2.3