Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Vulkan deferred operation handling errors #1192

Merged
merged 4 commits into from
Dec 18, 2023
Merged

Fix Vulkan deferred operation handling errors #1192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
59818c6
Fix deferred operation handling error
mizhen Dec 11, 2023
b252f05
Fix an error when target app provide null handle for deferred operation
mizhen Aug 23, 2023
afbbfc8
Fix replay issue on some hardware
mizhen Nov 2, 2023
adee9fb
Address code review comment
mizhen Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions framework/decode/vulkan_consumer_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
** Copyright (c) 2018-2023 Valve Corporation
** Copyright (c) 2018-2023 LunarG, Inc.
** Copyright (c) 2023 Advanced Micro Devices, Inc. All rights reserved.
**
** Permission is hereby granted, free of charge, to any person obtaining a
** copy of this software and associated documentation files (the "Software"),
Expand Down Expand Up @@ -77,6 +78,18 @@ class VulkanConsumerBase : public MetadataConsumerBase, public MarkerConsumerBas
DescriptorUpdateTemplateDecoder* pData)
{}

virtual void Process_vkCreateRayTracingPipelinesKHR(
const ApiCallInfo& call_info,
VkResult returnValue,
format::HandleId device,
format::HandleId deferredOperation,
format::HandleId pipelineCache,
uint32_t createInfoCount,
StructPointerDecoder<Decoded_VkRayTracingPipelineCreateInfoKHR>* pCreateInfos,
StructPointerDecoder<Decoded_VkAllocationCallbacks>* pAllocator,
HandlePointerDecoder<VkPipeline>* pPipelines)
{}

virtual void ProcessSetTlasToBlasRelationCommand(format::HandleId tlas, const std::vector<format::HandleId>& blases)
{}
};
Expand Down
5 changes: 4 additions & 1 deletion framework/decode/vulkan_object_info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
** Copyright (c) 2019-2022 LunarG, Inc.
** Copyright (c) 2023 Advanced Micro Devices, Inc. All rights reserved.
**
** Permission is hereby granted, free of charge, to any person obtaining a
** copy of this software and associated documentation files (the "Software"),
Expand Down Expand Up @@ -432,11 +433,13 @@ struct FramebufferInfo : public VulkanObjectInfo<VkFramebuffer>

struct DeferredOperationKHRInfo : public VulkanObjectInfo<VkDeferredOperationKHR>
{
VkResult join_state{ VK_NOT_READY };
bool pending_state{ false };

// Record CreateRayTracingPipelinesKHR parameters for safety.
std::vector<VkRayTracingPipelineCreateInfoKHR> record_modified_create_infos;
std::vector<std::vector<VkRayTracingShaderGroupCreateInfoKHR>> record_modified_pgroups;
std::vector<VkPipeline> replayPipelines;
std::vector<format::HandleId> capturePipelines;
};

struct VideoSessionKHRInfo : VulkanObjectInfo<VkVideoSessionKHR>
Expand Down
140 changes: 135 additions & 5 deletions framework/decode/vulkan_replay_consumer_base.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6401,9 +6401,14 @@ VkResult VulkanReplayConsumerBase::OverrideCreateRayTracingPipelinesKHR(

if (deferred_operation_info)
{
deferred_operation_info->join_state = VK_NOT_READY;
deferred_operation_info->pending_state = true;
deferred_operation_info->record_modified_create_infos.clear();
deferred_operation_info->record_modified_pgroups.clear();
deferred_operation_info->replayPipelines.resize(createInfoCount);
deferred_operation_info->capturePipelines.clear();
deferred_operation_info->capturePipelines.insert(deferred_operation_info->capturePipelines.end(),
&pPipelines->GetPointer()[0],
&pPipelines->GetPointer()[createInfoCount]);
}

if (device_info->property_feature_info.feature_rayTracingPipelineShaderGroupHandleCaptureReplay)
Expand Down Expand Up @@ -6462,13 +6467,52 @@ VkResult VulkanReplayConsumerBase::OverrideCreateRayTracingPipelinesKHR(
// Use modified shader group infos.
modified_create_infos[create_info_i].pGroups = modified_group_infos.data();
}

VkPipeline* created_pipelines = nullptr;

if (deferred_operation_info)
{
created_pipelines = deferred_operation_info->replayPipelines.data();
}
else
{
created_pipelines = out_pPipelines;
}

result = device_table->CreateRayTracingPipelinesKHR(device,
in_deferredOperation,
in_pipelineCache,
createInfoCount,
modified_create_infos.data(),
in_pAllocator,
out_pPipelines);
created_pipelines);

