Skip to content

Commit 394c855

Browse files
committed
layers: Implement submit time tracker
This implements the first version of submit time tracker. Track timeline signals so postponed wait-before-signal submissions can be validated directly during QueueSubmit and SignalSemaphore calls. As a first step, move submit time image layout validation out of the queue thread and integrate it with this system
1 parent 78161e5 commit 394c855

16 files changed

Lines changed: 497 additions & 118 deletions

layers/core_checks/cc_queue.cpp

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,9 @@ bool CoreChecks::PreCallValidateQueueSubmit(VkQueue queue, uint32_t submitCount,
290290
chained_device_group_struct->commandBufferCount, submit.commandBufferCount);
291291
}
292292
}
293-
skip |= submit_time_tracker.ProcessSubmissionBatch(submit);
293+
// Perform submit time validation at the end.
294+
// If submit API is used incorrectly, we want those errors to be reported first
295+
skip |= submit_time_tracker.ProcessSubmitInfo(submit, queue, submit_loc);
294296
}
295297

296298
return skip;
@@ -450,6 +452,9 @@ bool CoreChecks::ValidateQueueSubmit2(VkQueue queue, uint32_t submitCount, const
450452
skip |= LogError("VUID-VkSubmitInfo2-commandBuffer-06010", queue, submit_loc,
451453
"has a suspended render pass instance that was not resumed.");
452454
}
455+
// Perform submit time validation at the end.
456+
// If submit API is used incorrectly, we want those errors to be reported first
457+
skip |= submit_time_tracker.ProcessSubmitInfo(submit, queue, submit_loc);
453458
}
454459

455460
return skip;
@@ -778,3 +783,30 @@ bool CoreChecks::PreCallValidateQueueBindSparse(VkQueue queue, uint32_t bindInfo
778783
}
779784
return skip;
780785
}
786+
787+
bool CoreChecks::ProcessSubmissionBatch(const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
788+
const Location& submit_loc) {
789+
bool skip = false;
790+
// Validate image layouts on the command buffer boundaries
791+
{
792+
vvl::unordered_map<const vvl::Image*, ImageLayoutMap> local_image_layout_map;
793+
for (const auto& cb : command_buffers) {
794+
if (cb) {
795+
auto cb_guard = cb->ReadLock();
796+
skip |= ValidateCmdBufImageLayouts(submit_loc, *cb, local_image_layout_map);
797+
}
798+
}
799+
}
800+
if (!skip) {
801+
for (const auto& cb : command_buffers) {
802+
if (cb) {
803+
auto cb_guard = cb->WriteLock();
804+
for (const vvl::CommandBuffer* secondary : cb->linked_command_buffers) {
805+
UpdateCmdBufImageLayouts(*secondary);
806+
}
807+
UpdateCmdBufImageLayouts(*cb);
808+
}
809+
}
810+
}
811+
return skip;
812+
}

