Remove memory index mapping from physical device record

* This never worked because the spec requires a coherent memory type and it must
  be sorted first, many applications either used it anyway no matter how
  unappealing we made it look or else ignored the coherent flag entirely but
  treated memory as coherent anyway...
* The extra cold data pointer in the physical device record is the instDevInfo
This commit is contained in:
baldurk committed 2019-12-09 16:37:11 +00:00
1 parent 28e4039db8
commit e83bc8ed04
8 files changed
+17 -232

No files matched your search

-3
View File
@@ -185,9 +185,6 @@ WrappedVulkan::~WrappedVulkan()
SAFE_DELETE(m_FrameReader);
for(size_t i = 0; i < m_MemIdxMaps.size(); i++)
delete[] m_MemIdxMaps[i];
for(size_t i = 0; i < m_ThreadSerialisers.size(); i++)
delete m_ThreadSerialisers[i];
-6
View File
@@ -369,9 +369,6 @@ private:
uint32_t uploadMemIndex = 0;
uint32_t GPULocalMemIndex = 0;
VkPhysicalDeviceMemoryProperties *fakeMemProps = NULL;
uint32_t *memIdxMap = NULL;
VkPhysicalDeviceFeatures features = {};
VkPhysicalDeviceProperties props = {};
VkPhysicalDeviceMemoryProperties memProps = {};
@@ -453,9 +450,6 @@ private:
// (targetQueueFamily << 32) | (targetQueueIndex)
std::vector<QueueRemap> m_QueueRemapping[16];
std::vector<uint32_t *> m_MemIdxMaps;
void RemapMemoryIndices(VkPhysicalDeviceMemoryProperties *memProps, uint32_t **memIdxMap);
void WrapAndProcessCreatedSwapchain(VkDevice device, const VkSwapchainCreateInfoKHR *pCreateInfo,
VkSwapchainKHR *pSwapChain);
-92
View File
@@ -83,98 +83,6 @@ uint32_t WrappedVulkan::PhysicalDeviceData::GetMemoryIndex(uint32_t resourceRequ
return best;
}
#define CREATE_NON_COHERENT_ATTRACTIVE_MEMORY 0
void WrappedVulkan::RemapMemoryIndices(VkPhysicalDeviceMemoryProperties *memProps,
uint32_t **memIdxMap)
{
uint32_t *memmap = new uint32_t[VK_MAX_MEMORY_TYPES];
*memIdxMap = memmap;
m_MemIdxMaps.push_back(memmap);
for(size_t i = 0; i < VK_MAX_MEMORY_TYPES; i++)
memmap[i] = ~0U;
// basic idea here:
// We want to discourage coherent memory maps as much as possible while capturing,
// as they're painful to track. Unfortunately the spec guarantees that at least
// one such memory type will be available, and we must follow that.
//
// So, rather than removing the coherent memory type we make it as unappealing as
// possible and try and ensure that only someone looking specifically for a coherent
// memory type will find it. That way hopefully memory selection algorithms will
// pick non-coherent memory and do proper flushing as necessary.
// we want to add a new heap, hopefully there is room
#if CREATE_NON_COHERENT_ATTRACTIVE_MEMORY
RDCASSERT(memProps->memoryHeapCount < VK_MAX_MEMORY_HEAPS - 1);
uint32_t coherentHeap = memProps->memoryHeapCount;
memProps->memoryHeapCount++;
// make a new heap that's tiny. If any applications look at heap sizes to determine
// viability, they'll dislike the look of this one (the real heaps should be much
// bigger).
memProps->memoryHeaps[coherentHeap].flags = 0; // not device local
memProps->memoryHeaps[coherentHeap].size = 32 * 1024 * 1024;
#endif
// for every coherent memory type, add a non-coherent type first, then
// mark the coherent type with our crappy heap
uint32_t origCount = memProps->memoryTypeCount;
VkMemoryType origTypes[VK_MAX_MEMORY_TYPES];
memcpy(origTypes, memProps->memoryTypes, sizeof(origTypes));
uint32_t newtypeidx = 0;
for(uint32_t i = 0; i < origCount; i++)
{
#if CREATE_NON_COHERENT_ATTRACTIVE_MEMORY
if((origTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT) != 0)
{
// coherent type found.
// can we still add a new type without exceeding the max?
if(memProps->memoryTypeCount + 1 <= VK_MAX_MEMORY_TYPES)
{
// copy both types from the original type
memProps->memoryTypes[newtypeidx] = origTypes[i];
memProps->memoryTypes[newtypeidx + 1] = origTypes[i];
// mark first as non-coherent, cached
memProps->memoryTypes[newtypeidx].propertyFlags &= ~VK_MEMORY_PROPERTY_HOST_COHERENT_BIT;
memProps->memoryTypes[newtypeidx].propertyFlags |= VK_MEMORY_PROPERTY_HOST_CACHED_BIT;
// point second at bad heap
memProps->memoryTypes[newtypeidx + 1].heapIndex = coherentHeap;
// point both new types at this original type
memmap[newtypeidx++] = i;
memmap[newtypeidx++] = i;
// we added a type
memProps->memoryTypeCount++;
}
else
{
// can't add a new type, but we can at least repoint this coherent
// type at the bad heap to discourage use
memProps->memoryTypes[newtypeidx] = origTypes[i];
memProps->memoryTypes[newtypeidx].heapIndex = coherentHeap;
memmap[newtypeidx++] = i;
}
}
else
#endif
{
// non-coherent already or non-hostvisible, just copy through
memProps->memoryTypes[newtypeidx] = origTypes[i];
memmap[newtypeidx++] = i;
}
}
}
MemoryAllocation WrappedVulkan::AllocateMemoryForResource(bool buffer, VkMemoryRequirements mrq,
MemoryScope scope, MemoryType type)
{
+1 -4
View File
@@ -3391,14 +3391,11 @@ VkResourceRecord::~VkResourceRecord()
{
VkResourceType resType = Resource != NULL ? IdentifyTypeByPtr(Resource) : eResUnknown;
if(resType == eResPhysicalDevice)
SAFE_DELETE(memProps);
// bufferviews and imageviews have non-owning pointers to the sparseinfo struct
if(resType == eResBuffer || resType == eResImage)
SAFE_DELETE(resInfo);
if(resType == eResInstance || resType == eResDevice)
if(resType == eResInstance || resType == eResDevice || resType == eResPhysicalDevice)
SAFE_DELETE(instDevInfo);
if(resType == eResSwapchain)
+9 -20
View File
@@ -1422,12 +1422,7 @@ public:
static byte markerValue[32];
VkResourceRecord(ResourceId id)
: ResourceRecord(id, true),
Resource(NULL),
bakedCommands(NULL),
pool(NULL),
memIdxMap(NULL),
ptrunion(NULL)
: ResourceRecord(id, true), Resource(NULL), bakedCommands(NULL), pool(NULL), ptrunion(NULL)
{
}
@@ -1551,11 +1546,6 @@ public:
WrappedVkRes *Resource;
// externally allocated/freed, a mapping from memory idx
// in our modified properties that were passed to the app
// to the memory indices that actually exist
uint32_t *memIdxMap;
// this points to the base resource, either memory or an image -
// ie. the resource that can be modified or changes (or can become dirty)
// since typical memory bindings are immutable and must happen before
@@ -1582,15 +1572,14 @@ public:
// allocation type of the Resource
union
{
void *ptrunion; // for initialisation to NULL
VkPhysicalDeviceMemoryProperties *memProps; // only for physical devices
InstanceDeviceInfo *instDevInfo; // only for logical devices or instances
ResourceInfo *resInfo; // only for buffers, images, and views of them
SwapchainInfo *swapInfo; // only for swapchains
MemMapState *memMapState; // only for device memory
CmdBufferRecordingInfo *cmdInfo; // only for command buffers
AttachmentInfo *imageAttachments; // only for framebuffers and render passes
PipelineLayoutData *pipeLayoutInfo; // only for pipeline layouts
void *ptrunion; // for initialisation to NULL
InstanceDeviceInfo *instDevInfo; // only for instances or physical/logical devices
ResourceInfo *resInfo; // only for buffers, images, and views of them
SwapchainInfo *swapInfo; // only for swapchains
MemMapState *memMapState; // only for device memory
CmdBufferRecordingInfo *cmdInfo; // only for command buffers
AttachmentInfo *imageAttachments; // only for framebuffers and render passes
PipelineLayoutData *pipeLayoutInfo; // only for pipeline layouts
DescriptorSetData *descInfo; // only for descriptor sets and descriptor set layouts
DescUpdateTemplate *descTemplateInfo; // only for descriptor update templates
uint32_t queueFamilyIndex; // only for queues
@@ -472,7 +472,6 @@ ReplayStatus WrappedVulkan::Initialise(VkInitParams &params, uint64_t sectionVer
m_ReplayPhysicalDevices.resize(count);
m_ReplayPhysicalDevicesUsed.resize(count);
m_OriginalPhysicalDevices.resize(count);
m_MemIdxMaps.resize(count);
vkr = ObjDisp(m_Instance)
->EnumeratePhysicalDevices(Unwrap(m_Instance), &count, &m_ReplayPhysicalDevices[0]);
@@ -914,7 +913,7 @@ bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices(SerialiserType &ser, Vk
SERIALISE_ELEMENT_LOCAL(PhysicalDevice, GetResID(*pPhysicalDevices))
.TypedAs("VkPhysicalDevice"_lit);
uint32_t memIdxMap[VK_MAX_MEMORY_TYPES] = {0};
uint32_t legacyUnused_memIdxMap[VK_MAX_MEMORY_TYPES] = {0};
// not used at the moment but useful for reference and might be used
// in the future
VkPhysicalDeviceProperties physProps = {};
@@ -929,8 +928,6 @@ bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices(SerialiserType &ser, Vk
if(ser.IsWriting())
{
memcpy(memIdxMap, GetRecord(*pPhysicalDevices)->memIdxMap, sizeof(memIdxMap));
ObjDisp(instance)->GetPhysicalDeviceProperties(Unwrap(*pPhysicalDevices), &physProps);
ObjDisp(instance)->GetPhysicalDeviceMemoryProperties(Unwrap(*pPhysicalDevices), &memProps);
ObjDisp(instance)->GetPhysicalDeviceFeatures(Unwrap(*pPhysicalDevices), &physFeatures);
@@ -975,7 +972,7 @@ bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices(SerialiserType &ser, Vk
}
}
SERIALISE_ELEMENT(memIdxMap);
SERIALISE_ELEMENT(legacyUnused_memIdxMap).Hidden(); // was never used
SERIALISE_ELEMENT(physProps);
SERIALISE_ELEMENT(memProps);
SERIALISE_ELEMENT(physFeatures);
@@ -1019,7 +1016,7 @@ bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices(SerialiserType &ser, Vk
// hopefully the most common case is when there's a precise match, and maybe the order changed.
//
// If more GPUs were present on replay than during capture, we map many-to-one which might have
// bad side-effects as e.g. we have to pick one memidxmap, but this is as good as we can do.
// bad side-effects, but this is as good as we can do.
uint32_t bestIdx = 0;
VkPhysicalDeviceProperties bestPhysProps = {};
@@ -1309,17 +1306,6 @@ bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices(SerialiserType &ser, Vk
"Mapping multiple capture-time physical devices to a single replay-time physical device."
"This means the HW has changed between capture and replay and may cause bugs.");
}
else if(m_MemIdxMaps[bestIdx] == NULL)
{
// the first physical device 'wins' for the memory index map
uint32_t *storedMap = new uint32_t[32];
memcpy(storedMap, memIdxMap, sizeof(memIdxMap));
for(uint32_t i = 0; i < 32; i++)
storedMap[i] = i;
m_MemIdxMaps[bestIdx] = storedMap;
}
m_ReplayPhysicalDevicesUsed[bestIdx] = true;
}
@@ -1367,10 +1353,6 @@ VkResult WrappedVulkan::vkEnumeratePhysicalDevices(VkInstance instance,
VkResourceRecord *record = GetResourceManager()->AddResourceRecord(devices[i]);
RDCASSERT(record);
record->memProps = new VkPhysicalDeviceMemoryProperties();
ObjDisp(devices[i])->GetPhysicalDeviceMemoryProperties(Unwrap(devices[i]), record->memProps);
VkPhysicalDeviceProperties physProps;
ObjDisp(devices[i])->GetPhysicalDeviceProperties(Unwrap(devices[i]), &physProps);
@@ -1383,9 +1365,6 @@ VkResult WrappedVulkan::vkEnumeratePhysicalDevices(VkInstance instance,
m_PhysicalDevices[i] = devices[i];
// we remap memory indices to discourage coherent maps as much as possible
RemapMemoryIndices(record->memProps, &record->memIdxMap);
{
CACHE_THREAD_SERIALISER();
@@ -1399,6 +1378,9 @@ VkResult WrappedVulkan::vkEnumeratePhysicalDevices(VkInstance instance,
instrecord->AddParent(record);
// copy the instance's setup directly
record->instDevInfo = new InstanceDeviceInfo(*instrecord->instDevInfo);
// treat physical devices as pool members of the instance (ie. freed when the instance dies)
{
instrecord->LockChunks();
@@ -2758,15 +2740,6 @@ bool WrappedVulkan::Serialise_vkCreateDevice(SerialiserType &ser, VkPhysicalDevi
m_PhysicalDeviceData.GPULocalMemIndex = m_PhysicalDeviceData.GetMemoryIndex(
~0U, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT);
for(size_t i = 0; i < m_ReplayPhysicalDevices.size(); i++)
{
if(physicalDevice == m_ReplayPhysicalDevices[i])
{
m_PhysicalDeviceData.memIdxMap = m_MemIdxMaps[i];
break;
}
}
APIProps.vendor = GetDriverInfo().Vendor();
// temporarily disable the debug message sink, to ignore any false positive messages from our
@@ -3086,8 +3059,6 @@ VkResult WrappedVulkan::vkCreateDevice(VkPhysicalDevice physicalDevice,
record->AddChunk(chunk);
record->memIdxMap = GetRecord(physicalDevice)->memIdxMap;
record->instDevInfo = new InstanceDeviceInfo();
record->instDevInfo->brokenGetDeviceProcAddr =
@@ -3217,8 +3188,6 @@ VkResult WrappedVulkan::vkCreateDevice(VkPhysicalDevice physicalDevice,
m_PhysicalDeviceData.queueCount = qCount;
memcpy(m_PhysicalDeviceData.queueProps, props, qCount * sizeof(VkQueueFamilyProperties));
m_PhysicalDeviceData.fakeMemProps = GetRecord(physicalDevice)->memProps;
m_ShaderCache = new VulkanShaderCache(this);
m_TextRenderer = new VulkanTextRenderer(this);
@@ -211,12 +211,6 @@ void WrappedVulkan::vkGetPhysicalDeviceQueueFamilyProperties(
void WrappedVulkan::vkGetPhysicalDeviceMemoryProperties(
VkPhysicalDevice physicalDevice, VkPhysicalDeviceMemoryProperties *pMemoryProperties)
{
if(pMemoryProperties)
{
*pMemoryProperties = *GetRecord(physicalDevice)->memProps;
return;
}
ObjDisp(physicalDevice)->GetPhysicalDeviceMemoryProperties(Unwrap(physicalDevice), pMemoryProperties);
}
@@ -237,21 +231,6 @@ void WrappedVulkan::vkGetBufferMemoryRequirements(VkDevice device, VkBuffer buff
*pMemoryRequirements = GetRecord(buffer)->resInfo->memreqs;
else
ObjDisp(device)->GetBufferMemoryRequirements(Unwrap(device), Unwrap(buffer), pMemoryRequirements);
// don't do remapping here on replay.
if(IsReplayMode(m_State))
return;
uint32_t bits = pMemoryRequirements->memoryTypeBits;
uint32_t *memIdxMap = GetRecord(device)->memIdxMap;
pMemoryRequirements->memoryTypeBits = 0;
// for each of our fake memory indices, check if the real
// memory type it points to is set - if so, set our fake bit
for(uint32_t i = 0; i < VK_MAX_MEMORY_TYPES; i++)
if(memIdxMap[i] < 32U && (bits & (1U << memIdxMap[i])))
pMemoryRequirements->memoryTypeBits |= (1U << i);
}
void WrappedVulkan::vkGetImageMemoryRequirements(VkDevice device, VkImage image,
@@ -265,21 +244,6 @@ void WrappedVulkan::vkGetImageMemoryRequirements(VkDevice device, VkImage image,
else
ObjDisp(device)->GetImageMemoryRequirements(Unwrap(device), Unwrap(image), pMemoryRequirements);
// don't do remapping here on replay.
if(IsReplayMode(m_State))
return;
uint32_t bits = pMemoryRequirements->memoryTypeBits;
uint32_t *memIdxMap = GetRecord(device)->memIdxMap;
pMemoryRequirements->memoryTypeBits = 0;
// for each of our fake memory indices, check if the real
// memory type it points to is set - if so, set our fake bit
for(uint32_t i = 0; i < VK_MAX_MEMORY_TYPES; i++)
if(memIdxMap[i] < 32U && (bits & (1U << memIdxMap[i])))
pMemoryRequirements->memoryTypeBits |= (1U << i);
// AMD can have some variability in the returned size, so we need to pad the reported size to
// allow for this. The variability isn't quite clear, but for now we assume aligning size to
// alignment * 4 should be sufficient (adding on a fixed padding won't help the problem as it
@@ -326,21 +290,6 @@ void WrappedVulkan::vkGetBufferMemoryRequirements2(VkDevice device,
// pessimistic for the case of external memory bound resources. See vkCreateBuffer/vkCreateImage
if(IsCaptureMode(m_State) && GetRecord(pInfo->buffer)->resInfo)
pMemoryRequirements->memoryRequirements = GetRecord(pInfo->buffer)->resInfo->memreqs;
// don't do remapping here on replay.
if(IsReplayMode(m_State))
return;
uint32_t bits = pMemoryRequirements->memoryRequirements.memoryTypeBits;
uint32_t *memIdxMap = GetRecord(device)->memIdxMap;
pMemoryRequirements->memoryRequirements.memoryTypeBits = 0;
// for each of our fake memory indices, check if the real
// memory type it points to is set - if so, set our fake bit
for(uint32_t i = 0; i < VK_MAX_MEMORY_TYPES; i++)
if(memIdxMap[i] < 32U && (bits & (1U << memIdxMap[i])))
pMemoryRequirements->memoryRequirements.memoryTypeBits |= (1U << i);
}
void WrappedVulkan::vkGetImageMemoryRequirements2(VkDevice device,
@@ -361,17 +310,6 @@ void WrappedVulkan::vkGetImageMemoryRequirements2(VkDevice device,
if(IsReplayMode(m_State))
return;
uint32_t bits = pMemoryRequirements->memoryRequirements.memoryTypeBits;
uint32_t *memIdxMap = GetRecord(device)->memIdxMap;
pMemoryRequirements->memoryRequirements.memoryTypeBits = 0;
// for each of our fake memory indices, check if the real
// memory type it points to is set - if so, set our fake bit
for(uint32_t i = 0; i < VK_MAX_MEMORY_TYPES; i++)
if(memIdxMap[i] < 32U && (bits & (1U << memIdxMap[i])))
pMemoryRequirements->memoryRequirements.memoryTypeBits |= (1U << i);
// AMD can have some variability in the returned size, so we need to pad the reported size to
// allow for this. The variability isn't quite clear, but for now we assume aligning size to
// alignment * 4 should be sufficient (adding on a fixed padding won't help the problem as it
@@ -263,11 +263,6 @@ bool WrappedVulkan::Serialise_vkAllocateMemory(SerialiserType &ser, VkDevice dev
{
VkDeviceMemory mem = VK_NULL_HANDLE;
// serialised memory type index is non-remapped, so we remap now.
// PORTABILITY may need to re-write info to change memory type index to the
// appropriate index on replay
AllocateInfo.memoryTypeIndex = m_PhysicalDeviceData.memIdxMap[AllocateInfo.memoryTypeIndex];
VkMemoryAllocateInfo patched = AllocateInfo;
byte *tempMem = GetTempMemory(GetNextPatchSize(patched.pNext));
@@ -345,8 +340,6 @@ VkResult WrappedVulkan::vkAllocateMemory(VkDevice device, const VkMemoryAllocate
VkDeviceMemory *pMemory)
{
VkMemoryAllocateInfo info = *pAllocateInfo;
if(IsCaptureMode(m_State))
info.memoryTypeIndex = GetRecord(device)->memIdxMap[info.memoryTypeIndex];
{
// we need to be able to allocate a buffer that covers the whole memory range. However
@@ -469,7 +462,7 @@ VkResult WrappedVulkan::vkAllocateMemory(VkDevice device, const VkMemoryAllocate
record->Length = info.allocationSize;
uint32_t memProps =
m_PhysicalDeviceData.fakeMemProps->memoryTypes[info.memoryTypeIndex].propertyFlags;
m_PhysicalDeviceData.memProps.memoryTypes[info.memoryTypeIndex].propertyFlags;
// if memory is not host visible, so not mappable, don't create map state at all
if((memProps & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) != 0)