MMIO: Cleanup Mapping class by using templates instead of macros.

Source/Core/Core/HW/MMIO.cpp

@@ -224,10 +224,8 @@ ReadHandlingMethod<T>* ReadToSmaller(Mapping* mmio, u32 high_part_addr, u32 low_
 {
 	typedef typename SmallerAccessSize<T>::value ST;
 
-	ReadHandler<ST>* high_part;
+	ReadHandler<ST>* high_part = &mmio->GetHandlerForRead<ST>(high_part_addr);
-	ReadHandler<ST>* low_part;
+	ReadHandler<ST>* low_part = &mmio->GetHandlerForRead<ST>(low_part_addr);
-	mmio->GetHandlerForRead(high_part_addr, &high_part);
-	mmio->GetHandlerForRead(low_part_addr, &low_part);
 
 	// TODO(delroth): optimize
 	return ComplexRead<T>([=](u32 addr) {
@@ -241,10 +239,8 @@ WriteHandlingMethod<T>* WriteToSmaller(Mapping* mmio, u32 high_part_addr, u32 lo
 {
 	typedef typename SmallerAccessSize<T>::value ST;
 
-	WriteHandler<ST>* high_part;
+	WriteHandler<ST>* high_part = &mmio->GetHandlerForWrite<ST>(high_part_addr);
-	WriteHandler<ST>* low_part;
+	WriteHandler<ST>* low_part = &mmio->GetHandlerForWrite<ST>(low_part_addr);
-	mmio->GetHandlerForWrite(high_part_addr, &high_part);
-	mmio->GetHandlerForWrite(low_part_addr, &low_part);
 
 	// TODO(delroth): optimize
 	return ComplexWrite<T>([=](u32 addr, T val) {
@@ -258,8 +254,7 @@ ReadHandlingMethod<T>* ReadToLarger(Mapping* mmio, u32 larger_addr, u32 shift)
 {
 	typedef typename LargerAccessSize<T>::value LT;
 
-	ReadHandler<LT>* large;
+	ReadHandler<LT>* large = &mmio->GetHandlerForRead<LT>(larger_addr);
-	mmio->GetHandlerForRead(larger_addr, &large);
 
 	// TODO(delroth): optimize
 	return ComplexRead<T>([large, shift](u32 addr) {

Source/Core/Core/HW/MMIO.h

@@ -75,25 +75,24 @@ public:
 	//
 	// Example usages can be found in just about any HW/ module in Dolphin's
 	// codebase.
-#define REGISTER_FUNCS(Size) \
+	template<typename Unit>
-	void RegisterRead(u32 addr, ReadHandlingMethod<u##Size>* read) \
+	void RegisterRead(u32 addr, ReadHandlingMethod<Unit>* read)
-	{ \
+	{
-		u32 id = UniqueID(addr) / sizeof (u##Size); \
+		GetHandlerForRead<Unit>(addr).ResetMethod(read);
-		m_Read##Size##Handlers[id].ResetMethod(read); \
+	}
-	} \
+
-	void RegisterWrite(u32 addr, WriteHandlingMethod<u##Size>* write) \
+	template<typename Unit>
-	{ \
+	void RegisterWrite(u32 addr, WriteHandlingMethod<Unit>* write)
-		u32 id = UniqueID(addr) / sizeof (u##Size); \
+	{
-		m_Write##Size##Handlers[id].ResetMethod(write); \
+		GetHandlerForWrite<Unit>(addr).ResetMethod(write);
-	} \
+	}
-	void Register(u32 addr, ReadHandlingMethod<u##Size>* read, \
+
-	              WriteHandlingMethod<u##Size>* write) \
+	template<typename Unit>
-	{ \
+	void Register(u32 addr, ReadHandlingMethod<Unit>* read, WriteHandlingMethod<Unit>* write)
-		RegisterRead(addr, read); \
+	{
-		RegisterWrite(addr, write); \
+		RegisterRead(addr, read);
+		RegisterWrite(addr, write);
 	}
-	REGISTER_FUNCS(8) REGISTER_FUNCS(16) REGISTER_FUNCS(32)
-#undef REGISTER_FUNCS
 
 	// Direct read/write interface.
 	//
@@ -101,54 +100,34 @@ public:
 	// address. They are used by the Memory:: access functions, which are
 	// called in interpreter mode, from Dolphin's own code, or from JIT'd code
 	// where the access address could not be predicted.
-	//
+	template<typename Unit>
-	// Note that for reads we cannot simply return the read value because C++
+	Unit Read(u32 addr)
-	// allows overloading only with parameter types, not return types.
+	{
-#define READ_FUNC(Size) \
+		return GetHandlerForRead<Unit>(addr).Read(addr);
-	void Read(u32 addr, u##Size* val) \
-	{ \
-		u32 id = UniqueID(addr) / sizeof (u##Size); \
-		*val = m_Read##Size##Handlers[id].Read(addr); \
 	}
-	READ_FUNC(8) READ_FUNC(16) READ_FUNC(32)
-#undef READ_FUNC
 
-#define WRITE_FUNC(Size) \
+	template<typename Unit>
-	void Write(u32 addr, u##Size val) \
+	void Write(u32 addr, Unit val)
-	{ \
+	{
-		u32 id = UniqueID(addr) / sizeof (u##Size); \
+		GetHandlerForWrite<Unit>(addr).Write(addr, val);
-		m_Write##Size##Handlers[id].Write(addr, val); \
 	}
-	WRITE_FUNC(8) WRITE_FUNC(16) WRITE_FUNC(32)
-#undef WRITE_FUNC
 
 	// Handlers access interface.
 	//
 	// Use when you care more about how to access the MMIO register for an
 	// address than the current value of that register. For example, this is
 	// what could be used to implement fast MMIO accesses in Dolphin's JIT.
-	//
+	template<typename Unit>
-	// Two variants of each GetHandler function are provided: one that returns
+	ReadHandler<Unit>& GetHandlerForRead(u32 addr)
-	// the handler directly and one that has a pointer parameter to return the
+	{
-	// value. This second variant is needed because C++ doesn't do overloads
+		return GetReadHandler<Unit>(UniqueID(addr) / sizeof(Unit));
-	// based on return type but only based on argument types.
-#define GET_HANDLERS_FUNC(Type, Size) \
-	Type##Handler<u##Size>& GetHandlerFor##Type##Size(u32 addr) \
-	{ \
-		return m_##Type##Size##Handlers[UniqueID(addr) / sizeof (u##Size)]; \
-	} \
-	void GetHandlerFor##Type(u32 addr, Type##Handler<u##Size>** h) \
-	{ \
-		*h = &GetHandlerFor##Type##Size(addr); \
 	}
-	GET_HANDLERS_FUNC(Read, 8) GET_HANDLERS_FUNC(Read, 16) GET_HANDLERS_FUNC(Read, 32)
-	GET_HANDLERS_FUNC(Write, 8) GET_HANDLERS_FUNC(Write, 16) GET_HANDLERS_FUNC(Write, 32)
-#undef GET_HANDLERS_FUNC
 
-	// Dummy 64 bits variants of these functions. While 64 bits MMIO access is
+	template<typename Unit>
-	// not supported, we need these in order to make the code compile.
+	WriteHandler<Unit>& GetHandlerForWrite(u32 addr)
-	void Read(u32 addr, u64* val) { _dbg_assert_(MEMMAP, 0); }
+	{
-	void Write(u32 addr, u64 val) { _dbg_assert_(MEMMAP, 0); }
+		return GetWriteHandler<Unit>(UniqueID(addr) / sizeof(Unit));
+	}
 
 private:
 	// These arrays contain the handlers for each MMIO access type: read/write
@@ -159,11 +138,71 @@ private:
 	// Each array contains NUM_MMIOS / sizeof (AccessType) because larger
 	// access types mean less possible adresses (assuming aligned only
 	// accesses).
-#define HANDLERS(Size) \
+	template<typename Unit>
-	std::array<ReadHandler<u##Size>, NUM_MMIOS / sizeof (u##Size)> m_Read##Size##Handlers; \
+	struct HandlerArray
-	std::array<WriteHandler<u##Size>, NUM_MMIOS / sizeof (u##Size)> m_Write##Size##Handlers;
+	{
-	HANDLERS(8) HANDLERS(16) HANDLERS(32)
+		using Read = std::array<ReadHandler<Unit>, NUM_MMIOS / sizeof(Unit)>;
-#undef HANDLERS
+		using Write = std::array<WriteHandler<Unit>, NUM_MMIOS / sizeof(Unit)>;
+	};
+
+	HandlerArray<u8>::Read m_read_handlers8;
+	HandlerArray<u16>::Read m_read_handlers16;
+	HandlerArray<u32>::Read m_read_handlers32;
+
+	HandlerArray<u8>::Write m_write_handlers8;
+	HandlerArray<u16>::Write m_write_handlers16;
+	HandlerArray<u32>::Write m_write_handlers32;
+
+	// Getter functions for the handler arrays.
+	//
+	// TODO:
+	// It would be desirable to clean these methods up using tuples, i.e. doing something like
+	//
+	//     auto handlers = std::tie(m_read_handlers8, m_read_handlers16, m_read_handlers32);
+	//     return std::get<Unit>(handlers)[index];
+	//
+	// However, we cannot use this currently because of a compiler bug in clang, presumably related
+	// to http://llvm.org/bugs/show_bug.cgi?id=18345, due to which the above code makes the compiler
+	// exceed the template recursion depth.
+	// As a workaround, we cast all handlers to the requested one's type. This cast will
+	// compile to a NOP for the returned member variable, but it's necessary to get this
+	// code to compile at all.
+	template<typename Unit>
+	ReadHandler<Unit>& GetReadHandler(size_t index)
+	{
+		static_assert(std::is_same<Unit, u8>::value || std::is_same<Unit, u16>::value || std::is_same<Unit, u32>::value, "Invalid unit used");
+		using ArrayType = typename HandlerArray<Unit>::Read;
+		ArrayType& handler = *(std::is_same<Unit,u8>::value ? (ArrayType*)&m_read_handlers8
+		                                                    : std::is_same<Unit,u16>::value ? (ArrayType*)&m_read_handlers16
+		                                                                                    : (ArrayType*)&m_read_handlers32);
+		return handler[index];
+	}
+
+	template<typename Unit>
+	WriteHandler<Unit>& GetWriteHandler(size_t index)
+	{
+		static_assert(std::is_same<Unit, u8>::value || std::is_same<Unit, u16>::value || std::is_same<Unit, u32>::value, "Invalid unit used");
+		using ArrayType = typename HandlerArray<Unit>::Write;
+		ArrayType& handler = *(std::is_same<Unit,u8>::value ? (ArrayType*)&m_write_handlers8
+		                                                    : std::is_same<Unit,u16>::value ? (ArrayType*)&m_write_handlers16
+		                                                                                    : (ArrayType*)&m_write_handlers32);
+		return handler[index];
+	}
 };
 
+// Dummy 64 bits variants of these functions. While 64 bits MMIO access is
+// not supported, we need these in order to make the code compile.
+template<>
+inline u64 Mapping::Read<u64>(u32 addr)
+{
+    _dbg_assert_(MEMMAP, 0);
+    return 0;
+}
+
+template<>
+inline void Mapping::Write(u32 addr, u64 val)
+{
+    _dbg_assert_(MEMMAP, 0);
+}
+
 }

Source/Core/Core/HW/MemmapFunctions.cpp

@@ -92,7 +92,7 @@ inline void ReadFromHardware(T &_var, const u32 em_address, const u32 effective_
 		if (em_address < 0xcc000000)
 			_var = EFB_Read(em_address);
 		else
-			mmio_mapping->Read(em_address, &_var);
+			_var = mmio_mapping->Read<T>(em_address);
 	}
 	else if (((em_address & 0xF0000000) == 0x80000000) ||
 		((em_address & 0xF0000000) == 0xC0000000) ||

Source/Core/Core/PowerPC/JitCommon/Jit_Util.cpp

@@ -253,21 +253,21 @@ void EmuCodeBlock::MMIOLoadToReg(MMIO::Mapping* mmio, Gen::X64Reg reg_value,
 		{
 			MMIOReadCodeGenerator<u8> gen(this, registers_in_use, reg_value,
 			                              address, sign_extend);
-			mmio->GetHandlerForRead8(address).Visit(gen);
+			mmio->GetHandlerForRead<u8>(address).Visit(gen);
 			break;
 		}
 	case 16:
 		{
 			MMIOReadCodeGenerator<u16> gen(this, registers_in_use, reg_value,
 			                               address, sign_extend);
-			mmio->GetHandlerForRead16(address).Visit(gen);
+			mmio->GetHandlerForRead<u16>(address).Visit(gen);
 			break;
 		}
 	case 32:
 		{
 			MMIOReadCodeGenerator<u32> gen(this, registers_in_use, reg_value,
 			                               address, sign_extend);
-			mmio->GetHandlerForRead32(address).Visit(gen);
+			mmio->GetHandlerForRead<u32>(address).Visit(gen);
 			break;
 		}
 	}

Source/UnitTests/Core/MMIOTest.cpp

@@ -54,9 +54,9 @@ TEST_F(MappingTest, ReadConstant)
 	m_mapping->Register(0xCC001234, MMIO::Constant<u16>(0x1234), MMIO::Nop<u16>());
 	m_mapping->Register(0xCC001234, MMIO::Constant<u32>(0xdeadbeef), MMIO::Nop<u32>());
 
-	u8 val8;   m_mapping->Read(0xCC001234, &val8);
+	u8 val8 = m_mapping->Read<u8>(0xCC001234);
-	u16 val16; m_mapping->Read(0xCC001234, &val16);
+	u16 val16 = m_mapping->Read<u16>(0xCC001234);
-	u32 val32; m_mapping->Read(0xCC001234, &val32);
+	u32 val32 = m_mapping->Read<u32>(0xCC001234);
 
 	EXPECT_EQ(0x42, val8);
 	EXPECT_EQ(0x1234, val16);
@@ -75,9 +75,9 @@ TEST_F(MappingTest, ReadWriteDirect)
 
 	for (u32 i = 0; i < 100; ++i)
 	{
-		u8 val8;   m_mapping->Read(0xCC001234, &val8);  EXPECT_EQ(i, val8);
+		u8 val8 = m_mapping->Read<u8>(0xCC001234);  EXPECT_EQ(i, val8);
-		u16 val16; m_mapping->Read(0xCC001234, &val16); EXPECT_EQ(i, val16);
+		u16 val16 = m_mapping->Read<u16>(0xCC001234); EXPECT_EQ(i, val16);
-		u32 val32; m_mapping->Read(0xCC001234, &val32); EXPECT_EQ(i, val32);
+		u32 val32 = m_mapping->Read<u32>(0xCC001234); EXPECT_EQ(i, val32);
 
 		val8 += 1; m_mapping->Write(0xCC001234, val8);
 		val16 += 1; m_mapping->Write(0xCC001234, val16);
@@ -102,7 +102,7 @@ TEST_F(MappingTest, ReadWriteComplex)
 		})
 	);
 
-	u8 val; m_mapping->Read(0xCC001234, &val); EXPECT_EQ(0x12, val);
+	u8 val = m_mapping->Read<u8>(0xCC001234); EXPECT_EQ(0x12, val);
 	m_mapping->Write(0xCC001234, (u8)0x34);
 
 	EXPECT_TRUE(read_called);