if ((result == VK_SUCCESS) || (result == VK_OPERATION_NOT_DEFERRED_KHR) ||
(result == VK_PIPELINE_COMPILE_REQUIRED_EXT))
{
// The above return values mean the command is not deferred and driver will finish all workload in current
// thread. Therefore the created pipelines can be read and copied to out_pPipelines which will be
// referenced later.
//
// Note:
// Some pipelines might actually fail creation if the return value is VK_PIPELINE_COMPILE_REQUIRED_EXT.
// These failed pipelines will generate VK_NULL_HANDLE.
//
// If the return value is VK_OPERATION_DEFERRED_KHR, it means the command is deferred, and thus pipeline
// creation is not finished. Subsequent handling will be done by
// vkDeferredOperationJoinKHR/vkGetDeferredOperationResultKHR after pipeline creation is finished.

if (deferred_operation_info)
{
memcpy(out_pPipelines, created_pipelines, createInfoCount * sizeof(VkPipeline));

// Eventhough vkCreateRayTracingPipelinesKHR was called with a valid deferred operation object, the
// driver may opt to not defer the command. In this case, set pending_state flag to false to skip
// vkDeferredOperationJoinKHR handling.
deferred_operation_info->pending_state = false;
}
}

if (deferred_operation_info)
{
deferred_operation_info->record_modified_create_infos = std::move(modified_create_infos);
Expand All @@ -6481,13 +6525,35 @@ VkResult VulkanReplayConsumerBase::OverrideCreateRayTracingPipelinesKHR(
"rayTracingPipelineShaderGroupHandleCaptureReplay feature for accurate capture and "
"replay. The replay device does not support this feature, so replay may fail.");

VkPipeline* created_pipelines = nullptr;

if (deferred_operation_info)
{
created_pipelines = deferred_operation_info->replayPipelines.data();
}
else
{
created_pipelines = out_pPipelines;
}

result = device_table->CreateRayTracingPipelinesKHR(device,
in_deferredOperation,
in_pipelineCache,
createInfoCount,
in_pCreateInfos,
in_pAllocator,
out_pPipelines);
created_pipelines);

if ((result == VK_SUCCESS) || (result == VK_OPERATION_NOT_DEFERRED_KHR) ||
(result == VK_PIPELINE_COMPILE_REQUIRED_EXT))
{

if (deferred_operation_info)
{
memcpy(out_pPipelines, created_pipelines, createInfoCount * sizeof(VkPipeline));
deferred_operation_info->pending_state = false;
}
}
}

return result;
Expand All @@ -6498,8 +6564,9 @@ VkResult VulkanReplayConsumerBase::OverrideDeferredOperationJoinKHR(PFN_vkDeferr
const DeviceInfo* device_info,
DeferredOperationKHRInfo* deferred_operation_info)
{
if (deferred_operation_info->join_state == VK_SUCCESS)
if (deferred_operation_info->pending_state == false)
{
// The deferred operation object has no deferred command or its deferred command has been finished.
return VK_SUCCESS;
}

Expand Down Expand Up @@ -6537,9 +6604,19 @@ VkResult VulkanReplayConsumerBase::OverrideDeferredOperationJoinKHR(PFN_vkDeferr
j.get();
}

deferred_operation_info->join_state = VK_SUCCESS;
AddHandles<PipelineInfo>(device_info->capture_id,
deferred_operation_info->capturePipelines.data(),
deferred_operation_info->capturePipelines.size(),
deferred_operation_info->replayPipelines.data(),
deferred_operation_info->replayPipelines.size(),
&VulkanObjectInfoTable::AddPipelineInfo);

deferred_operation_info->pending_state = false;
deferred_operation_info->record_modified_create_infos.clear();
deferred_operation_info->record_modified_pgroups.clear();
deferred_operation_info->capturePipelines.clear();
deferred_operation_info->replayPipelines.clear();

return VK_SUCCESS;
}

Expand Down Expand Up @@ -7347,5 +7424,58 @@ void VulkanReplayConsumerBase::Process_vkUpdateDescriptorSetWithTemplateKHR(cons
in_device, in_descriptorSet, in_descriptorUpdateTemplate, pData->GetPointer());
}

