Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/41434 )
Change subject: base: Clean up style in bitfield.(hh|cc|test.cc).
......................................................................
base: Clean up style in bitfield.(hh|cc|test.cc).
Fix pure style issues, convert bit indices to unsigned, and add
constexpr to most functions. constexpr will allow these functions to be
used to calculate constants at compile time like template parameters or
fixed constants.
Change-Id: Ic98a8ff451ca13f0872411ba5b8b27fc42de8093
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/41434
Reviewed-by: Gabe Black <[email protected]>
Maintainer: Gabe Black <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/base/bitfield.cc
M src/base/bitfield.hh
M src/base/bitfield.test.cc
3 files changed, 125 insertions(+), 90 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/base/bitfield.cc b/src/base/bitfield.cc
index 4e51b12..47055df 100644
--- a/src/base/bitfield.cc
+++ b/src/base/bitfield.cc
@@ -38,7 +38,7 @@
#include "base/bitfield.hh"
/** Lookup table used for High Speed bit reversing */
-const uint8_t reverseLookUpTable[] =
+const uint8_t reverseBitsLookUpTable[] =
{
0x00, 0x80, 0x40, 0xC0, 0x20, 0xA0, 0x60, 0xE0, 0x10, 0x90, 0x50, 0xD0,
0x30, 0xB0, 0x70, 0xF0, 0x08, 0x88, 0x48, 0xC8, 0x28, 0xA8, 0x68, 0xE8,
diff --git a/src/base/bitfield.hh b/src/base/bitfield.hh
index 98a93d4..1ec684f 100644
--- a/src/base/bitfield.hh
+++ b/src/base/bitfield.hh
@@ -41,13 +41,12 @@
#ifndef __BASE_BITFIELD_HH__
#define __BASE_BITFIELD_HH__
-#include <inttypes.h>
#include <cassert>
#include <cstddef>
+#include <cstdint>
#include <type_traits>
-/** Lookup table used for High Speed bit reversing */
-extern const uint8_t reverseLookUpTable[];
+extern const uint8_t reverseBitsLookUpTable[];
/**
* Generate a 64-bit mask of 'nbits' 1s, right justified. If a number of
bits
@@ -57,8 +56,8 @@
*
* @ingroup api_bitfield
*/
-inline uint64_t
-mask(int nbits)
+constexpr inline uint64_t
+mask(unsigned nbits)
{
return (nbits >= 64) ? (uint64_t)-1LL : (1ULL << nbits) - 1;
}
@@ -70,12 +69,11 @@
* @ingroup api_bitfield
*/
template <class T>
-inline
-T
-bits(T val, int first, int last)
+constexpr inline T
+bits(T val, unsigned first, unsigned last)
{
+ assert(first >= last);
int nbits = first - last + 1;
- assert((first - last) >= 0);
return (val >> last) & mask(nbits);
}
@@ -85,9 +83,8 @@
* @ingroup api_bitfield
*/
template <class T>
-inline
-T
-bits(T val, int bit)
+constexpr inline T
+bits(T val, unsigned bit)
{
return bits(val, bit, bit);
}
@@ -99,18 +96,17 @@
* @ingroup api_bitfield
*/
template <class T>
-inline
-T
-mbits(T val, int first, int last)
+constexpr inline T
+mbits(T val, unsigned first, unsigned last)
{
- return val & (mask(first+1) & ~mask(last));
+ return val & (mask(first + 1) & ~mask(last));
}
/**
* @ingroup api_bitfield
*/
-inline uint64_t
-mask(int first, int last)
+constexpr inline uint64_t
+mask(unsigned first, unsigned last)
{
return mbits((uint64_t)-1LL, first, last);
}
@@ -121,12 +117,13 @@
* @ingroup api_bitfield
*/
template <int N>
-inline
-uint64_t
+constexpr inline uint64_t
sext(uint64_t val)
{
- int sign_bit = bits(val, N-1, N-1);
- return sign_bit ? (val | ~mask(N)) : val;
+ bool sign_bit = bits(val, N - 1);
+ if (sign_bit)
+ val |= ~mask(N);
+ return val;
}
/**
@@ -142,14 +139,14 @@
* @ingroup api_bitfield
*/
template <class T, class B>
-inline
-T
-insertBits(T val, int first, int last, B bit_val)
+constexpr inline T
+insertBits(T val, unsigned first, unsigned last, B bit_val)
{
- T t_bit_val = bit_val;
- assert((first - last) >= 0);
- T bmask = mask(first - last + 1) << last;
- return ((t_bit_val << last) & bmask) | (val & ~bmask);
+ assert(first >= last);
+ T bmask = mask(first, last);
+ val &= ~bmask;
+ val |= ((T)bit_val << last) & bmask;
+ return val;
}
/**
@@ -158,9 +155,8 @@
* @ingroup api_bitfield
*/
template <class T, class B>
-inline
-T
-insertBits(T val, int bit, B bit_val)
+constexpr inline T
+insertBits(T val, unsigned bit, B bit_val)
{
return insertBits(val, bit, bit, bit_val);
}
@@ -174,9 +170,8 @@
* @ingroup api_bitfield
*/
template <class T, class B>
-inline
-void
-replaceBits(T& val, int first, int last, B bit_val)
+constexpr inline void
+replaceBits(T& val, unsigned first, unsigned last, B bit_val)
{
val = insertBits(val, first, last, bit_val);
}
@@ -187,38 +182,42 @@
* @ingroup api_bitfield
*/
template <class T, class B>
-inline
-void
-replaceBits(T& val, int bit, B bit_val)
+constexpr inline void
+replaceBits(T& val, unsigned bit, B bit_val)
{
val = insertBits(val, bit, bit, bit_val);
}
/**
- * Takes a variable lenght word and returns the mirrored version
- * (Bit by bit, LSB=>MSB).
+ * Takes a value and returns the bit reversed version.
*
- * algorithm from
- * http://graphics.stanford.edu/~seander/bithacks.html
- * #ReverseBitsByLookupTable
+ * E.g.:
+ * val: 0x0303
+ * returned: 0xc0c0
*
- * @param val: variable lenght word
+ * val: 0x0303
+ * size: 1
+ * returned: 0x03c0
+ *
+ * Algorithm from:
+ *
http://graphics.stanford.edu/~seander/bithacks.html#ReverseBitsByLookupTable
+ *
+ * @param val: variable length value
* @param size: number of bytes to mirror
- * @return mirrored word
+ * @return reversed value
*
* @ingroup api_bitfield
*/
template <class T>
-T
-reverseBits(T val, std::size_t size = sizeof(T))
+std::enable_if_t<std::is_integral<T>::value, T>
+reverseBits(T val, size_t size=sizeof(T))
{
- static_assert(std::is_integral<T>::value, "Expecting an integer type");
-
assert(size <= sizeof(T));
- T output = 0;
- for (auto byte = 0; byte < size; byte++, val = static_cast<T>(val >>
8)) {
- output = (output << 8) | reverseLookUpTable[val & 0xFF];
+ T output = {};
+ for (size_t byte = 0; byte < size; byte++) {
+ output = (output << 8) | reverseBitsLookUpTable[val & mask(8)];
+ val >>= 8;
}
return output;
@@ -229,18 +228,34 @@
*
* @ingroup api_bitfield
*/
-inline
-int
-findMsbSet(uint64_t val) {
+constexpr inline int
+findMsbSet(uint64_t val)
+{
int msb = 0;
if (!val)
return 0;
- if (bits(val, 63,32)) { msb += 32; val >>= 32; }
- if (bits(val, 31,16)) { msb += 16; val >>= 16; }
- if (bits(val, 15,8)) { msb += 8; val >>= 8; }
- if (bits(val, 7,4)) { msb += 4; val >>= 4; }
- if (bits(val, 3,2)) { msb += 2; val >>= 2; }
- if (bits(val, 1,1)) { msb += 1; }
+ if (bits(val, 63, 32)) {
+ msb += 32;
+ val >>= 32;
+ }
+ if (bits(val, 31, 16)) {
+ msb += 16;
+ val >>= 16;
+ }
+ if (bits(val, 15, 8)) {
+ msb += 8;
+ val >>= 8;
+ }
+ if (bits(val, 7, 4)) {
+ msb += 4;
+ val >>= 4;
+ }
+ if (bits(val, 3, 2)) {
+ msb += 2;
+ val >>= 2;
+ }
+ if (bits(val, 1, 1))
+ msb += 1;
return msb;
}
@@ -249,17 +264,34 @@
*
* @ingroup api_bitfield
*/
-inline int
-findLsbSet(uint64_t val) {
+constexpr inline int
+findLsbSet(uint64_t val)
+{
int lsb = 0;
if (!val)
return sizeof(val) * 8;
- if (!bits(val, 31,0)) { lsb += 32; val >>= 32; }
- if (!bits(val, 15,0)) { lsb += 16; val >>= 16; }
- if (!bits(val, 7,0)) { lsb += 8; val >>= 8; }
- if (!bits(val, 3,0)) { lsb += 4; val >>= 4; }
- if (!bits(val, 1,0)) { lsb += 2; val >>= 2; }
- if (!bits(val, 0,0)) { lsb += 1; }
+ if (!bits(val, 31, 0)) {
+ lsb += 32;
+ val >>= 32;
+ }
+ if (!bits(val, 15, 0)) {
+ lsb += 16;
+ val >>= 16;
+ }
+ if (!bits(val, 7, 0)) {
+ lsb += 8;
+ val >>= 8;
+ }
+ if (!bits(val, 3, 0)) {
+ lsb += 4;
+ val >>= 4;
+ }
+ if (!bits(val, 1, 0)) {
+ lsb += 2;
+ val >>= 2;
+ }
+ if (!bits(val, 0, 0))
+ lsb += 1;
return lsb;
}
@@ -269,10 +301,7 @@
* @ingroup api_bitfield
*/
template <class T>
-inline bool
-isPow2(T v) {
- return (v & (v - 1)) == (T)0;
-}
+constexpr inline bool isPow2(T v) { return (v & (v - 1)) == (T)0; }
/**
* Returns the number of set ones in the provided value.
@@ -281,24 +310,27 @@
*
* @ingroup api_bitfield
*/
-inline int
-popCount(uint64_t val) {
+constexpr inline int
+popCount(uint64_t val)
+{
#ifndef __has_builtin
- #define __has_builtin(foo) 0
+# define __has_builtin(foo) 0
#endif
-#if defined(__GNUC__) || (defined(__clang__) &&
__has_builtin(__builtin_popcountl))
+#if defined(__GNUC__) || \
+ (defined(__clang__) && __has_builtin(__builtin_popcountl))
return __builtin_popcountl(val);
#else
- const uint64_t m1 = 0x5555555555555555; // ..010101b
- const uint64_t m2 = 0x3333333333333333; // ..110011b
- const uint64_t m4 = 0x0f0f0f0f0f0f0f0f; // ..001111b
- const uint64_t sum = 0x0101010101010101;
+ const uint64_t m1 = 0x5555555555555555ULL; // ..010101b
+ const uint64_t m2 = 0x3333333333333333ULL; // ..110011b
+ const uint64_t m4 = 0x0f0f0f0f0f0f0f0fULL; // ..001111b
+ const uint64_t sum = 0x0101010101010101ULL;
val -= (val >> 1) & m1; // 2 bits count -> 2 bits
val = (val & m2) + ((val >> 2) & m2); // 4 bits count -> 4 bits
val = (val + (val >> 4)) & m4; // 8 bits count -> 8 bits
return (val * sum) >> 56; // horizontal sum
-#endif // defined(__GNUC__) || (defined(__clang__) &&
__has_builtin(__builtin_popcountl))
+#endif // defined(__GNUC__) ||
+ //(defined(__clang__) && __has_builtin(__builtin_popcountl))
}
/**
@@ -313,7 +345,8 @@
*
* @ingroup api_bitfield
*/
-inline uint64_t alignToPowerOfTwo(uint64_t val)
+constexpr inline uint64_t
+alignToPowerOfTwo(uint64_t val)
{
val--;
val |= val >> 1;
@@ -335,7 +368,8 @@
*
* @ingroup api_bitfield
*/
-inline int ctz32(uint32_t value)
+constexpr inline int
+ctz32(uint32_t value)
{
return value ? __builtin_ctzl(value) : 32;
}
@@ -348,7 +382,8 @@
*
* @ingroup api_bitfield
*/
-inline int ctz64(uint64_t value)
+constexpr inline int
+ctz64(uint64_t value)
{
return value ? __builtin_ctzll(value) : 64;
}
diff --git a/src/base/bitfield.test.cc b/src/base/bitfield.test.cc
index 3d07e69..51c316e 100644
--- a/src/base/bitfield.test.cc
+++ b/src/base/bitfield.test.cc
@@ -87,22 +87,22 @@
*/
TEST(BitfieldTest, MaskOneBit)
{
- EXPECT_EQ(1, mask(0,0));
+ EXPECT_EQ(1, mask(0, 0));
}
TEST(BitfieldTest, MaskTwoBits)
{
- EXPECT_EQ((1 << 1) + 1, mask(1,0));
+ EXPECT_EQ((1 << 1) + 1, mask(1, 0));
}
TEST(BitfieldTest, MaskThreeBits)
{
- EXPECT_EQ((1 << 5) + (1 << 4) + (1 << 3), mask(5,3));
+ EXPECT_EQ((1 << 5) + (1 << 4) + (1 << 3), mask(5, 3));
}
TEST(BitfieldTest, MaskEntireRange)
{
- EXPECT_EQ(0xFFFFFFFFFFFFFFFF, mask(63,0));
+ EXPECT_EQ(0xFFFFFFFFFFFFFFFF, mask(63, 0));
}
TEST(BitfieldTest, MaskOutsideOfRange)
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/41434
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: Ic98a8ff451ca13f0872411ba5b8b27fc42de8093
Gerrit-Change-Number: 41434
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Nikos Nikoleris <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-CC: Matt Sinclair <[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