layers/core_checks/cc_state_tracker.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1229,7 +1229,6 @@ void QueueSubState::PreSubmit(std::vector<vvl::QueueSubmission>& submissions) {
12291229

12301230
void QueueSubState::Retire(vvl::QueueSubmission& submission) {
12311231
queue_submission_validator_.Validate(submission);
1232-
queue_submission_validator_.Update(submission);
12331232

12341233
auto is_query_updated_after = [this](const QueryObject& query_object) {
12351234
auto guard = base.Lock();

layers/core_checks/cc_submit.cpp

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -34,24 +34,6 @@ static Location GetSignaledSemaphoreLocation(const Location& submit_loc, uint32_
3434
return submit_loc.dot(field, index);
3535
}
3636

37-
static bool FindLayouts(const vvl::Image& image_state, std::vector<VkImageLayout>& layouts) {
38-
if (!image_state.layout_map) {
39-
return false;
40-
}
41-
const auto& layout_map = *image_state.layout_map;
42-
auto guard = image_state.LayoutMapReadLock();
43-
44-
// TODO: Make this robust for >1 aspect mask. Now it will just say ignore potential errors in this case.
45-
if (layout_map.size() > image_state.GetArrayLayers() * image_state.GetMipLevels()) {
46-
return false;
47-
}
48-
49-
for (const auto& entry : layout_map) {
50-
layouts.emplace_back(entry.second);
51-
}
52-
return true;
53-
}
54-
5537
void QueueSubmissionValidator::Validate(const vvl::QueueSubmission& submission) const {
5638
// Ensure that timeline signals are monotonically increasing values
5739
for (uint32_t i = 0; i < (uint32_t)submission.signal_semaphores.size(); ++i) {
@@ -74,39 +56,4 @@ void QueueSubmissionValidator::Validate(const vvl::QueueSubmission& submission)
7456
core_checks.FormatHandle(signal.semaphore->VkHandle()).c_str(), signal.payload, current_payload);
7557
}
7658
}
77-
78-
// Validate image layouts on the command buffer boundaries
79-
{
80-
vvl::unordered_map<const vvl::Image*, ImageLayoutMap> local_image_layout_map;
81-
for (const vvl::CommandBufferSubmission& cb_submission : submission.cb_submissions) {
82-
auto cb_guard = cb_submission.cb->ReadLock();
83-
core_checks.ValidateCmdBufImageLayouts(submission.loc.Get(), *cb_submission.cb, local_image_layout_map);
84-
}
85-
}
86-
87-
// Check that image being presented has correct layout
88-
if (submission.swapchain) {
89-
std::vector<VkImageLayout> layouts;
90-
if (submission.swapchain_image && FindLayouts(*submission.swapchain_image, layouts)) {
91-
for (auto layout : layouts) {
92-
if (layout != VK_IMAGE_LAYOUT_PRESENT_SRC_KHR && layout != VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR) {
93-
core_checks.LogError(
94-
"VUID-VkPresentInfoKHR-pImageIndices-01430", submission.swapchain_image->Handle(), submission.loc.Get(),
95-
"images passed to present must be in layout VK_IMAGE_LAYOUT_PRESENT_SRC_KHR or "
96-
"VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR but %s is in %s.",
97-
core_checks.FormatHandle(submission.swapchain_image->Handle()).c_str(), string_VkImageLayout(layout));
98-
}
99-
}
100-
}
101-
}
102-
}
103-
104-
void QueueSubmissionValidator::Update(vvl::QueueSubmission& submission) {
105-
for (vvl::CommandBufferSubmission& cb_submission : submission.cb_submissions) {
106-
auto cb_guard = cb_submission.cb->WriteLock();
107-
for (const vvl::CommandBuffer* secondary : cb_submission.cb->linked_command_buffers) {
108-
core_checks.UpdateCmdBufImageLayouts(*secondary);
109-
}
110-
core_checks.UpdateCmdBufImageLayouts(*cb_submission.cb);
111-
}
11259
}

layers/core_checks/cc_submit.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
/* Copyright (c) 2025 The Khronos Group Inc.
2-
* Copyright (c) 2025 Valve Corporation
3-
* Copyright (c) 2025 LunarG, Inc.
1+
/* Copyright (c) 2026 The Khronos Group Inc.
2+
* Copyright (c) 2026 Valve Corporation
3+
* Copyright (c) 2026 LunarG, Inc.
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -36,5 +36,4 @@ struct QueueSubmissionValidator {
3636

3737
QueueSubmissionValidator(CoreChecks &core_checks) : core_checks(core_checks) {}
3838
void Validate(const vvl::QueueSubmission &submission) const;
39-
void Update(vvl::QueueSubmission &submission);
4039
};

layers/core_checks/cc_synchronization.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,26 @@ bool CoreChecks::PreCallValidateCreateSemaphore(VkDevice device, const VkSemapho
719719
return skip;
720720
}
721721

722+
void CoreChecks::PostCallRecordCreateSemaphore(VkDevice device, const VkSemaphoreCreateInfo* pCreateInfo,
723+
const VkAllocationCallbacks* pAllocator, VkSemaphore* pSemaphore,
724+
const RecordObject& record_obj) {
725+
if (record_obj.result != VK_SUCCESS) {
726+
return;
727+
}
728+
auto semaphore_state = Get<vvl::Semaphore>(*pSemaphore);
729+
if (semaphore_state && semaphore_state->type == VK_SEMAPHORE_TYPE_TIMELINE) {
730+
submit_time_tracker.OnCreateTimelineSemaphore(*pSemaphore, semaphore_state->initial_value);
731+
}
732+
}
733+
734+
void CoreChecks::PreCallRecordDestroySemaphore(VkDevice device, VkSemaphore semaphore, const VkAllocationCallbacks* pAllocator,
735+
const RecordObject& record_obj) {
736+
auto semaphore_state = Get<vvl::Semaphore>(semaphore);
737+
if (semaphore_state && semaphore_state->type == VK_SEMAPHORE_TYPE_TIMELINE) {
738+
submit_time_tracker.OnDestroyTimelineSemaphore(semaphore);
739+
}
740+
}
741+
722742
bool CoreChecks::PreCallValidateWaitSemaphoresKHR(VkDevice device, const VkSemaphoreWaitInfo* pWaitInfo, uint64_t timeout,
723743
const ErrorObject& error_obj) const {
724744
return PreCallValidateWaitSemaphores(device, pWaitInfo, timeout, error_obj);
@@ -1627,6 +1647,10 @@ bool CoreChecks::PreCallValidateSignalSemaphore(VkDevice device, const VkSemapho
16271647
"(%" PRIu64 ") exceeds limit regarding %s semaphore %s payload (%" PRIu64 ").", pSignalInfo->value,
16281648
FormatHandle(*semaphore_state).c_str(), payload_type, *far_away_payload);
16291649
}
1650+
1651+
// Perform submit time validation at the end.
1652+
// If signal semaphore API is used incorrectly, we want those errors to be reported first
1653+
skip |= submit_time_tracker.ProcessSignalSemaphore(*pSignalInfo);
16301654
return skip;
16311655
}
16321656

layers/core_checks/cc_wsi.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,6 +1257,10 @@ bool CoreChecks::PreCallValidateQueuePresentKHR(VkQueue queue, const VkPresentIn
12571257
skip |= ValidateSwapchainPresentFenceInfo(queue, *pPresentInfo, *swapchain_present_fence_info, present_info_loc);
12581258
}
12591259

1260+
// Perform submit time validation at the end.
1261+
// If present API is used incorrectly, we want those errors to be reported first
1262+
skip |= submit_time_tracker.ProcessPresent(*pPresentInfo, present_info_loc);
1263+
12601264
return skip;
12611265
}
12621266

@@ -1542,6 +1546,41 @@ bool CoreChecks::PreCallValidateWaitForPresent2KHR(VkDevice device, VkSwapchainK
15421546
return skip;
15431547
}
15441548

1549+
static bool FindLayouts(const vvl::Image& image_state, std::vector<VkImageLayout>& layouts) {
1550+
if (!image_state.layout_map) {
1551+
return false;
1552+
}
1553+
const auto& layout_map = *image_state.layout_map;
1554+
auto guard = image_state.LayoutMapReadLock();
1555+
1556+
// TODO: Make this robust for >1 aspect mask. Now it will just say ignore potential errors in this case.
1557+
if (layout_map.size() > image_state.GetArrayLayers() * image_state.GetMipLevels()) {
1558+
return false;
1559+
}
1560+
1561+
for (const auto& entry : layout_map) {
1562+
layouts.emplace_back(entry.second);
1563+
}
1564+
return true;
1565+
}
1566+
1567+
bool CoreChecks::ProcessPresentBatch(const vvl::Image& swapchain_image, const Location& present_info_loc) {
1568+
bool skip = false;
1569+
// Check that image being presented has correct layout
1570+
std::vector<VkImageLayout> layouts;
1571+
if (FindLayouts(swapchain_image, layouts)) {
1572+
for (auto layout : layouts) {
1573+
if (layout != VK_IMAGE_LAYOUT_PRESENT_SRC_KHR && layout != VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR) {
1574+
skip |= LogError("VUID-VkPresentInfoKHR-pImageIndices-01430", swapchain_image.Handle(), present_info_loc,
1575+
"images passed to present must be in layout VK_IMAGE_LAYOUT_PRESENT_SRC_KHR or "
1576+
"VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR but %s is in %s.",
1577+
FormatHandle(swapchain_image.Handle()).c_str(), string_VkImageLayout(layout));
1578+
}
1579+
}
1580+
}
1581+
return skip;
1582+
}
1583+
15451584
bool core::Instance::PreCallValidateDestroySurfaceKHR(VkInstance instance, VkSurfaceKHR surface,
15461585
const VkAllocationCallbacks* pAllocator, const ErrorObject& error_obj) const {
15471586
bool skip = false;

layers/core_checks/core_validation.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1107,7 +1107,6 @@ class CoreChecks : public vvl::DeviceProxy {
11071107
VkDataGraphPipelineSessionARM session,
11081108
const VkDataGraphPipelineDispatchInfoARM *pInfo,
11091109
const ErrorObject& error_obj) const override;
1110-
11111110
void PostCallRecordCreateImage(VkDevice device, const VkImageCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator,
11121111
VkImage* pImage, const RecordObject& record_obj) override;
11131112

@@ -1580,6 +1579,9 @@ class CoreChecks : public vvl::DeviceProxy {
15801579
const RecordObject& record_obj) override;
15811580
void PostCallRecordQueueSubmit2(VkQueue queue, uint32_t submitCount, const VkSubmitInfo2* pSubmits, VkFence fence,
15821581
const RecordObject& record_obj) override;
1582+
bool ProcessSubmissionBatch(const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
1583+
const Location& submit_loc) override;
1584+
bool ProcessPresentBatch(const vvl::Image& swapchain_image, const Location& present_info_loc) override;
15831585
bool IgnoreAllocationSize(const VkMemoryAllocateInfo& allocate_info) const;
15841586
bool HasExternalMemoryImportSupport(const vvl::Buffer& buffer, VkExternalMemoryHandleTypeFlagBits handle_type) const;
15851587
bool HasExternalMemoryImportSupport(const vvl::Image& image, VkExternalMemoryHandleTypeFlagBits handle_type) const;
@@ -1594,6 +1596,11 @@ class CoreChecks : public vvl::DeviceProxy {
15941596
bool PreCallValidateCreateSemaphore(VkDevice device, const VkSemaphoreCreateInfo* pCreateInfo,
15951597
const VkAllocationCallbacks* pAllocator, VkSemaphore* pSemaphore,
15961598
const ErrorObject& error_obj) const override;
1599+
void PostCallRecordCreateSemaphore(VkDevice device, const VkSemaphoreCreateInfo* pCreateInfo,
1600+
const VkAllocationCallbacks* pAllocator, VkSemaphore* pSemaphore,
1601+
const RecordObject& record_obj) override;
1602+
void PreCallRecordDestroySemaphore(VkDevice device, VkSemaphore semaphore, const VkAllocationCallbacks* pAllocator,
1603+
const RecordObject& record_obj) override;
15971604
bool PreCallValidateWaitSemaphores(VkDevice device, const VkSemaphoreWaitInfo* pWaitInfo, uint64_t timeout,
15981605
const ErrorObject& error_obj) const override;
15991606
bool PreCallValidateWaitSemaphoresKHR(VkDevice device, const VkSemaphoreWaitInfo* pWaitInfo, uint64_t timeout,

layers/state_tracker/state_tracker.h

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343

4444
namespace vvl {
4545
struct AllocateDescriptorSetsData;
46-
struct SubmissionBatch;
4746
class Fence;
4847
class DescriptorPool;
4948
class DescriptorSet;
@@ -2352,14 +2351,21 @@ class DeviceProxy : public vvl::BaseDevice {
23522351
virtual void Created(vvl::ShaderObject& state) {}
23532352
virtual void Created(vvl::Pipeline& state){};
23542353

2355-
// Validate submission batch and update state if necessary.
2356-
// Called by SubmitTimeTracker and protected by the mutex. Only one batch is processed at a time.
2354+
// Validate a submission batch and update state if needed.
2355+
// This call is protected by the global submit-time mutex.
23572356
//
2358-
// NOTE: Classic Validate/Record split made it a challenge to synchronize threaded queues,
2359-
// especialy when timeline signal resolves pending work on another queue.
2360-
// This became increasingly important after the spec allowed internally synchronized queues,
2361-
// meaning the same queue can be used from multiple threads.
2362-
virtual bool ProcessSubmissionBatch(SubmissionBatch& batch) { return false; }
2357+
// NOTE: The classic Validate/Record split made threaded queues difficult to synchronize,
2358+
// especially when a timeline signal resolves pending work on another queue.
2359+
// This became even more important after the spec allowed internally synchronized queues,
2360+
// which means the same queue can be used from multiple threads
2361+
virtual bool ProcessSubmissionBatch(const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
2362+
const Location& submit_loc) {
2363+
return false;
2364+
}
2365+
2366+
// Validate a submission batch and update state if needed.
2367+
// This call is protected by the global submit-time mutex
2368+
virtual bool ProcessPresentBatch(const vvl::Image& swapchain_image, const Location& present_info_loc) { return false; }
23632369

23642370
// callbacks for image layout validation, which is implemented in both core validation and gpu-av
23652371
// TODO - It would be nice to have a way to not need a duplicate copy in both CoreChecks and GPU-AV code

0 commit comments

Comments
 (0)