void VulkanReplayConsumerBase::Process_vkCreateRayTracingPipelinesKHR(
const ApiCallInfo& call_info,
VkResult returnValue,
format::HandleId device,
format::HandleId deferredOperation,
format::HandleId pipelineCache,
uint32_t createInfoCount,
StructPointerDecoder<Decoded_VkRayTracingPipelineCreateInfoKHR>* pCreateInfos,
StructPointerDecoder<Decoded_VkAllocationCallbacks>* pAllocator,
HandlePointerDecoder<VkPipeline>* pPipelines)
{
auto in_device = GetObjectInfoTable().GetDeviceInfo(device);
auto in_deferredOperation = GetObjectInfoTable().GetDeferredOperationKHRInfo(deferredOperation);
auto in_pipelineCache = GetObjectInfoTable().GetPipelineCacheInfo(pipelineCache);
MapStructArrayHandles(pCreateInfos->GetMetaStructPointer(), pCreateInfos->GetLength(), GetObjectInfoTable());

if (!pPipelines->IsNull())
{
pPipelines->SetHandleLength(createInfoCount);
}

std::vector<PipelineInfo> handle_info(createInfoCount);

for (size_t i = 0; i < createInfoCount; ++i)
{
pPipelines->SetConsumerData(i, &handle_info[i]);
}

VkResult replay_result =
OverrideCreateRayTracingPipelinesKHR(GetDeviceTable(in_device->handle)->CreateRayTracingPipelinesKHR,
returnValue,
in_device,
in_deferredOperation,
in_pipelineCache,
createInfoCount,
pCreateInfos,
pAllocator,
pPipelines);
CheckResult("vkCreateRayTracingPipelinesKHR", returnValue, replay_result, call_info);

if ((replay_result == VK_SUCCESS) || (replay_result == VK_OPERATION_NOT_DEFERRED_KHR) ||
(replay_result == VK_PIPELINE_COMPILE_REQUIRED_EXT))
{
AddHandles<PipelineInfo>(device,
pPipelines->GetPointer(),
pPipelines->GetLength(),
pPipelines->GetHandlePointer(),
createInfoCount,
std::move(handle_info),
&VulkanObjectInfoTable::AddPipelineInfo);
}
}

GFXRECON_END_NAMESPACE(decode)
GFXRECON_END_NAMESPACE(gfxrecon)
10 changes: 10 additions & 0 deletions framework/decode/vulkan_replay_consumer_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,16 @@ class VulkanReplayConsumerBase : public VulkanConsumer
format::HandleId descriptorSet,
format::HandleId descriptorUpdateTemplate,
DescriptorUpdateTemplateDecoder* pData) override;
virtual void Process_vkCreateRayTracingPipelinesKHR(
const ApiCallInfo& call_info,
VkResult returnValue,
format::HandleId device,
format::HandleId deferredOperation,
format::HandleId pipelineCache,
uint32_t createInfoCount,
StructPointerDecoder<Decoded_VkRayTracingPipelineCreateInfoKHR>* pCreateInfos,
StructPointerDecoder<Decoded_VkAllocationCallbacks>* pAllocator,
HandlePointerDecoder<VkPipeline>* pPipelines);

protected:
const VulkanObjectInfoTable& GetObjectInfoTable() const { return object_info_table_; }
Expand Down
57 changes: 40 additions & 17 deletions framework/encode/custom_vulkan_api_call_encoders.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
** Copyright (c) 2018-2020 Valve Corporation
** Copyright (c) 2018-2020 LunarG, Inc.
** Copyright (c) 2023 Advanced Micro Devices, Inc. All rights reserved.
**
** Permission is hereby granted, free of charge, to any person obtaining a
** copy of this software and associated documentation files (the "Software"),
Expand Down Expand Up @@ -652,24 +653,46 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateRayTracingPipelinesKHR(VkDevice
omit_output_data = true;
}

