From 38c8a4efb245aec4bfd34c28e00c73b140c08489 Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Sat, 12 Jul 2014 14:35:47 +0200 Subject: [PATCH] MMIO: Cleanup Mapping class by using templates instead of macros. --- Source/Core/Core/HW/MMIO.cpp | 15 +- Source/Core/Core/HW/MMIO.h | 159 +++++++++++------- Source/Core/Core/HW/MemmapFunctions.cpp | 2 +- .../Core/Core/PowerPC/JitCommon/Jit_Util.cpp | 6 +- Source/UnitTests/Core/MMIOTest.cpp | 14 +- 5 files changed, 115 insertions(+), 81 deletions(-) diff --git a/Source/Core/Core/HW/MMIO.cpp b/Source/Core/Core/HW/MMIO.cpp index d1b37e5c54..e1cd422f57 100644 --- a/Source/Core/Core/HW/MMIO.cpp +++ b/Source/Core/Core/HW/MMIO.cpp @@ -224,10 +224,8 @@ ReadHandlingMethod* ReadToSmaller(Mapping* mmio, u32 high_part_addr, u32 low_ { typedef typename SmallerAccessSize::value ST; - ReadHandler* high_part; - ReadHandler* low_part; - mmio->GetHandlerForRead(high_part_addr, &high_part); - mmio->GetHandlerForRead(low_part_addr, &low_part); + ReadHandler* high_part = &mmio->GetHandlerForRead(high_part_addr); + ReadHandler* low_part = &mmio->GetHandlerForRead(low_part_addr); // TODO(delroth): optimize return ComplexRead([=](u32 addr) { @@ -241,10 +239,8 @@ WriteHandlingMethod* WriteToSmaller(Mapping* mmio, u32 high_part_addr, u32 lo { typedef typename SmallerAccessSize::value ST; - WriteHandler* high_part; - WriteHandler* low_part; - mmio->GetHandlerForWrite(high_part_addr, &high_part); - mmio->GetHandlerForWrite(low_part_addr, &low_part); + WriteHandler* high_part = &mmio->GetHandlerForWrite(high_part_addr); + WriteHandler* low_part = &mmio->GetHandlerForWrite(low_part_addr); // TODO(delroth): optimize return ComplexWrite([=](u32 addr, T val) { @@ -258,8 +254,7 @@ ReadHandlingMethod* ReadToLarger(Mapping* mmio, u32 larger_addr, u32 shift) { typedef typename LargerAccessSize::value LT; - ReadHandler* large; - mmio->GetHandlerForRead(larger_addr, &large); + ReadHandler* large = &mmio->GetHandlerForRead(larger_addr); // TODO(delroth): optimize return ComplexRead([large, shift](u32 addr) { diff --git a/Source/Core/Core/HW/MMIO.h b/Source/Core/Core/HW/MMIO.h index 721689fb54..79577aafe9 100644 --- a/Source/Core/Core/HW/MMIO.h +++ b/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) \ - void RegisterRead(u32 addr, ReadHandlingMethod* read) \ - { \ - u32 id = UniqueID(addr) / sizeof (u##Size); \ - m_Read##Size##Handlers[id].ResetMethod(read); \ - } \ - void RegisterWrite(u32 addr, WriteHandlingMethod* write) \ - { \ - u32 id = UniqueID(addr) / sizeof (u##Size); \ - m_Write##Size##Handlers[id].ResetMethod(write); \ - } \ - void Register(u32 addr, ReadHandlingMethod* read, \ - WriteHandlingMethod* write) \ - { \ - RegisterRead(addr, read); \ - RegisterWrite(addr, write); \ + template + void RegisterRead(u32 addr, ReadHandlingMethod* read) + { + GetHandlerForRead(addr).ResetMethod(read); + } + + template + void RegisterWrite(u32 addr, WriteHandlingMethod* write) + { + GetHandlerForWrite(addr).ResetMethod(write); + } + + template + void Register(u32 addr, ReadHandlingMethod* read, WriteHandlingMethod* 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. - // - // Note that for reads we cannot simply return the read value because C++ - // allows overloading only with parameter types, not return types. -#define READ_FUNC(Size) \ - void Read(u32 addr, u##Size* val) \ - { \ - u32 id = UniqueID(addr) / sizeof (u##Size); \ - *val = m_Read##Size##Handlers[id].Read(addr); \ + template + Unit Read(u32 addr) + { + return GetHandlerForRead(addr).Read(addr); } - READ_FUNC(8) READ_FUNC(16) READ_FUNC(32) -#undef READ_FUNC -#define WRITE_FUNC(Size) \ - void Write(u32 addr, u##Size val) \ - { \ - u32 id = UniqueID(addr) / sizeof (u##Size); \ - m_Write##Size##Handlers[id].Write(addr, val); \ + template + void Write(u32 addr, Unit val) + { + GetHandlerForWrite(addr).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. - // - // Two variants of each GetHandler function are provided: one that returns - // 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 - // based on return type but only based on argument types. -#define GET_HANDLERS_FUNC(Type, Size) \ - Type##Handler& GetHandlerFor##Type##Size(u32 addr) \ - { \ - return m_##Type##Size##Handlers[UniqueID(addr) / sizeof (u##Size)]; \ - } \ - void GetHandlerFor##Type(u32 addr, Type##Handler** h) \ - { \ - *h = &GetHandlerFor##Type##Size(addr); \ + template + ReadHandler& GetHandlerForRead(u32 addr) + { + return GetReadHandler(UniqueID(addr) / sizeof(Unit)); } - 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 - // not supported, we need these in order to make the code compile. - void Read(u32 addr, u64* val) { _dbg_assert_(MEMMAP, 0); } - void Write(u32 addr, u64 val) { _dbg_assert_(MEMMAP, 0); } + template + WriteHandler& GetHandlerForWrite(u32 addr) + { + return GetWriteHandler(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) \ - std::array, NUM_MMIOS / sizeof (u##Size)> m_Read##Size##Handlers; \ - std::array, NUM_MMIOS / sizeof (u##Size)> m_Write##Size##Handlers; - HANDLERS(8) HANDLERS(16) HANDLERS(32) -#undef HANDLERS + template + struct HandlerArray + { + using Read = std::array, NUM_MMIOS / sizeof(Unit)>; + using Write = std::array, NUM_MMIOS / sizeof(Unit)>; + }; + + HandlerArray::Read m_read_handlers8; + HandlerArray::Read m_read_handlers16; + HandlerArray::Read m_read_handlers32; + + HandlerArray::Write m_write_handlers8; + HandlerArray::Write m_write_handlers16; + HandlerArray::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(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 + ReadHandler& GetReadHandler(size_t index) + { + static_assert(std::is_same::value || std::is_same::value || std::is_same::value, "Invalid unit used"); + using ArrayType = typename HandlerArray::Read; + ArrayType& handler = *(std::is_same::value ? (ArrayType*)&m_read_handlers8 + : std::is_same::value ? (ArrayType*)&m_read_handlers16 + : (ArrayType*)&m_read_handlers32); + return handler[index]; + } + + template + WriteHandler& GetWriteHandler(size_t index) + { + static_assert(std::is_same::value || std::is_same::value || std::is_same::value, "Invalid unit used"); + using ArrayType = typename HandlerArray::Write; + ArrayType& handler = *(std::is_same::value ? (ArrayType*)&m_write_handlers8 + : std::is_same::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(u32 addr) +{ + _dbg_assert_(MEMMAP, 0); + return 0; +} + +template<> +inline void Mapping::Write(u32 addr, u64 val) +{ + _dbg_assert_(MEMMAP, 0); +} + } diff --git a/Source/Core/Core/HW/MemmapFunctions.cpp b/Source/Core/Core/HW/MemmapFunctions.cpp index 1a7c86a6d6..c06a8cb9cb 100644 --- a/Source/Core/Core/HW/MemmapFunctions.cpp +++ b/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(em_address); } else if (((em_address & 0xF0000000) == 0x80000000) || ((em_address & 0xF0000000) == 0xC0000000) || diff --git a/Source/Core/Core/PowerPC/JitCommon/Jit_Util.cpp b/Source/Core/Core/PowerPC/JitCommon/Jit_Util.cpp index 3c8eababe1..18710d06d2 100644 --- a/Source/Core/Core/PowerPC/JitCommon/Jit_Util.cpp +++ b/Source/Core/Core/PowerPC/JitCommon/Jit_Util.cpp @@ -253,21 +253,21 @@ void EmuCodeBlock::MMIOLoadToReg(MMIO::Mapping* mmio, Gen::X64Reg reg_value, { MMIOReadCodeGenerator gen(this, registers_in_use, reg_value, address, sign_extend); - mmio->GetHandlerForRead8(address).Visit(gen); + mmio->GetHandlerForRead(address).Visit(gen); break; } case 16: { MMIOReadCodeGenerator gen(this, registers_in_use, reg_value, address, sign_extend); - mmio->GetHandlerForRead16(address).Visit(gen); + mmio->GetHandlerForRead(address).Visit(gen); break; } case 32: { MMIOReadCodeGenerator gen(this, registers_in_use, reg_value, address, sign_extend); - mmio->GetHandlerForRead32(address).Visit(gen); + mmio->GetHandlerForRead(address).Visit(gen); break; } } diff --git a/Source/UnitTests/Core/MMIOTest.cpp b/Source/UnitTests/Core/MMIOTest.cpp index 64b226ce5d..f9173ce0c4 100644 --- a/Source/UnitTests/Core/MMIOTest.cpp +++ b/Source/UnitTests/Core/MMIOTest.cpp @@ -54,9 +54,9 @@ TEST_F(MappingTest, ReadConstant) m_mapping->Register(0xCC001234, MMIO::Constant(0x1234), MMIO::Nop()); m_mapping->Register(0xCC001234, MMIO::Constant(0xdeadbeef), MMIO::Nop()); - u8 val8; m_mapping->Read(0xCC001234, &val8); - u16 val16; m_mapping->Read(0xCC001234, &val16); - u32 val32; m_mapping->Read(0xCC001234, &val32); + u8 val8 = m_mapping->Read(0xCC001234); + u16 val16 = m_mapping->Read(0xCC001234); + u32 val32 = m_mapping->Read(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); - u16 val16; m_mapping->Read(0xCC001234, &val16); EXPECT_EQ(i, val16); - u32 val32; m_mapping->Read(0xCC001234, &val32); EXPECT_EQ(i, val32); + u8 val8 = m_mapping->Read(0xCC001234); EXPECT_EQ(i, val8); + u16 val16 = m_mapping->Read(0xCC001234); EXPECT_EQ(i, val16); + u32 val32 = m_mapping->Read(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(0xCC001234); EXPECT_EQ(0x12, val); m_mapping->Write(0xCC001234, (u8)0x34); EXPECT_TRUE(read_called);