From 7f9b5d11036fd2458e245a532a4fca7ef5c39638 Mon Sep 17 00:00:00 2001 From: baldurk Date: Mon, 10 Aug 2020 22:16:23 +0100 Subject: [PATCH] Detect mapped memory writes to tiled images and skip. Closes #1863 * We track memory bindings to see which regions of a memory object are only used for tiled images, and discard any writes in case this was accidental detection of changes by the GPU which we don't want to replay. In the case of aliasing if there's linear and tiled resources then we still replay the writes. * Note that we have to take a slower path involving a copy since we can't serialise straight into memory in this case, so applications should avoid mapping memory behind --- renderdoc/driver/vulkan/vk_core.cpp | 3 + renderdoc/driver/vulkan/vk_core.h | 2 + renderdoc/driver/vulkan/vk_info.cpp | 37 ++++ renderdoc/driver/vulkan/vk_info.h | 19 ++ .../vulkan/wrappers/vk_resource_funcs.cpp | 166 +++++++++++++++++- 5 files changed, 221 insertions(+), 6 deletions(-) diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index 4ea4ab88c..fba523eea 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -2382,6 +2382,9 @@ ReplayStatus WrappedVulkan::ReadLogInitialisation(RDCFile *rdc, bool storeStruct FreeAllMemory(MemoryScope::IndirectReadback); + for(auto it = m_CreationInfo.m_Memory.begin(); it != m_CreationInfo.m_Memory.end(); ++it) + it->second.SimplifyBindings(); + return ReplayStatus::Succeeded; } diff --git a/renderdoc/driver/vulkan/vk_core.h b/renderdoc/driver/vulkan/vk_core.h index 9c0555362..2f518e739 100644 --- a/renderdoc/driver/vulkan/vk_core.h +++ b/renderdoc/driver/vulkan/vk_core.h @@ -804,6 +804,8 @@ private: std::map> m_ResourceUses; std::map m_EventFlags; + bytebuf m_MaskedMapData; + // returns thread-local temporary memory byte *GetTempMemory(size_t s); template diff --git a/renderdoc/driver/vulkan/vk_info.cpp b/renderdoc/driver/vulkan/vk_info.cpp index 4f73d75cf..d56680960 100644 --- a/renderdoc/driver/vulkan/vk_info.cpp +++ b/renderdoc/driver/vulkan/vk_info.cpp @@ -986,6 +986,41 @@ void VulkanCreationInfo::Memory::Init(VulkanResourceManager *resourceMan, Vulkan size = pAllocInfo->allocationSize; } +void VulkanCreationInfo::Memory::SimplifyBindings() +{ + // after initialisation we're likely to end up with a lot of gaps of 'none' in between tiled or + // linear resources. Regions of memory with no bindings are not visible in any meaningful way + // (memory can only be read with an image or buffer bound to it) so we perform a pass collapsing + // any 'None' intervals into the previous to be able to simplify the set of intervals. This means + // we might promote some regions to tiled, but that's fine since as above their contents are + // essentially meaningless. + + // if the first entry is None and we have a second entry, then set the first to whatever the + // second is + if(bindings.size() > 1 && bindings.begin()->value() == VulkanCreationInfo::Memory::None) + { + auto it = bindings.begin(); + it++; + bindings.begin()->setValue(it->value()); + } + + for(auto it = bindings.begin(); it != bindings.end(); it++) + { + // if we're not at the begining and the current range is None, copy whatever was in the previous + // range + if(it != bindings.begin() && it->value() == VulkanCreationInfo::Memory::None) + { + auto previt = it; + previt--; + + it->setValue(previt->value()); + } + + // merge left when possible + it->mergeLeft(); + } +} + void VulkanCreationInfo::Buffer::Init(VulkanResourceManager *resourceMan, VulkanCreationInfo &info, const VkBufferCreateInfo *pCreateInfo) { @@ -1014,6 +1049,8 @@ void VulkanCreationInfo::Image::Init(VulkanResourceManager *resourceMan, VulkanC mipLevels = pCreateInfo->mipLevels; samples = RDCMAX(VK_SAMPLE_COUNT_1_BIT, pCreateInfo->samples); + linear = pCreateInfo->tiling == VK_IMAGE_TILING_LINEAR; + creationFlags = TextureCategory::NoFlags; if(pCreateInfo->usage & VK_IMAGE_USAGE_SAMPLED_BIT) diff --git a/renderdoc/driver/vulkan/vk_info.h b/renderdoc/driver/vulkan/vk_info.h index 2ad9659c7..05ff2e743 100644 --- a/renderdoc/driver/vulkan/vk_info.h +++ b/renderdoc/driver/vulkan/vk_info.h @@ -447,6 +447,24 @@ struct VulkanCreationInfo uint64_t size; VkBuffer wholeMemBuf; + + enum MemoryBinding + { + None = 0x0, + Linear = 0x1, + Tiled = 0x2, + LinearAndTiled = 0x3, + }; + + Intervals bindings; + + void BindMemory(uint64_t offs, uint64_t sz, MemoryBinding b) + { + bindings.update(offs, offs + sz, b, + [](MemoryBinding a, MemoryBinding b) { return MemoryBinding(a | b); }); + } + + void SimplifyBindings(); }; std::map m_Memory; @@ -484,6 +502,7 @@ struct VulkanCreationInfo uint32_t arrayLayers, mipLevels; VkSampleCountFlagBits samples; + bool linear; bool cube; TextureCategory creationFlags; }; diff --git a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp index e68dff5d5..afd0bcd90 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp @@ -708,6 +708,8 @@ bool WrappedVulkan::Serialise_vkUnmapMemory(SerialiserType &ser, VkDevice device SERIALISE_ELEMENT(MapOffset); SERIALISE_ELEMENT(MapSize); + bool directStream = true; + if(IsReplayingAndReading() && memory != VK_NULL_HANDLE) { if(IsLoading(m_State)) @@ -717,11 +719,78 @@ bool WrappedVulkan::Serialise_vkUnmapMemory(SerialiserType &ser, VkDevice device (void **)&MapData); if(vkr != VK_SUCCESS) RDCERR("Error mapping memory on replay: %s", ToStr(vkr).c_str()); + + const Intervals &bindings = + m_CreationInfo.m_Memory[GetResID(memory)].bindings; + + uint64_t finish = MapOffset + MapSize; + + auto it = bindings.find(MapOffset); + + // iterate the bindings that this map region overlaps, if we overlap with any tiled memory we + // need to take the slow path + while(it->finish() < finish) + { + if(it->value() == VulkanCreationInfo::Memory::Tiled) + { + if(IsLoading(m_State)) + { + AddDebugMessage(MessageCategory::Performance, MessageSeverity::Medium, + MessageSource::GeneralPerformance, + "Unmapped memory overlaps tiled-only memory region. " + "Taking slow path to mask tiled memory writes"); + } + directStream = false; + m_MaskedMapData.resize((size_t)MapSize); + break; + } + + it++; + } } - // not using SERIALISE_ELEMENT_ARRAY so we can deliberately avoid allocation - we serialise - // directly into upload memory - ser.Serialise("MapData"_lit, MapData, MapSize, SerialiserFlags::NoFlags); + if(directStream) + { + // not using SERIALISE_ELEMENT_ARRAY so we can deliberately avoid allocation - we serialise + // directly into upload memory + ser.Serialise("MapData"_lit, MapData, MapSize, SerialiserFlags::NoFlags); + } + else + { + // serialise into temp storage + byte *tmp = m_MaskedMapData.data(); + ser.Serialise("MapData"_lit, tmp, MapSize, SerialiserFlags::NoFlags); + + const Intervals &bindings = + m_CreationInfo.m_Memory[GetResID(memory)].bindings; + + uint64_t finish = MapOffset + MapSize; + + auto it = bindings.find(MapOffset); + + // iterate the bindings that this map region overlaps, and only memcpy the bits that we overlap + // which are linear + while(it->finish() < finish) + { + if(it->value() != VulkanCreationInfo::Memory::Tiled) + { + // start at the map offset or the region offset, whichever is *later*. E.g. if the region is + // larger than the map we only start where the map started, and vice-versa if the map + // started earlier than the region. + // We also rebase it so that it's relative to the map, so it's the byte offset for the + // memcpy + size_t offs = size_t(RDCMAX(it->start(), MapOffset) - MapOffset); + + // similarly, only copy up to the end of the region or the end ofthe map whichever is + // *sooner*. + size_t size = size_t(RDCMIN(it->finish(), finish) - offs); + + memcpy(MapData + offs, m_MaskedMapData.data() + offs, size); + } + + it++; + } + } if(IsReplayingAndReading() && MapData && memory != VK_NULL_HANDLE) ObjDisp(device)->UnmapMemory(Unwrap(device), Unwrap(memory)); @@ -840,6 +909,8 @@ bool WrappedVulkan::Serialise_vkFlushMappedMemoryRanges(SerialiserType &ser, VkD MappedData = state->cpuReadPtr + (size_t)MemRange.offset; } + bool directStream = true; + if(IsReplayingAndReading() && MemRange.memory != VK_NULL_HANDLE && MemRange.size > 0) { if(IsLoading(m_State)) @@ -851,11 +922,78 @@ bool WrappedVulkan::Serialise_vkFlushMappedMemoryRanges(SerialiserType &ser, VkD MemRange.size, 0, (void **)&MappedData); if(ret != VK_SUCCESS) RDCERR("Error mapping memory on replay: %s", ToStr(ret).c_str()); + + const Intervals &bindings = + m_CreationInfo.m_Memory[GetResID(MemRange.memory)].bindings; + + uint64_t finish = MemRange.offset + MemRange.size; + + auto it = bindings.find(MemRange.offset); + + // iterate the bindings that this map region overlaps, if we overlap with any tiled memory we + // need to take the slow path + while(it->finish() < finish) + { + if(it->value() == VulkanCreationInfo::Memory::Tiled) + { + if(IsLoading(m_State)) + { + AddDebugMessage(MessageCategory::Performance, MessageSeverity::Medium, + MessageSource::GeneralPerformance, + "Unmapped memory overlaps tiled-only memory region. " + "Taking slow path to mask tiled memory writes"); + } + directStream = false; + m_MaskedMapData.resize((size_t)MemRange.size); + break; + } + + it++; + } } - // not using SERIALISE_ELEMENT_ARRAY so we can deliberately avoid allocation - we serialise - // directly into upload memory - ser.Serialise("MappedData"_lit, MappedData, memRangeSize, SerialiserFlags::NoFlags); + if(directStream) + { + // not using SERIALISE_ELEMENT_ARRAY so we can deliberately avoid allocation - we serialise + // directly into upload memory + ser.Serialise("MappedData"_lit, MappedData, memRangeSize, SerialiserFlags::NoFlags); + } + else + { + // serialise into temp storage + byte *tmp = m_MaskedMapData.data(); + ser.Serialise("MapData"_lit, tmp, MemRange.size, SerialiserFlags::NoFlags); + + const Intervals &bindings = + m_CreationInfo.m_Memory[GetResID(MemRange.memory)].bindings; + + uint64_t finish = MemRange.offset + MemRange.size; + + auto it = bindings.find(MemRange.offset); + + // iterate the bindings that this map region overlaps, and only memcpy the bits that we overlap + // which are linear + while(it->finish() < finish) + { + if(it->value() != VulkanCreationInfo::Memory::Tiled) + { + // start at the map offset or the region offset, whichever is *later*. E.g. if the region is + // larger than the map we only start where the map started, and vice-versa if the map + // started earlier than the region. + // We also rebase it so that it's relative to the map, so it's the byte offset for the + // memcpy + size_t offs = size_t(RDCMAX(it->start(), MemRange.offset) - MemRange.offset); + + // similarly, only copy up to the end of the region or the end ofthe map whichever is + // *sooner*. + size_t size = size_t(RDCMIN(it->finish(), finish) - offs); + + memcpy(MappedData + offs, m_MaskedMapData.data() + offs, size); + } + + it++; + } + } if(IsReplayingAndReading() && MappedData && MemRange.memory != VK_NULL_HANDLE && MemRange.size > 0) ObjDisp(device)->UnmapMemory(Unwrap(device), Unwrap(MemRange.memory)); @@ -1053,6 +1191,9 @@ bool WrappedVulkan::Serialise_vkBindBufferMemory(SerialiserType &ser, VkDevice d else if(GetExtensions(GetRecord(device)).ext_EXT_buffer_device_address) bufInfo.gpuAddress = ObjDisp(device)->GetBufferDeviceAddressEXT(Unwrap(device), &getInfo); } + + m_CreationInfo.m_Memory[GetResID(memory)].BindMemory(memoryOffset, mrq.size, + VulkanCreationInfo::Memory::Linear); } return true; @@ -1153,6 +1294,11 @@ bool WrappedVulkan::Serialise_vkBindImageMemory(SerialiserType &ser, VkDevice de AddResourceCurChunk(memOrigId); AddResourceCurChunk(resOrigId); + + VulkanCreationInfo::Image &imgInfo = m_CreationInfo.m_Image[GetResID(image)]; + m_CreationInfo.m_Memory[GetResID(memory)].BindMemory( + memoryOffset, mrq.size, + imgInfo.linear ? VulkanCreationInfo::Memory::Linear : VulkanCreationInfo::Memory::Tiled); } return true; @@ -2170,6 +2316,9 @@ bool WrappedVulkan::Serialise_vkBindBufferMemory2(SerialiserType &ser, VkDevice else if(GetExtensions(GetRecord(device)).ext_EXT_buffer_device_address) bufInfo.gpuAddress = ObjDisp(device)->GetBufferDeviceAddressEXT(Unwrap(device), &getInfo); } + + m_CreationInfo.m_Memory[GetResID(bindInfo.memory)].BindMemory( + bindInfo.memoryOffset, mrq.size, VulkanCreationInfo::Memory::Linear); } VkBindBufferMemoryInfo *unwrapped = UnwrapInfos(pBindInfos, bindInfoCount); @@ -2283,6 +2432,11 @@ bool WrappedVulkan::Serialise_vkBindImageMemory2(SerialiserType &ser, VkDevice d AddResourceCurChunk(memOrigId); AddResourceCurChunk(resOrigId); + + VulkanCreationInfo::Image &imgInfo = m_CreationInfo.m_Image[GetResID(bindInfo.image)]; + m_CreationInfo.m_Memory[GetResID(bindInfo.memory)].BindMemory( + bindInfo.memoryOffset, mrq.size, + imgInfo.linear ? VulkanCreationInfo::Memory::Linear : VulkanCreationInfo::Memory::Tiled); } VkBindImageMemoryInfo *unwrapped = UnwrapInfos(pBindInfos, bindInfoCount);