From 57744806c30974da7237948c1a2eaaac0bb05a5d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 9 May 2019 18:59:47 -0400 Subject: [PATCH 1/5] video_core/memory_manager: Remove superfluous const from function declarations These are able to be omitted from the declaration of functions, since they don't do anything at the type system level. The definitions of the functions can retain the use of const though, since they make the variables immutable in the implementation of the function where they're used. --- src/video_core/memory_manager.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/video_core/memory_manager.h b/src/video_core/memory_manager.h index e4f0c4bd6e..b9a683497d 100644 --- a/src/video_core/memory_manager.h +++ b/src/video_core/memory_manager.h @@ -66,7 +66,7 @@ public: const u8* GetPointer(GPUVAddr addr) const; // Returns true if the block is continous in host memory, false otherwise - bool IsBlockContinous(const GPUVAddr start, const std::size_t size); + bool IsBlockContinous(GPUVAddr start, std::size_t size); /** * ReadBlock and WriteBlock are full read and write operations over virtual @@ -74,9 +74,9 @@ public: * in the Host Memory counterpart. Note: This functions cause Host GPU Memory * Flushes and Invalidations, respectively to each operation. */ - void ReadBlock(GPUVAddr src_addr, void* dest_buffer, const std::size_t size) const; - void WriteBlock(GPUVAddr dest_addr, const void* src_buffer, const std::size_t size); - void CopyBlock(GPUVAddr dest_addr, GPUVAddr src_addr, const std::size_t size); + void ReadBlock(GPUVAddr src_addr, void* dest_buffer, std::size_t size) const; + void WriteBlock(GPUVAddr dest_addr, const void* src_buffer, std::size_t size); + void CopyBlock(GPUVAddr dest_addr, GPUVAddr src_addr, std::size_t size); /** * ReadBlockUnsafe and WriteBlockUnsafe are special versions of ReadBlock and @@ -88,9 +88,9 @@ public: * WriteBlockUnsafe instead of WriteBlock since it shouldn't invalidate the texture * being flushed. */ - void ReadBlockUnsafe(GPUVAddr src_addr, void* dest_buffer, const std::size_t size) const; - void WriteBlockUnsafe(GPUVAddr dest_addr, const void* src_buffer, const std::size_t size); - void CopyBlockUnsafe(GPUVAddr dest_addr, GPUVAddr src_addr, const std::size_t size); + void ReadBlockUnsafe(GPUVAddr src_addr, void* dest_buffer, std::size_t size) const; + void WriteBlockUnsafe(GPUVAddr dest_addr, const void* src_buffer, std::size_t size); + void CopyBlockUnsafe(GPUVAddr dest_addr, GPUVAddr src_addr, std::size_t size); private: using VMAMap = std::map; From cf685d3e437799d7933fac9966b54614db019774 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 9 May 2019 19:02:52 -0400 Subject: [PATCH 2/5] video_core/memory_manager: Amend doxygen comments Corrects references to non-existent parameters and corrects typos. --- src/video_core/memory_manager.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/video_core/memory_manager.h b/src/video_core/memory_manager.h index b9a683497d..ade18c139d 100644 --- a/src/video_core/memory_manager.h +++ b/src/video_core/memory_manager.h @@ -65,12 +65,12 @@ public: u8* GetPointer(GPUVAddr addr); const u8* GetPointer(GPUVAddr addr) const; - // Returns true if the block is continous in host memory, false otherwise + // Returns true if the block is continuous in host memory, false otherwise bool IsBlockContinous(GPUVAddr start, std::size_t size); /** * ReadBlock and WriteBlock are full read and write operations over virtual - * GPU Memory. It's important to use these when GPU memory may not be continous + * GPU Memory. It's important to use these when GPU memory may not be continuous * in the Host Memory counterpart. Note: This functions cause Host GPU Memory * Flushes and Invalidations, respectively to each operation. */ @@ -111,10 +111,10 @@ private: /** * Maps an unmanaged host memory pointer at a given address. * - * @param target The guest address to start the mapping at. - * @param memory The memory to be mapped. - * @param size Size of the mapping. - * @param state MemoryState tag to attach to the VMA. + * @param target The guest address to start the mapping at. + * @param memory The memory to be mapped. + * @param size Size of the mapping in bytes. + * @param backing_addr The base address of the range to back this mapping. */ VMAHandle MapBackingMemory(GPUVAddr target, u8* memory, u64 size, VAddr backing_addr); @@ -124,7 +124,7 @@ private: /// Converts a VMAHandle to a mutable VMAIter. VMAIter StripIterConstness(const VMAHandle& iter); - /// Marks as the specfied VMA as allocated. + /// Marks as the specified VMA as allocated. VMAIter Allocate(VMAIter vma); /** From cb867f250ae447ca371d30bb193405445674aac2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 9 May 2019 19:04:41 -0400 Subject: [PATCH 3/5] video_core/memory_manager: Default the destructor within the cpp file Makes the class less surprising when it comes to forward declaring the type, and also prevents inlining the destruction code of the class, given it contains non-trivial types. --- src/video_core/memory_manager.cpp | 2 ++ src/video_core/memory_manager.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/video_core/memory_manager.cpp b/src/video_core/memory_manager.cpp index 6c98c67012..2890ea947e 100644 --- a/src/video_core/memory_manager.cpp +++ b/src/video_core/memory_manager.cpp @@ -25,6 +25,8 @@ MemoryManager::MemoryManager(VideoCore::RasterizerInterface& rasterizer) : raste UpdatePageTableForVMA(initial_vma); } +MemoryManager::~MemoryManager() = default; + GPUVAddr MemoryManager::AllocateSpace(u64 size, u64 align) { const u64 aligned_size{Common::AlignUp(size, page_size)}; const GPUVAddr gpu_addr{FindFreeRegion(address_space_base, aligned_size)}; diff --git a/src/video_core/memory_manager.h b/src/video_core/memory_manager.h index ade18c139d..4f7b57f8e4 100644 --- a/src/video_core/memory_manager.h +++ b/src/video_core/memory_manager.h @@ -48,6 +48,7 @@ struct VirtualMemoryArea { class MemoryManager final { public: MemoryManager(VideoCore::RasterizerInterface& rasterizer); + ~MemoryManager(); GPUVAddr AllocateSpace(u64 size, u64 align); GPUVAddr AllocateSpace(GPUVAddr addr, u64 size, u64 align); From 24e9c43cf152e9224f3469e2cb01fd4f81bccfd2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 9 May 2019 19:05:50 -0400 Subject: [PATCH 4/5] video_core/memory_manager: Mark the constructor as explicit Prevents implicit converting constructions of the memory manager. --- src/video_core/memory_manager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/video_core/memory_manager.h b/src/video_core/memory_manager.h index 4f7b57f8e4..f69a257065 100644 --- a/src/video_core/memory_manager.h +++ b/src/video_core/memory_manager.h @@ -47,7 +47,7 @@ struct VirtualMemoryArea { class MemoryManager final { public: - MemoryManager(VideoCore::RasterizerInterface& rasterizer); + explicit MemoryManager(VideoCore::RasterizerInterface& rasterizer); ~MemoryManager(); GPUVAddr AllocateSpace(u64 size, u64 align); From 542ab1b1b98809543ff7922f07b4196f8fe86d37 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 9 May 2019 19:13:14 -0400 Subject: [PATCH 5/5] video_core/memory_manager: Mark IsBlockContinuous() as a const member function Corrects the typo in its name and marks the function as a const member function, given it doesn't actually modify memory manager state. --- src/video_core/memory_manager.cpp | 4 ++-- src/video_core/memory_manager.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/video_core/memory_manager.cpp b/src/video_core/memory_manager.cpp index 2890ea947e..5d8d126c18 100644 --- a/src/video_core/memory_manager.cpp +++ b/src/video_core/memory_manager.cpp @@ -201,11 +201,11 @@ const u8* MemoryManager::GetPointer(GPUVAddr addr) const { return {}; } -bool MemoryManager::IsBlockContinous(const GPUVAddr start, const std::size_t size) { +bool MemoryManager::IsBlockContinuous(const GPUVAddr start, const std::size_t size) const { const GPUVAddr end = start + size; const auto host_ptr_start = reinterpret_cast(GetPointer(start)); const auto host_ptr_end = reinterpret_cast(GetPointer(end)); - const std::size_t range = static_cast(host_ptr_end - host_ptr_start); + const auto range = static_cast(host_ptr_end - host_ptr_start); return range == size; } diff --git a/src/video_core/memory_manager.h b/src/video_core/memory_manager.h index f69a257065..113f9d8f3e 100644 --- a/src/video_core/memory_manager.h +++ b/src/video_core/memory_manager.h @@ -66,8 +66,8 @@ public: u8* GetPointer(GPUVAddr addr); const u8* GetPointer(GPUVAddr addr) const; - // Returns true if the block is continuous in host memory, false otherwise - bool IsBlockContinous(GPUVAddr start, std::size_t size); + /// Returns true if the block is continuous in host memory, false otherwise + bool IsBlockContinuous(GPUVAddr start, std::size_t size) const; /** * ReadBlock and WriteBlock are full read and write operations over virtual