Merge pull request #1470 from TomHarte/SimplerReverse

Unify and simplify bit reversal functions.

Numeric/BitReverse.hpp

@@ -8,52 +8,54 @@
 
 #pragma once
 
+#include <array>
 #include <cstdint>
 
 namespace Numeric {
+namespace {
+
+constexpr auto reverse_map = [] {
+	std::array<uint8_t, 256> table{};
+	for(std::size_t c = 0; c < 256; ++c) {
+		table[c] = uint8_t(
+			((c & 0x80) >> 7) |
+			((c & 0x40) >> 5) |
+			((c & 0x20) >> 3) |
+			((c & 0x10) >> 1) |
+			((c & 0x08) << 1) |
+			((c & 0x04) << 3) |
+			((c & 0x02) << 5) |
+			((c & 0x01) << 7)
+		);
+	}
+	return table;
+} ();
+
+}
 
 /// @returns @c source with the order of its bits reversed. E.g. if @c IntT is @c uint8_t then
 /// the reverse of bit pattern abcd efgh is hgfd dcba.
 template <typename IntT> constexpr IntT bit_reverse(IntT source);
 
 // The single-byte specialisation uses a lookup table.
-template<> constexpr uint8_t bit_reverse<uint8_t>(uint8_t source) {
-	struct ReverseTable {
-		static constexpr std::array<uint8_t, 256> reverse_table() {
-			std::array<uint8_t, 256> map{};
-			for(std::size_t c = 0; c < 256; ++c) {
-				map[c] = uint8_t(
-					((c & 0x80) >> 7) |
-					((c & 0x40) >> 5) |
-					((c & 0x20) >> 3) |
-					((c & 0x10) >> 1) |
-					((c & 0x08) << 1) |
-					((c & 0x04) << 3) |
-					((c & 0x02) << 5) |
-					((c & 0x01) << 7)
-				);
-			}
-			return map;
-		}
-	};
-
-	const std::array<uint8_t, 256> map = ReverseTable::reverse_table();
-	return map[source];
+template<> constexpr uint8_t bit_reverse<uint8_t>(const uint8_t source) {
+	return reverse_map[source];
 }
 
-// All other versions just call the byte-level reverse the appropriate number of times.
-template <typename IntT> constexpr IntT bit_reverse(IntT source) {
-	IntT result;
-
-	uint8_t *src = reinterpret_cast<uint8_t *>(&source);
-	uint8_t *dest = reinterpret_cast<uint8_t *>(&result) + sizeof(result) - 1;
-	for(size_t c = 0; c < sizeof(source); c++) {
-		*dest = bit_reverse(*src);
-		++src;
-		--dest;
-	}
-
-	return result;
+// All other versions recursively subdivide.
+template <typename IntT>
+constexpr IntT bit_reverse(const IntT source) {
+	static_assert(std::is_same_v<IntT, uint16_t> || std::is_same_v<IntT, uint32_t> || std::is_same_v<IntT, uint64_t>);
+	using HalfIntT =
+		std::conditional_t<std::is_same_v<IntT, uint16_t>, uint8_t,
+			std::conditional_t<std::is_same_v<IntT, uint32_t>, uint16_t,
+				uint32_t>>;
+	constexpr auto HalfShift = sizeof(IntT) * 4;
+
+	return IntT(
+		IntT(bit_reverse(HalfIntT(source))) << HalfShift) |
+		IntT(bit_reverse(HalfIntT(source >> HalfShift))
+	);
 }
 
 }

Numeric/CRC.hpp

@@ -8,6 +8,7 @@
 
 #pragma once
 
+#include "BitReverse.hpp"
 #include "Carry.hpp"
 
 #include <array>
@@ -16,18 +17,6 @@
 
 namespace CRC {
 
-constexpr uint8_t reverse_byte(uint8_t byte) {
-	return
-		((byte & 0x80) ? 0x01 : 0x00) |
-		((byte & 0x40) ? 0x02 : 0x00) |
-		((byte & 0x20) ? 0x04 : 0x00) |
-		((byte & 0x10) ? 0x08 : 0x00) |
-		((byte & 0x08) ? 0x10 : 0x00) |
-		((byte & 0x04) ? 0x20 : 0x00) |
-		((byte & 0x02) ? 0x40 : 0x00) |
-		((byte & 0x01) ? 0x80 : 0x00);
-}
-
 /*! Provides a class capable of generating a CRC from source data. */
 template <
 	typename IntType,
@@ -64,26 +53,22 @@ class Generator {
 			return table;
 		} ();
 
-		if constexpr (reflect_input) byte = reverse_byte(byte);
+		if constexpr (reflect_input) byte = Numeric::bit_reverse(byte);
 		value_ = IntType((value_ << 8) ^ xor_table[(value_ >> multibyte_shift) ^ byte]);
 	}
 
 	/// @returns The current value of the CRC.
-	inline IntType get_value() const {
-		IntType result = value_ ^ output_xor;
+	IntType get_value() const {
+		const IntType result = value_ ^ output_xor;
 		if constexpr (reflect_output) {
-			IntType reflected_output = 0;
-			for(std::size_t c = 0; c < sizeof(IntType); ++c) {
-				reflected_output = IntType(reflected_output << 8) | IntType(reverse_byte(result & 0xff));
-				result >>= 8;
-			}
-			return reflected_output;
+			return Numeric::bit_reverse(result);
+		} else {
+			return result;
 		}
-		return result;
 	}
 
 	/// Sets the current value of the CRC.
-	inline void set_value(IntType value) { value_ = value; }
+	void set_value(const IntType value) { value_ = value; }
 
 	/*!
 		A compound for:
@@ -103,7 +88,7 @@ class Generator {
 			[add all data from @c begin to @c end]
 			get_value()
 	*/
-	template <typename Iterator> IntType compute_crc(Iterator begin, Iterator end) {
+	template <typename Iterator> IntType compute_crc(Iterator begin, const Iterator end) {
 		reset();
 		while(begin != end) {
 			add(*begin);

OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme

@@ -23,7 +23,7 @@
       </BuildActionEntries>
    </BuildAction>
    <TestAction
-      buildConfiguration = "Release"
+      buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
       shouldUseLaunchSchemeArgsEnv = "YES"

OSBindings/Mac/Clock SignalTests/Bridges/MOS6522Bridge.mm

@@ -29,11 +29,13 @@ void set_interrupt_status(bool new_status) {
 			irq_line = new_status;
 		}
 
-		uint8_t get_port_input(MOS::MOS6522::Port port) {
+		template<MOS::MOS6522::Port port>
+		uint8_t get_port_input() {
 			return port ? port_b_value : port_a_value;
 		}
 
-		void set_control_line_output(MOS::MOS6522::Port port, MOS::MOS6522::Line line, bool value) {
+		template<MOS::MOS6522::Port port, MOS::MOS6522::Line line>
+		void set_control_line_output(bool value) {
 			control_line_values[int(port)][int(line)] = value;
 		}
 };

Storage/Tape/Parsers/Spectrum.cpp

@@ -8,7 +8,7 @@
 
 #include "Spectrum.hpp"
 
-#include "../../../Numeric/CRC.hpp"
+#include "../../../Numeric/BitReverse.hpp"
 
 #include <cstring>
 
@@ -212,7 +212,7 @@ std::optional<uint8_t> Parser::get_byte(Storage::Tape::TapeSerialiser &serialise
 	}
 
 	if(should_flip_bytes()) {
-		result = CRC::reverse_byte(result);
+		result = Numeric::bit_reverse(result);
 	}
 
 	checksum_ ^= result;