From 5235b053b46769fd808974ef409e86df992fd051 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 e4f0c4bd6..b9a683497 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 53afe47cec0bfb7ae8b78a95d15f0e081e1a84c9 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 b9a683497..ade18c139 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 fd12788967729dc3712c23723957ab6b15d36822 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 6c98c6701..2890ea947 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 ade18c139..4f7b57f8e 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 d4bcd006b298e7a58026e2cd311d581b25731ddd 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 4f7b57f8e..f69a25706 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 716fbaef743e46a063dc8b4e5e542e0c2ee2141e 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 2890ea947..5d8d126c1 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 f69a25706..113f9d8f3 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