auto encoder = VulkanCaptureManager::Get()->BeginTrackedApiCallCapture(
format::ApiCallId::ApiCall_vkCreateRayTracingPipelinesKHR);
if (encoder)
auto device_wrapper = GetWrapper<DeviceWrapper>(device);
if (result != VK_OPERATION_DEFERRED_KHR)
{
encoder->EncodeHandleValue<DeviceWrapper>(device);
encoder->EncodeHandleValue<DeferredOperationKHRWrapper>(deferredOperation);
encoder->EncodeHandleValue<PipelineCacheWrapper>(pipelineCache);
encoder->EncodeUInt32Value(createInfoCount);
EncodeStructArray(encoder, pCreateInfos, createInfoCount);
EncodeStructPtr(encoder, pAllocator);
encoder->EncodeHandleArray<PipelineWrapper>(pPipelines, createInfoCount, omit_output_data);
encoder->EncodeEnumValue(result);
VulkanCaptureManager::Get()
->EndGroupCreateApiCallCapture<VkDevice,
VkDeferredOperationKHR,
PipelineWrapper,
VkRayTracingPipelineCreateInfoKHR>(
result, device, deferredOperation, createInfoCount, pPipelines, pCreateInfos);
// If the operation is not deferred by driver. or the system doesn't support
Copy link
Contributor

@bradgrantham-lunarg bradgrantham-lunarg Nov 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, this block ends up being written twice by the code in this branch. I don't think that's what we want for this solution. If there has to be a deferred operation read of the output variable, we probably need a MetaData command to write that value, and then replay should handle the case of postponing creation of the VkPipeline handles until that MetaData command is written.

I'm confused about what's happening here. Is the problem that the application creates an RT pipeline and gets VK_OPERATION_DEFERRED_KHR then joins the deferred operation until success, then uses the pipeline handles but GFXR somehow writes the operations out of order so the pipeline handles are used in replay before their creation is complete?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a test case where the application creates an RT pipeline and gets VK_OPERATION_DEFERRED_KHR then joins the deferred operation until success, then uses the pipeline handles but GFXR somehow writes the operations out of order so the pipeline handles are used in replay before their creation is complete?

If that's what's happening, then I think I need to understand how that could happen and we may need to introduce synchronization around the pipeline handles and anything using those pipeline handles, and not create this block twice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you describe in steps how the problem occurs so I can understand how this branch is expected to the issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The modified app is valid.

Yes, I think the hardware doesn't support the feature rayTracingPipelineShaderGroupHandleCaptureReplay, it triggers the issue that the above code block generates one more trace block. I'll add a commit for the issue. It may playback the trace of raytracingbasic as current dev branch. However, for this hardware, it's not guarantee playback always work, the reason is the following:

GFXR current framework requires the feature for capture/replay raytracing titles. It needs the related GPU address/ShaderGroupHandle capture/replay extension to reproduce these capture time GPU address/handle value during replay time. Without the feature (rayTracingPipelineShaderGroupHandleCaptureReplay) support, it's not guarantee for replaying raytracing trace file because shadergroup handle value may have different value during replay time, especially for complicated apps or real game, simple app and full trace file replay sometimes may work because the used GPU addresses/handles way less than real title.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// rayTracingPipelineShaderGroupHandleCaptureReplay, we don't need to delay writing the block of
// vkCreateRayTracingPipelinesKHR to the capture file.
//
// If the operation is deferred by driver and the system supports
// rayTracingPipelineShaderGroupHandleCaptureReplay, we will need to make sure opaque capture replay data for
// pipeline shader group handles are ready when calling vkCreateRayTracingPipelinesKHR during replay. However,
// opaque capture replay data for pipeline shader group handles needs pipeline creation to be finished so that
// vkGetRayTracingCaptureReplayShaderGroupHandlesKHR may be used. For this reason, we will delay the writing of
// vkCreateRayTracingPipelinesKHR block to capture file until the deferred operation is finished.

auto encoder = VulkanCaptureManager::Get()->BeginTrackedApiCallCapture(
format::ApiCallId::ApiCall_vkCreateRayTracingPipelinesKHR);
if (encoder)
{
encoder->EncodeHandleValue<DeviceWrapper>(device);
encoder->EncodeHandleValue<DeferredOperationKHRWrapper>(deferredOperation);
encoder->EncodeHandleValue<PipelineCacheWrapper>(pipelineCache);
encoder->EncodeUInt32Value(createInfoCount);
EncodeStructArray(encoder, pCreateInfos, createInfoCount);
EncodeStructPtr(encoder, pAllocator);
encoder->EncodeHandleArray<PipelineWrapper>(pPipelines, createInfoCount, omit_output_data);
encoder->EncodeEnumValue(result);
VulkanCaptureManager::Get()
->EndGroupCreateApiCallCapture<VkDevice,
VkDeferredOperationKHR,
PipelineWrapper,
VkRayTracingPipelineCreateInfoKHR>(
result, device, deferredOperation, createInfoCount, pPipelines, pCreateInfos);
}
}
else
{
GFXRECON_LOG_WARNING("The operation of vkCreateRayTracingPipelinesKHR call is deferred, so the writing of "
"vkCreateRayTracingPipelinesKHR block to capture file will be delayed until the deferred "
"operation (handle = 0x%" PRIx64 ") is finished.",
deferredOperation);
}

CustomEncoderPostCall<format::ApiCallId::ApiCall_vkCreateRayTracingPipelinesKHR>::Dispatch(
Expand Down
Loading