From d6af74d9b0e8f45fffbdbc0c587a37209ffad8ac Mon Sep 17 00:00:00 2001 From: Yahan Zhou Date: Tue, 9 Apr 2024 16:39:07 -0700 Subject: [PATCH] Handle dependency by VkMemoryDedicatedAllocateInfo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have a situation that if an image memory is created with VkMemoryDedicatedAllocateInfo, the following commands that refer to the same image must happen in this exact order: vkCreateImage vkAllocateMemory (with dedicated image memory) vkBindImageMemory vkCreateImageView Our previous dependency graph implementation uses Vulkan objects as nodes and does not haven enough granularity to handle this situation. vkCreateImageView can only be executed after an image is created and bound to memory. Our previous dependency graph does not have the notion of "bound to memory". We worked around it by adding vkBindImageMemory as part of the image initialization steps. To make sure we can bind image at initialization, we marked image to be dependent on the memory it bounds to. When adding the new vkAllocateMemory with dedicated image memory extension into the mix, it introduced a circular dependency. vkAllocateMemory will say the memory has dependency on the image. vkBindImageMemory will say the image has dependency on the memory. To properly resolve the issue, we will need to record the state of an Vulkan object, and declare that vkCreateImageView not only depends on an image, but also requires the image to be bound to memory. We do so by introducing a "State" enum and use the pair as the vertex in the deppendency graph. Currently State is an enum with 2 values: CREATED and BOUND_MEMORY. By default, after a VkCreate* command, an object will be in CREATED state. When calling vkBindImageMemory or vkBindBufferMemory it will transform the image or buffer into BOUND_MEMORY state. Then vkCreateImageView will have dependency with {VkImage, BOUND_MEMORY}. Then we can create a dependency graph that looks like this: VkImageView --> {VkImage,BOUND_MEMORY} --> VkMemory | | | ┌-------------------⅃ v v VkImage No more circular dependency. Reviewed-by: Aaron Ruby Acked-by: Yonggang Luo Acked-by: Adam Jackson Part-of: --- .../codegen/scripts/cereal/decodersnapshot.py | 52 +++++++++++++++---- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py b/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py index 450cf8afbfd..dc708502557 100644 --- a/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py +++ b/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py @@ -9,6 +9,7 @@ from .wrapperdefs import API_PREFIX_UNMARSHAL from .wrapperdefs import VULKAN_STREAM_TYPE from copy import copy +from dataclasses import dataclass decoder_snapshot_decl_preamble = """ @@ -106,12 +107,25 @@ SNAPSHOT_HANDLE_DEPENDENCIES = [ handleDependenciesDict = dict(SNAPSHOT_HANDLE_DEPENDENCIES) +def extract_deps_vkAllocateMemory(param, access, lenExpr, api, cgen): + cgen.stmt("const VkMemoryDedicatedAllocateInfo* dedicatedAllocateInfo = vk_find_struct(pAllocateInfo)"); + cgen.beginIf("dedicatedAllocateInfo"); + cgen.beginIf("dedicatedAllocateInfo->image") + cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s)" % \ + (access, lenExpr, "unboxed_to_boxed_non_dispatchable_VkImage(dedicatedAllocateInfo->image)")) + cgen.endIf() + cgen.beginIf("dedicatedAllocateInfo->buffer") + cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s)" % \ + (access, lenExpr, "unboxed_to_boxed_non_dispatchable_VkBuffer(dedicatedAllocateInfo->buffer)")) + cgen.endIf() + cgen.endIf() + def extract_deps_vkAllocateCommandBuffers(param, access, lenExpr, api, cgen): cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s)" % \ (access, lenExpr, "unboxed_to_boxed_non_dispatchable_VkCommandPool(pAllocateInfo->commandPool)")) def extract_deps_vkCreateImageView(param, access, lenExpr, api, cgen): - cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s)" % \ + cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s, VkReconstruction::CREATED, VkReconstruction::BOUND_MEMORY)" % \ (access, lenExpr, "unboxed_to_boxed_non_dispatchable_VkImage(pCreateInfo->image)")) def extract_deps_vkCreateGraphicsPipelines(param, access, lenExpr, api, cgen): @@ -131,11 +145,14 @@ def extract_deps_vkCreateFramebuffer(param, access, lenExpr, api, cgen): cgen.endFor() def extract_deps_vkBindImageMemory(param, access, lenExpr, api, cgen): - cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s)" % \ + cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s, VkReconstruction::BOUND_MEMORY)" % \ (access, lenExpr, "(uint64_t)(uintptr_t)unboxed_to_boxed_non_dispatchable_VkDeviceMemory(memory)")) + cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)((%s)[0]), VkReconstruction::BOUND_MEMORY)" % \ + (access, lenExpr, access)) specialCaseDependencyExtractors = { "vkAllocateCommandBuffers" : extract_deps_vkAllocateCommandBuffers, + "vkAllocateMemory" : extract_deps_vkAllocateMemory, "vkCreateImageView" : extract_deps_vkCreateImageView, "vkCreateGraphicsPipelines" : extract_deps_vkCreateGraphicsPipelines, "vkCreateFramebuffer" : extract_deps_vkCreateFramebuffer, @@ -146,11 +163,14 @@ apiSequences = { "vkAllocateMemory" : ["vkAllocateMemory", "vkMapMemoryIntoAddressSpaceGOOGLE"] } -# vkBindImageMemory needs to execute before vkCreateImageView -# Thus we inject it into VkImage creation sequence. +@dataclass(frozen=True) +class VkObjectState: + vk_object : str + state : str = "VkReconstruction::CREATED" + # TODO: add vkBindImageMemory2 into this list -apiInitializes = { - "vkBindImageMemory": ["image"], +apiChangeState = { + "vkBindImageMemory": VkObjectState("image", "VkReconstruction::BOUND_MEMORY"), } apiModifies = { @@ -162,12 +182,22 @@ delayedDestroys = [ "vkDestroyShaderModule", ] -def is_initialize_operation(api, param): - if api.name in apiInitializes: - if param.paramName in apiInitializes[api.name]: +def is_state_change_operation(api, param): + if param.isCreatedBy(api): + return True + if api.name in apiChangeState: + if param.paramName == apiChangeState[api.name].vk_object: return True return False +def get_target_state(api, param): + if param.isCreatedBy(api): + return "VkReconstruction::CREATED" + if api.name in apiChangeState: + if param.paramName == apiChangeState[api.name].vk_object: + return apiChangeState[api.name].state + return None + def is_modify_operation(api, param): if api.name in apiModifies: if param.paramName in apiModifies[api.name]: @@ -192,7 +222,7 @@ def emit_impl(typeInfo, api, cgen): else: access = "(&%s)" % p.paramName - if p.isCreatedBy(api) or is_initialize_operation(api, p): + if is_state_change_operation(api, p): if p.isCreatedBy(api): boxed_access = access else: @@ -218,7 +248,7 @@ def emit_impl(typeInfo, api, cgen): cgen.stmt("mReconstruction.setApiTrace(apiInfo, OP_%s, snapshotTraceBegin, snapshotTraceBytes)" % api.name) if lenAccessGuard is not None: cgen.beginIf(lenAccessGuard) - cgen.stmt("mReconstruction.forEachHandleAddApi((const uint64_t*)%s, %s, apiHandle)" % (boxed_access, lenExpr)) + cgen.stmt(f"mReconstruction.forEachHandleAddApi((const uint64_t*){boxed_access}, {lenExpr}, apiHandle, {get_target_state(api, p)})") if p.isCreatedBy(api): cgen.stmt("mReconstruction.setCreatedHandlesForApi(apiHandle, (const uint64_t*)%s, %s)" % (boxed_access, lenExpr)) if lenAccessGuard is not None: