diff --git a/Source/Core/Common/BitUtils.h b/Source/Core/Common/BitUtils.h index eacf6bf099..8d5398fb39 100644 --- a/Source/Core/Common/BitUtils.h +++ b/Source/Core/Common/BitUtils.h @@ -99,4 +99,26 @@ constexpr Result ExtractBits(const T src) noexcept return ExtractBits(src, begin, end); } + +/// +/// Verifies whether the supplied value is a valid bit mask of the form 0b00...0011...11. +/// Both edge cases of all zeros and all ones are considered valid masks, too. +/// +/// @param mask The mask value to test for validity. +/// +/// @tparam T The type of the value. +/// +/// @return A bool indicating whether the mask is valid. +/// +template +constexpr bool IsValidLowMask(const T mask) noexcept +{ + static_assert(std::is_integral::value, "Mask must be an integral type."); + static_assert(std::is_unsigned::value, "Signed masks can introduce hard to find bugs."); + + // Can be efficiently determined without looping or bit counting. It's the counterpart + // to https://graphics.stanford.edu/~seander/bithacks.html#DetermineIfPowerOf2 + // and doesn't require special casing either edge case. + return (mask & (mask + 1)) == 0; +} } // namespace Common diff --git a/Source/Core/Core/PowerPC/MMU.cpp b/Source/Core/Core/PowerPC/MMU.cpp index b5e2448617..ad218b4244 100644 --- a/Source/Core/Core/PowerPC/MMU.cpp +++ b/Source/Core/Core/PowerPC/MMU.cpp @@ -7,7 +7,7 @@ #include #include "Common/Atomic.h" -#include "Common/BitSet.h" +#include "Common/BitUtils.h" #include "Common/CommonTypes.h" #include "Core/ConfigManager.h" @@ -947,26 +947,17 @@ static void GenerateISIException(u32 _EffectiveAddress) void SDRUpdated() { u32 htabmask = SDR1_HTABMASK(PowerPC::ppcState.spr[SPR_SDR]); - u32 x = 1; - u32 xx = 0; - int n = 0; - while ((htabmask & x) && (n < 9)) - { - n++; - xx |= x; - x <<= 1; - } - if (htabmask & ~xx) + if (!Common::IsValidLowMask(htabmask)) { return; } u32 htaborg = SDR1_HTABORG(PowerPC::ppcState.spr[SPR_SDR]); - if (htaborg & xx) + if (htaborg & htabmask) { return; } PowerPC::ppcState.pagetable_base = htaborg << 16; - PowerPC::ppcState.pagetable_hashmask = ((xx << 10) | 0x3ff); + PowerPC::ppcState.pagetable_hashmask = ((htabmask << 10) | 0x3ff); } enum TLBLookupResult @@ -1175,7 +1166,7 @@ static void UpdateBATs(BatTable& bat_table, u32 base_spr) // implemented this way for invalid BATs as well. WARN_LOG(POWERPC, "Bad BAT setup: BPRN overlaps BL"); } - if (CountSetBits((u32)(batu.BL + 1)) != 1) + if (!Common::IsValidLowMask((u32)batu.BL)) { // With a valid BAT, the simplest way of masking is // (input & ~BL_mask) for matching and (input & BL_mask) for diff --git a/Source/UnitTests/Common/BitUtilsTest.cpp b/Source/UnitTests/Common/BitUtilsTest.cpp index 987868e6e4..75f7adf50e 100644 --- a/Source/UnitTests/Common/BitUtilsTest.cpp +++ b/Source/UnitTests/Common/BitUtilsTest.cpp @@ -57,3 +57,35 @@ TEST(BitUtils, ExtractBits) // Ensure bit extraction with type overriding works as expected EXPECT_EQ((Common::ExtractBits<0, 31, s32, s32>(negative_one)), -1); } + +TEST(BitUtils, IsValidLowMask) +{ + EXPECT_TRUE(Common::IsValidLowMask(0b0u)); + EXPECT_TRUE(Common::IsValidLowMask(0b1u)); + EXPECT_FALSE(Common::IsValidLowMask(0b10u)); + EXPECT_TRUE(Common::IsValidLowMask(0b11u)); + EXPECT_FALSE(Common::IsValidLowMask(0b1110u)); + EXPECT_TRUE(Common::IsValidLowMask(0b1111u)); + EXPECT_FALSE(Common::IsValidLowMask(0b10000u)); + EXPECT_FALSE(Common::IsValidLowMask(0b101111u)); + + EXPECT_TRUE(Common::IsValidLowMask((u8)~0b0)); + EXPECT_FALSE(Common::IsValidLowMask((u8)(~0b0 - 1))); + EXPECT_FALSE(Common::IsValidLowMask((u8)~(0b10000))); + EXPECT_FALSE(Common::IsValidLowMask((u8)(~((u8)(~0b0) >> 1) | 0b1111))); + + EXPECT_TRUE(Common::IsValidLowMask((u16)~0b0)); + EXPECT_FALSE(Common::IsValidLowMask((u16)(~0b0 - 1))); + EXPECT_FALSE(Common::IsValidLowMask((u16)~(0b10000))); + EXPECT_FALSE(Common::IsValidLowMask((u16)(~((u16)(~0b0) >> 1) | 0b1111))); + + EXPECT_TRUE(Common::IsValidLowMask((u32)~0b0)); + EXPECT_FALSE(Common::IsValidLowMask((u32)(~0b0 - 1))); + EXPECT_FALSE(Common::IsValidLowMask((u32)~(0b10000))); + EXPECT_FALSE(Common::IsValidLowMask((u32)(~((u32)(~0b0) >> 1) | 0b1111))); + + EXPECT_TRUE(Common::IsValidLowMask((u64)~0b0)); + EXPECT_FALSE(Common::IsValidLowMask((u64)(~0b0 - 1))); + EXPECT_FALSE(Common::IsValidLowMask((u64)~(0b10000))); + EXPECT_FALSE(Common::IsValidLowMask((u64)(~((u64)(~0b0) >> 1) | 0b1111))); +}