diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index dc7adc9411f..e7b4a2e8427 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -3499,19 +3499,6 @@ void CoreChecks::PostCallRecordQueueSubmit2KHR(VkQueue queue, uint32_t submitCou } } -bool CoreChecks::SemaphoreWasSignaled(VkSemaphore semaphore) const { - return AnyOf([semaphore](const QUEUE_STATE &queue_state) { - for (const auto &submission : queue_state.submissions) { - for (const auto &signal_semaphore : submission.signal_semaphores) { - if (signal_semaphore.semaphore->semaphore() == semaphore) { - return true; - } - } - } - return false; - }); -} - struct SemaphoreSubmitState { const CoreChecks *core; VkQueueFlags queue_flags; @@ -3523,8 +3510,7 @@ struct SemaphoreSubmitState { bool CannotWait(const SEMAPHORE_STATE &semaphore_state) const { auto semaphore = semaphore_state.semaphore(); - return unsignaled_semaphores.count(semaphore) || - (!(signaled_semaphores.count(semaphore)) && !(semaphore_state.signaled) && !core->SemaphoreWasSignaled(semaphore)); + return unsignaled_semaphores.count(semaphore) || (!signaled_semaphores.count(semaphore) && !semaphore_state.CanBeWaited()); } bool ValidateBinaryWait(const core_error::Location &loc, VkQueue queue, const SEMAPHORE_STATE &semaphore_state) { @@ -3532,43 +3518,38 @@ struct SemaphoreSubmitState { using sync_vuid_maps::GetQueueSubmitVUID; using sync_vuid_maps::SubmitError; auto semaphore = semaphore_state.semaphore(); - if ((semaphore_state.scope == kSyncScopeInternal || internal_semaphores.count(semaphore))) { + if ((semaphore_state.Scope() == kSyncScopeInternal || internal_semaphores.count(semaphore))) { if (CannotWait(semaphore_state)) { - auto error = IsExtEnabled(core->device_extensions.vk_khr_timeline_semaphore) - ? SubmitError::kTimelineCannotBeSignalled - : SubmitError::kBinaryCannotBeSignalled; - const auto &vuid = GetQueueSubmitVUID(loc, error); - LogObjectList objlist(semaphore); - objlist.add(queue); - skip |= core->LogError( - objlist, semaphore_state.scope == kSyncScopeInternal ? vuid : kVUID_Core_DrawState_QueueForwardProgress, - "%s Queue %s is waiting on semaphore (%s) that has no way to be signaled.", loc.Message().c_str(), - core->report_data->FormatHandle(queue).c_str(), core->report_data->FormatHandle(semaphore).c_str()); + auto last_op = semaphore_state.LastOp(); + if (last_op && last_op->IsWait()) { + auto other_queue = last_op->queue->Handle(); + const char *vuid = loc.function == core_error::Func::vkQueueSubmit ? "VUID-vkQueueSubmit-pWaitSemaphores-00068" + : "VUID-vkQueueSubmit2KHR-semaphore-03871"; + LogObjectList objlist(semaphore); + objlist.add(queue); + objlist.add(other_queue); + skip |= core->LogError(objlist, vuid, "%s Queue %s is already waiting on semaphore (%s).", + loc.Message().c_str(), core->report_data->FormatHandle(other_queue).c_str(), + core->report_data->FormatHandle(semaphore).c_str()); + } else { + auto error = IsExtEnabled(core->device_extensions.vk_khr_timeline_semaphore) + ? SubmitError::kTimelineCannotBeSignalled + : SubmitError::kBinaryCannotBeSignalled; + const auto &vuid = GetQueueSubmitVUID(loc, error); + LogObjectList objlist(semaphore); + objlist.add(queue); + skip |= core->LogError( + objlist, semaphore_state.Scope() == kSyncScopeInternal ? vuid : kVUID_Core_DrawState_QueueForwardProgress, + "%s Queue %s is waiting on semaphore (%s) that has no way to be signaled.", loc.Message().c_str(), + core->report_data->FormatHandle(queue).c_str(), core->report_data->FormatHandle(semaphore).c_str()); + } } else { signaled_semaphores.erase(semaphore); unsignaled_semaphores.insert(semaphore); } - } else if (semaphore_state.scope == kSyncScopeExternalTemporary) { + } else if (semaphore_state.Scope() == kSyncScopeExternalTemporary) { internal_semaphores.insert(semaphore); } - core->ForEach([loc, queue, semaphore, &skip, this](const QUEUE_STATE &q) { - if (q.Queue() != queue) { - for (const auto &cb : q.submissions) { - for (const auto &wait_semaphore : cb.wait_semaphores) { - if (wait_semaphore.semaphore->semaphore() == semaphore) { - const char *vuid = loc.function == core_error::Func::vkQueueSubmit - ? "VUID-vkQueueSubmit-pWaitSemaphores-00068" - : "VUID-vkQueueSubmit2KHR-semaphore-03871"; - LogObjectList objlist(semaphore); - objlist.add(queue); - skip |= core->LogError(objlist, vuid, "%s Queue %s is already waiting on semaphore (%s).", - loc.Message().c_str(), core->report_data->FormatHandle(q.Handle()).c_str(), - core->report_data->FormatHandle(semaphore).c_str()); - } - } - } - } - }); return skip; } @@ -3595,7 +3576,8 @@ struct SemaphoreSubmitState { bool CannotSignal(const SEMAPHORE_STATE &semaphore_state) const { auto semaphore = semaphore_state.semaphore(); - return signaled_semaphores.count(semaphore) || (!(unsignaled_semaphores.count(semaphore)) && semaphore_state.signaled); + return signaled_semaphores.count(semaphore) || + (!unsignaled_semaphores.count(semaphore) && !semaphore_state.CanBeSignaled()); } bool ValidateSignalSemaphore(const core_error::Location &loc, VkQueue queue, VkSemaphore semaphore, uint64_t value, @@ -3612,35 +3594,43 @@ struct SemaphoreSubmitState { } switch (semaphore_state->type) { case VK_SEMAPHORE_TYPE_BINARY: - if (semaphore_state->scope == kSyncScopeInternal || internal_semaphores.count(semaphore)) { - if (signaled_semaphores.count(semaphore) || - (!(unsignaled_semaphores.count(semaphore)) && semaphore_state->signaled)) { - objlist.add(semaphore_state->signaler.queue->Handle()); + if ((semaphore_state->Scope() == kSyncScopeInternal || internal_semaphores.count(semaphore))) { + if (CannotSignal(*semaphore_state)) { + VkQueue other_queue; + auto last_op = semaphore_state->LastOp(); + if (last_op) { + other_queue = last_op->queue ? last_op->queue->Queue() : VK_NULL_HANDLE; + } else { + auto completed = semaphore_state->Completed(); + other_queue = completed.queue ? completed.queue->Queue() : VK_NULL_HANDLE; + } + objlist.add(other_queue); skip |= core->LogError(objlist, kVUID_Core_DrawState_QueueForwardProgress, "%s is signaling %s (%s) that was previously " "signaled by %s but has not since been waited on by any queue.", loc.Message().c_str(), core->report_data->FormatHandle(queue).c_str(), core->report_data->FormatHandle(semaphore).c_str(), - core->report_data->FormatHandle(semaphore_state->signaler.queue->Handle()).c_str()); + core->report_data->FormatHandle(other_queue).c_str()); } else { unsignaled_semaphores.erase(semaphore); signaled_semaphores.insert(semaphore); } } break; - case VK_SEMAPHORE_TYPE_TIMELINE: - if (value <= semaphore_state->payload) { + case VK_SEMAPHORE_TYPE_TIMELINE: { + auto completed = semaphore_state->Completed(); + if (value <= completed.payload) { const auto &vuid = GetQueueSubmitVUID(loc, SubmitError::kTimelineSemSmallValue); skip |= core->LogError(objlist, vuid, "%s signal value (0x%" PRIx64 ") in %s must be greater than current timeline semaphore %s value (0x%" PRIx64 ")", - loc.Message().c_str(), semaphore_state->payload, - core->report_data->FormatHandle(queue).c_str(), + loc.Message().c_str(), completed.payload, core->report_data->FormatHandle(queue).c_str(), core->report_data->FormatHandle(semaphore).c_str(), value); } else { skip |= core->ValidateMaxTimelineSemaphoreValueDifference(loc, *semaphore_state, value); } break; + } default: break; } @@ -3720,6 +3710,7 @@ bool CoreChecks::ValidateSemaphoresForSubmit(SemaphoreSubmitState &state, VkQueu } return skip; } + bool CoreChecks::ValidateSemaphoresForSubmit(SemaphoreSubmitState &state, VkQueue queue, const VkSubmitInfo2KHR *submit, const Location &outer_loc) const { bool skip = false; @@ -3753,39 +3744,26 @@ bool CoreChecks::ValidateMaxTimelineSemaphoreValueDifference(const Location &loc if (semaphore_state.type != VK_SEMAPHORE_TYPE_TIMELINE) return false; auto semaphore = semaphore_state.Handle(); - uint64_t diff = TimelineDiff(value, semaphore_state.payload); - + auto completed = semaphore_state.Completed(); + uint64_t diff = TimelineDiff(value, completed.payload); if (diff > phys_dev_props_core12.maxTimelineSemaphoreValueDifference) { const auto &vuid = GetQueueSubmitVUID(loc, SubmitError::kTimelineSemMaxDiff); - skip |= LogError(semaphore, vuid, "%s value exceeds limit regarding current semaphore %s payload", loc.Message().c_str(), - report_data->FormatHandle(semaphore).c_str()); - } - - ForEach([&skip, loc, value, semaphore, this](const QUEUE_STATE &queue_state) { - for (const auto &submission : queue_state.submissions) { - for (const auto &signal_semaphore : submission.signal_semaphores) { - if (signal_semaphore.semaphore->Handle() == semaphore) { - uint64_t diff = TimelineDiff(value, signal_semaphore.payload); - if (diff > phys_dev_props_core12.maxTimelineSemaphoreValueDifference) { - const auto &vuid = GetQueueSubmitVUID(loc, SubmitError::kTimelineSemMaxDiff); - skip |= LogError(semaphore, vuid, "%s value exceeds limit regarding pending semaphore %s signal value", - loc.Message().c_str(), report_data->FormatHandle(semaphore).c_str()); - } - } - } - for (const auto &wait_semaphore : submission.wait_semaphores) { - if (wait_semaphore.semaphore->Handle() == semaphore) { - uint64_t diff = TimelineDiff(value, wait_semaphore.payload); - if (diff > phys_dev_props_core12.maxTimelineSemaphoreValueDifference) { - const auto &vuid = GetQueueSubmitVUID(loc, SubmitError::kTimelineSemMaxDiff); - skip |= LogError(semaphore, vuid, "%s value exceeds limit regarding pending semaphore %s wait value", - loc.Message().c_str(), report_data->FormatHandle(semaphore).c_str()); - } - } + skip |= + LogError(semaphore, vuid, "%s value (%" PRIu64 ") exceeds limit regarding current semaphore %s payload (%" PRIu64 ").", + loc.Message().c_str(), value, report_data->FormatHandle(semaphore).c_str(), completed.payload); + } else if (semaphore_state.HasPendingOps()) { + auto last_op = semaphore_state.LastOp(); + if (last_op && last_op->op_type != SEMAPHORE_STATE::kNone) { + diff = TimelineDiff(value, last_op->payload); + if (diff > phys_dev_props_core12.maxTimelineSemaphoreValueDifference) { + const auto &vuid = GetQueueSubmitVUID(loc, SubmitError::kTimelineSemMaxDiff); + skip |= LogError(semaphore, vuid, + "%s value (%" PRIu64 ") exceeds limit regarding pending semaphore %s %s value (%" PRIu64 ").", + loc.Message().c_str(), value, report_data->FormatHandle(semaphore).c_str(), + SEMAPHORE_STATE::OpTypeName(last_op->op_type), last_op->payload); } } - }); - + } return skip; } @@ -14687,8 +14665,6 @@ bool CoreChecks::PreCallValidateQueueBindSparse(VkQueue queue, uint32_t bindInfo const VkBindSparseInfo &bind_info = pBindInfo[bind_idx]; auto timeline_semaphore_submit_info = LvlFindInChain(pBindInfo->pNext); - std::vector semaphore_waits; - std::vector semaphore_signals; for (uint32_t i = 0; i < bind_info.waitSemaphoreCount; ++i) { VkSemaphore semaphore = bind_info.pWaitSemaphores[i]; const auto semaphore_state = Get(semaphore); @@ -14717,16 +14693,15 @@ bool CoreChecks::PreCallValidateQueueBindSparse(VkQueue queue, uint32_t bindInfo } break; case VK_SEMAPHORE_TYPE_BINARY: - if (semaphore_state->scope == kSyncScopeInternal || internal_semaphores.count(semaphore)) { + if ((semaphore_state->Scope() == kSyncScopeInternal || internal_semaphores.count(semaphore))) { if (unsignaled_semaphores.count(semaphore) || - (!(signaled_semaphores.count(semaphore)) && !(semaphore_state->signaled) && - !SemaphoreWasSignaled(semaphore))) { + (!signaled_semaphores.count(semaphore) && !semaphore_state->CanBeWaited())) { LogObjectList objlist(semaphore); objlist.add(queue); skip |= LogError(objlist, - semaphore_state->scope == kSyncScopeInternal ? vuid_error - : kVUID_Core_DrawState_QueueForwardProgress, + semaphore_state->Scope() == kSyncScopeInternal ? vuid_error + : kVUID_Core_DrawState_QueueForwardProgress, "vkQueueBindSparse(): Queue %s is waiting on pBindInfo[%u].pWaitSemaphores[%u] (%s) that " "has no way to be " "signaled.", @@ -14736,7 +14711,7 @@ bool CoreChecks::PreCallValidateQueueBindSparse(VkQueue queue, uint32_t bindInfo signaled_semaphores.erase(semaphore); unsignaled_semaphores.insert(semaphore); } - } else if (semaphore_state->scope == kSyncScopeExternalTemporary) { + } else if (semaphore_state->Scope() == kSyncScopeExternalTemporary) { internal_semaphores.insert(semaphore); } break; @@ -14766,14 +14741,14 @@ bool CoreChecks::PreCallValidateQueueBindSparse(VkQueue queue, uint32_t bindInfo "than pBindInfo[%u].signalSemaphoreCount (%u)", bind_idx, i, report_data->FormatHandle(semaphore).c_str(), timeline_semaphore_submit_info->signalSemaphoreValueCount, bind_idx, bind_info.signalSemaphoreCount); - } else if (timeline_semaphore_submit_info->pSignalSemaphoreValues[i] <= semaphore_state->payload) { + } else if (timeline_semaphore_submit_info->pSignalSemaphoreValues[i] <= semaphore_state->Completed().payload) { LogObjectList objlist(semaphore); objlist.add(queue); skip |= LogError(objlist, "VUID-VkBindSparseInfo-pSignalSemaphores-03249", "VkQueueBindSparse: signal value (0x%" PRIx64 ") in %s must be greater than current timeline semaphore %s value (0x%" PRIx64 ") in pBindInfo[%u].pSignalSemaphores[%u]", - semaphore_state->payload, report_data->FormatHandle(queue).c_str(), + semaphore_state->Completed().payload, report_data->FormatHandle(queue).c_str(), report_data->FormatHandle(semaphore).c_str(), timeline_semaphore_submit_info->pSignalSemaphoreValues[i], bind_idx, i); } else { @@ -14783,18 +14758,20 @@ bool CoreChecks::PreCallValidateQueueBindSparse(VkQueue queue, uint32_t bindInfo } break; case VK_SEMAPHORE_TYPE_BINARY: - if (semaphore_state->scope == kSyncScopeInternal) { + if (semaphore_state->Scope() == kSyncScopeInternal) { if (signaled_semaphores.count(semaphore) || - (!(unsignaled_semaphores.count(semaphore)) && semaphore_state->signaled)) { + (!(unsignaled_semaphores.count(semaphore)) && !semaphore_state->CanBeSignaled())) { + auto last_op = semaphore_state->LastOp(); + VkQueue other_queue = last_op && last_op->queue ? last_op->queue->Queue() : VK_NULL_HANDLE; LogObjectList objlist(semaphore); objlist.add(queue); - objlist.add(semaphore_state->signaler.queue->Handle()); + objlist.add(other_queue); skip |= LogError( objlist, kVUID_Core_DrawState_QueueForwardProgress, "vkQueueBindSparse(): %s is signaling pBindInfo[%u].pSignalSemaphores[%u] (%s) that was " "previously signaled by %s but has not since been waited on by any queue.", report_data->FormatHandle(queue).c_str(), bind_idx, i, report_data->FormatHandle(semaphore).c_str(), - report_data->FormatHandle(semaphore_state->signaler.queue->Handle()).c_str()); + report_data->FormatHandle(other_queue).c_str()); } else { unsignaled_semaphores.erase(semaphore); signaled_semaphores.insert(semaphore); @@ -14918,34 +14895,31 @@ bool CoreChecks::ValidateSignalSemaphore(VkDevice device, const VkSemaphoreSigna } if (semaphore_state->type != VK_SEMAPHORE_TYPE_TIMELINE) { skip |= LogError(pSignalInfo->semaphore, "VUID-VkSemaphoreSignalInfo-semaphore-03257", - "%s(): semaphore %s must be of VK_SEMAPHORE_TYPE_TIMELINE type", api_name, + "%s(): semaphore %s must be of VK_SEMAPHORE_TYPE_TIMELINE type.", api_name, report_data->FormatHandle(pSignalInfo->semaphore).c_str()); return skip; } - if (semaphore_state->payload >= pSignalInfo->value) { + + auto completed = semaphore_state->Completed(); + if (completed.payload >= pSignalInfo->value) { skip |= LogError(pSignalInfo->semaphore, "VUID-VkSemaphoreSignalInfo-value-03258", - "%s(): value must be greater than current semaphore %s value", api_name, - report_data->FormatHandle(pSignalInfo->semaphore).c_str()); - } - ForEach([this, &skip, api_name, pSignalInfo](const QUEUE_STATE &queue_state) { - for (const auto &submission : queue_state.submissions) { - for (const auto &signal_semaphore : submission.signal_semaphores) { - if (signal_semaphore.semaphore->semaphore() == pSignalInfo->semaphore && - pSignalInfo->value >= signal_semaphore.payload) { - skip |= LogError(pSignalInfo->semaphore, "VUID-VkSemaphoreSignalInfo-value-03259", - "%s(): value must be greater than value of pending signal operation " - "for semaphore %s", - api_name, report_data->FormatHandle(pSignalInfo->semaphore).c_str()); - } - } + "%s(): value (%" PRIu64 ") must be greater than current semaphore %s value (%" PRIu64 ").", api_name, + pSignalInfo->value, report_data->FormatHandle(pSignalInfo->semaphore).c_str(), completed.payload); + return skip; + } else if (semaphore_state->HasPendingOps()) { + // look back for the last signal operation, but there could be pending waits with higher payloads behind it. + auto last_op = semaphore_state->LastOp([](const SEMAPHORE_STATE::SemOp &op) { return op.IsSignal(); }); + if (last_op && pSignalInfo->value >= last_op->payload) { + skip |= LogError( + pSignalInfo->semaphore, "VUID-VkSemaphoreSignalInfo-value-03259", + "%s(): value (%" PRIu64 ") must be less than value of any pending signal operation (%" PRIu64 ") for semaphore %s.", + api_name, pSignalInfo->value, last_op->payload, report_data->FormatHandle(pSignalInfo->semaphore).c_str()); } - }); - + } if (!skip) { Location loc(Func::vkSignalSemaphore, Struct::VkSemaphoreSignalInfo, Field::value); skip |= ValidateMaxTimelineSemaphoreValueDifference(loc, *semaphore_state, pSignalInfo->value); } - return skip; } @@ -15480,7 +15454,7 @@ bool CoreChecks::PreCallValidateQueuePresentKHR(VkQueue queue, const VkPresentIn "vkQueuePresentKHR: pWaitSemaphores[%u] (%s) is not a VK_SEMAPHORE_TYPE_BINARY", i, report_data->FormatHandle(pPresentInfo->pWaitSemaphores[i]).c_str()); } - if (semaphore_state && !semaphore_state->signaled && !SemaphoreWasSignaled(pPresentInfo->pWaitSemaphores[i])) { + if (semaphore_state && !semaphore_state->CanBeWaited()) { LogObjectList objlist(queue); objlist.add(pPresentInfo->pWaitSemaphores[i]); skip |= LogError(objlist, "VUID-vkQueuePresentKHR-pWaitSemaphores-03268", @@ -15685,12 +15659,19 @@ bool CoreChecks::ValidateAcquireNextImage(VkDevice device, const AcquireVersion if (semaphore_state->type != VK_SEMAPHORE_TYPE_BINARY) { skip |= LogError(semaphore, semaphore_type_vuid, "%s: %s is not a VK_SEMAPHORE_TYPE_BINARY", func_name, report_data->FormatHandle(semaphore).c_str()); - return skip; - } - if (semaphore_state->scope == kSyncScopeInternal && semaphore_state->signaled) { - const char *vuid = version == ACQUIRE_VERSION_2 ? "VUID-VkAcquireNextImageInfoKHR-semaphore-01288" - : "VUID-vkAcquireNextImageKHR-semaphore-01286"; - skip |= LogError(semaphore, vuid, "%s: Semaphore must not be currently signaled or in a wait state.", func_name); + } else if (semaphore_state->Scope() == kSyncScopeInternal) { + auto last_op = semaphore_state->LastOp(); + // For vkQueuePresentImage(), we don't currently have a good way to determine when the wait by the + // implementation has completed. + if (last_op && last_op->op_type != SEMAPHORE_STATE::kBinaryPresent) { + const char *vuid = version == ACQUIRE_VERSION_2 ? "VUID-VkAcquireNextImageInfoKHR-semaphore-01781" + : "VUID-vkAcquireNextImageKHR-semaphore-01779"; + skip |= LogError(semaphore, vuid, "%s: Semaphore has pending wait or signal operations.", func_name); + } else if (!semaphore_state->Completed().CanBeSignaled()) { + const char *vuid = version == ACQUIRE_VERSION_2 ? "VUID-VkAcquireNextImageInfoKHR-semaphore-01288" + : "VUID-vkAcquireNextImageKHR-semaphore-01286"; + skip |= LogError(semaphore, vuid, "%s: Semaphore must not be currently signaled.", func_name); + } } } diff --git a/layers/core_validation.h b/layers/core_validation.h index 42647f370b5..11bfe4f8de8 100644 --- a/layers/core_validation.h +++ b/layers/core_validation.h @@ -163,7 +163,6 @@ class CoreChecks : public ValidationStateTracker { bool CheckCommandBufferInFlight(const CMD_BUFFER_STATE* cb_node, const char* action, const char* error_code) const; void StoreMemRanges(VkDeviceMemory mem, VkDeviceSize offset, VkDeviceSize size); bool ValidateIdleDescriptorSet(VkDescriptorSet set, const char* func_str) const; - bool SemaphoreWasSignaled(VkSemaphore semaphore) const; bool ValidatePipelineLocked(std::vector> const& pPipelines, int pipelineIndex) const; bool ValidatePipelineUnlocked(const PIPELINE_STATE* pPipeline, uint32_t pipelineIndex) const; bool ValidImageBufferQueue(const CMD_BUFFER_STATE* cb_node, const VulkanTypedHandle& object, uint32_t queueFamilyIndex, diff --git a/layers/queue_state.cpp b/layers/queue_state.cpp index cacd09f532a..42dedd845c8 100644 --- a/layers/queue_state.cpp +++ b/layers/queue_state.cpp @@ -28,10 +28,9 @@ #include "cmd_buffer_state.h" #include "state_tracker.h" -uint64_t QUEUE_STATE::Submit(CB_SUBMISSION &&submission) { - const uint64_t next_seq = seq + submissions.size() + 1; - bool retire_early = false; +using SemOp = SEMAPHORE_STATE::SemOp; +uint64_t QUEUE_STATE::Submit(CB_SUBMISSION &&submission) { for (auto &cb_node : submission.cbs) { for (auto *secondary_cmd_buffer : cb_node->linkedCommandBuffers) { secondary_cmd_buffer->IncrementResources(); @@ -42,34 +41,17 @@ uint64_t QUEUE_STATE::Submit(CB_SUBMISSION &&submission) { cb_node->Submit(submission.perf_submit_pass); } + const uint64_t next_seq = seq_ + submissions.size() + 1; + bool retire_early = false; for (auto &wait : submission.wait_semaphores) { - switch (wait.semaphore->scope) { - case kSyncScopeInternal: - if (wait.semaphore->type == VK_SEMAPHORE_TYPE_BINARY) { - wait.semaphore->signaled = false; - } - break; - case kSyncScopeExternalTemporary: - wait.semaphore->scope = kSyncScopeInternal; - break; - default: - break; - } - wait.seq = next_seq; + wait.semaphore->EnqueueWait(this, next_seq, wait.payload); wait.semaphore->BeginUse(); } for (auto &signal : submission.signal_semaphores) { - if (signal.semaphore->scope == kSyncScopeInternal) { - signal.semaphore->signaler.queue = this; - signal.semaphore->signaler.seq = next_seq; - if (signal.semaphore->type == VK_SEMAPHORE_TYPE_BINARY) { - signal.semaphore->signaled = true; - } - } else { + if (signal.semaphore->EnqueueSignal(this, next_seq, signal.payload)) { retire_early = true; } - signal.seq = next_seq; signal.semaphore->BeginUse(); } @@ -84,56 +66,60 @@ uint64_t QUEUE_STATE::Submit(CB_SUBMISSION &&submission) { return retire_early ? next_seq : 0; } -void QUEUE_STATE::Retire(uint64_t next_seq) { - layer_data::unordered_map other_queue_seqs; - layer_data::unordered_map, uint64_t> timeline_semaphore_counters; +static void MergeResults(SEMAPHORE_STATE::RetireResult &results, const SEMAPHORE_STATE::RetireResult &sem_result) { + for (auto &entry : sem_result) { + auto &last_seq = results[entry.first]; + last_seq = std::max(last_seq, entry.second); + } +} + +layer_data::optional QUEUE_STATE::NextSubmission(uint64_t until_seq) { + layer_data::optional result; + if (seq_ < until_seq && !submissions.empty()) { + result.emplace(std::move(submissions.front())); + submissions.pop_front(); + seq_++; + } + return result; +} + +void QUEUE_STATE::Retire(uint64_t until_seq) { + SEMAPHORE_STATE::RetireResult other_queue_seqs; + + layer_data::optional submission; // Roll this queue forward, one submission at a time. - while (seq < next_seq) { - auto &submission = submissions.front(); - for (auto &wait : submission.wait_semaphores) { - if (wait.semaphore->type == VK_SEMAPHORE_TYPE_TIMELINE) { - auto &last_counter = timeline_semaphore_counters[wait.semaphore]; - last_counter = std::max(last_counter, wait.payload); - wait.semaphore->payload = std::max(wait.semaphore->payload, wait.payload); - } else if (wait.semaphore->signaler.queue) { - auto &last_seq = other_queue_seqs[wait.semaphore->signaler.queue]; - last_seq = std::max(last_seq, wait.semaphore->signaler.seq); - } // else this is an external semaphore + while ((submission = NextSubmission(until_seq))) { + for (auto &wait : submission->wait_semaphores) { + auto result = wait.semaphore->Retire(this, wait.payload); + MergeResults(other_queue_seqs, result); wait.semaphore->EndUse(); } - - for (auto &signal : submission.signal_semaphores) { - if (signal.semaphore->type == VK_SEMAPHORE_TYPE_TIMELINE && signal.semaphore->payload < signal.payload) { - signal.semaphore->payload = signal.payload; - } + for (auto &signal : submission->signal_semaphores) { + auto result = signal.semaphore->Retire(this, signal.payload); + MergeResults(other_queue_seqs, result); signal.semaphore->EndUse(); } - for (auto &cb_node : submission.cbs) { + for (auto &cb_node : submission->cbs) { for (auto *secondary_cmd_buffer : cb_node->linkedCommandBuffers) { - secondary_cmd_buffer->Retire(submission.perf_submit_pass); + secondary_cmd_buffer->Retire(submission->perf_submit_pass); } - cb_node->Retire(submission.perf_submit_pass); + cb_node->Retire(submission->perf_submit_pass); cb_node->EndUse(); } - if (submission.fence) { - submission.fence->Retire(false); - submission.fence->EndUse(); + if (submission->fence) { + submission->fence->Retire(false); + submission->fence->EndUse(); } - submissions.pop_front(); - seq++; } // Roll other queues forward to the highest seq we saw a wait for for (const auto &qs : other_queue_seqs) { - qs.first->Retire(qs.second); - } - - // Roll other semaphores forward to the highest seq we saw a wait for - for (const auto &sc : timeline_semaphore_counters) { - sc.first->Retire(sc.second); + if (qs.first != this) { + qs.first->Retire(qs.second); + } } } @@ -189,20 +175,112 @@ void FENCE_STATE::Export(VkExternalFenceHandleTypeFlagBits handle_type) { } } -void SEMAPHORE_STATE::Retire(uint64_t until_payload) { - if (signaler.queue) { - uint64_t max_seq = 0; - for (const auto &submission : signaler.queue->submissions) { - for (const auto &signal : submission.signal_semaphores) { - if (signal.semaphore.get() == this) { - if (signal.payload <= until_payload && signal.seq > max_seq) { - max_seq = std::max(max_seq, signal.seq); - } - } - } +bool SEMAPHORE_STATE::EnqueueSignal(QUEUE_STATE *queue, uint64_t queue_seq, uint64_t &payload) { + if (scope_ != kSyncScopeInternal) { + return true; // retire early + } + if (type == VK_SEMAPHORE_TYPE_BINARY) { + payload = next_payload_++; + } + operations_.insert(SemOp{kSignal, queue, queue_seq, payload}); + return false; +} + +void SEMAPHORE_STATE::EnqueueWait(QUEUE_STATE *queue, uint64_t queue_seq, uint64_t &payload) { + switch (scope_) { + case kSyncScopeExternalTemporary: + scope_ = kSyncScopeInternal; + break; + default: + break; + } + if (type == VK_SEMAPHORE_TYPE_BINARY) { + payload = next_payload_++; + } + operations_.insert(SemOp{kWait, queue, queue_seq, payload}); +} + +void SEMAPHORE_STATE::EnqueueAcquire() { + assert(type == VK_SEMAPHORE_TYPE_BINARY); + operations_.insert(SemOp{kBinaryAcquire, nullptr, next_payload_++}); +} + +void SEMAPHORE_STATE::EnqueuePresent(QUEUE_STATE *queue) { + assert(type == VK_SEMAPHORE_TYPE_BINARY); + operations_.insert(SemOp{kBinaryPresent, queue, 0, next_payload_++}); +} + +layer_data::optional SEMAPHORE_STATE::LastOp(std::function filter) const { + layer_data::optional result; + + for (auto pos = operations_.rbegin(); pos != operations_.rend(); ++pos) { + if (!filter || filter(*pos)) { + result.emplace(*pos); + break; } - if (max_seq) { - signaler.queue->Retire(max_seq); + } + return result; +} + +bool SEMAPHORE_STATE::CanBeSignaled() const { + if (type == VK_SEMAPHORE_TYPE_TIMELINE) { + return true; + } + auto op = LastOp(); + return op ? op->CanBeSignaled() : completed_.CanBeSignaled(); +} + +bool SEMAPHORE_STATE::CanBeWaited() const { + if (type == VK_SEMAPHORE_TYPE_TIMELINE) { + return true; + } + auto op = LastOp(); + if (op) { + return op->op_type == kSignal || op->op_type == kBinaryAcquire; + } + return completed_.op_type == kSignal || completed_.op_type == kBinaryAcquire; +} + +SEMAPHORE_STATE::RetireResult SEMAPHORE_STATE::Retire(QUEUE_STATE *queue, uint64_t payload) { + RetireResult result; + + while (!operations_.empty() && operations_.begin()->payload <= payload) { + completed_ = *operations_.begin(); + operations_.erase(operations_.begin()); + // Note: even though presentation is directed to a queue, there is no direct ordering between QP and subsequent work, + // so QP (and its semaphore waits) /never/ participate in any completion proof. Likewise, Acquire is not associated + // with a queue. + if (completed_.op_type != kBinaryAcquire && completed_.op_type != kBinaryPresent) { + auto &last_seq = result[completed_.queue]; + last_seq = std::max(last_seq, completed_.seq); } } + return result; +} + +void SEMAPHORE_STATE::RetireTimeline(uint64_t payload) { + if (type == VK_SEMAPHORE_TYPE_TIMELINE) { + auto results = Retire(nullptr, payload); + for (auto &entry : results) { + entry.first->Retire(entry.second); + } + } +} + +void SEMAPHORE_STATE::Import(VkExternalSemaphoreHandleTypeFlagBits handle_type, VkSemaphoreImportFlags flags) { + if (scope_ != kSyncScopeExternalPermanent) { + if ((handle_type == VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT || flags & VK_SEMAPHORE_IMPORT_TEMPORARY_BIT) && + scope_ == kSyncScopeInternal) { + scope_ = kSyncScopeExternalTemporary; + } else { + scope_ = kSyncScopeExternalPermanent; + } + } +} + +void SEMAPHORE_STATE::Export(VkExternalSemaphoreHandleTypeFlagBits handle_type) { + if (handle_type != VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT) { + // Cannot track semaphore state once it is exported, except for Sync FD handle types which have copy transference + scope_ = kSyncScopeExternalPermanent; + } } diff --git a/layers/queue_state.h b/layers/queue_state.h index 7066b07ae32..d2fc8346f94 100644 --- a/layers/queue_state.h +++ b/layers/queue_state.h @@ -26,8 +26,9 @@ */ #pragma once #include "base_node.h" -#include #include +#include +#include class CMD_BUFFER_STATE; class QUEUE_STATE; @@ -40,11 +41,6 @@ enum SyncScope { enum FENCE_STATUS { FENCE_UNSIGNALED, FENCE_INFLIGHT, FENCE_RETIRED }; -struct QUEUE_SIGNALER { - QUEUE_STATE *queue{nullptr}; - uint64_t seq{0}; -}; - class FENCE_STATE : public REFCOUNTED_NODE { public: // Default constructor @@ -82,53 +78,124 @@ class FENCE_STATE : public REFCOUNTED_NODE { class SEMAPHORE_STATE : public REFCOUNTED_NODE { public: - QUEUE_SIGNALER signaler; - bool signaled{false}; - SyncScope scope{kSyncScopeInternal}; - const VkSemaphoreType type; - uint64_t payload; + // possible payload values for binary semaphore + enum OpType { + kNone, + kWait, + kSignal, + kBinaryAcquire, + kBinaryPresent, + }; + static inline const char *OpTypeName(OpType t) { + switch (t) { + case kWait: + return "wait"; + case kSignal: + return "signal"; + case kBinaryAcquire: + return "acquire"; + case kBinaryPresent: + return "present"; + case kNone: + default: + return "NONE"; + } + } + + struct SemOp { + // NOTE: c++11 doesn't allow aggregate initialization and default member + // initializers in the same strict. This limitation is removed in c++14 + OpType op_type; + QUEUE_STATE *queue; + uint64_t seq; + uint64_t payload; + + bool operator<(const SemOp &rhs) const { return payload < rhs.payload; } + + bool IsWait() const { return op_type == kWait || op_type == kBinaryPresent; } + bool IsSignal() const { return op_type == kSignal; } + + // NOTE: Present semaphores are waited on by the implementation, not queue operations. We do not yet + // have a good way to figure out when this wait completes, so we must assume they are safe to re-use + bool CanBeSignaled() const { return op_type == kNone || op_type == kWait || op_type == kBinaryPresent; } + bool CanBeWaited() const { return op_type == kSignal || op_type == kBinaryAcquire; } + }; SEMAPHORE_STATE(VkSemaphore sem, const VkSemaphoreTypeCreateInfo *type_create_info) : REFCOUNTED_NODE(sem, kVulkanObjectTypeSemaphore), type(type_create_info ? type_create_info->semaphoreType : VK_SEMAPHORE_TYPE_BINARY), - payload(type_create_info ? type_create_info->initialValue : 0) {} + completed_{kNone, nullptr, 0, type_create_info ? type_create_info->initialValue : 0}, + next_payload_(completed_.payload + 1) {} VkSemaphore semaphore() const { return handle_.Cast(); } + SyncScope Scope() const { return scope_; } + // this is the most recently completed operation + SemOp Completed() const { return completed_; } - void Retire(uint64_t next_seq); -}; + // Enqueue a semaphore operation. For binary semaphores, the payload value is generated and + // returned, so that every semaphore operation has a unique value. + bool EnqueueSignal(QUEUE_STATE *queue, uint64_t queue_seq, uint64_t &payload); + void EnqueueWait(QUEUE_STATE *queue, uint64_t queue_seq, uint64_t &payload); -struct SEMAPHORE_WAIT { - std::shared_ptr semaphore; - uint64_t payload{0}; - uint64_t seq{0}; -}; + // Binary only special cases enqueue functions + void EnqueueAcquire(); + void EnqueuePresent(QUEUE_STATE *queue); + + // Remove completed operations and return highest sequence numbers for all affected queues + using RetireResult = layer_data::unordered_map; + RetireResult Retire(QUEUE_STATE *queue, uint64_t payload); + + // Helper for retiring timeline semaphores and then retiring all queues using the semaphore + void RetireTimeline(uint64_t payload); + + // look for most recent / highest payload operation that matches + layer_data::optional LastOp(std::function filter = nullptr) const; + + bool CanBeSignaled() const; + bool CanBeWaited() const; + bool HasPendingOps() const { return !operations_.empty(); } + + void Import(VkExternalSemaphoreHandleTypeFlagBits handle_type, VkSemaphoreImportFlags flags); + void Export(VkExternalSemaphoreHandleTypeFlagBits handle_type); -struct SEMAPHORE_SIGNAL { - std::shared_ptr semaphore; - uint64_t payload{0}; - uint64_t seq{0}; + const VkSemaphoreType type; + + private: + SyncScope scope_{kSyncScopeInternal}; + // the most recently completed operation + SemOp completed_{}; + // next payload value for binary semaphore operations + uint64_t next_payload_; + + // Set of pending operations ordered by payload. This must be a multiset because + // timeline operations can be added in any order and multiple operations + // can use the same payload value. + std::multiset operations_; }; struct CB_SUBMISSION { + struct SemaphoreInfo { + std::shared_ptr semaphore; + uint64_t payload{0}; + }; + std::vector> cbs; - std::vector wait_semaphores; - std::vector signal_semaphores; + std::vector wait_semaphores; + std::vector signal_semaphores; std::shared_ptr fence; uint32_t perf_submit_pass{0}; void AddCommandBuffer(std::shared_ptr &&cb_node) { cbs.emplace_back(std::move(cb_node)); } void AddSignalSemaphore(std::shared_ptr &&semaphore_state, uint64_t value) { - SEMAPHORE_SIGNAL signal; + SemaphoreInfo signal; signal.semaphore = std::move(semaphore_state); signal.payload = value; - signal.seq = 0; signal_semaphores.emplace_back(std::move(signal)); } void AddWaitSemaphore(std::shared_ptr &&semaphore_state, uint64_t value) { - SEMAPHORE_WAIT wait; + SemaphoreInfo wait; wait.semaphore = std::move(semaphore_state); wait.payload = value; wait_semaphores.emplace_back(std::move(wait)); @@ -139,19 +206,22 @@ struct CB_SUBMISSION { class QUEUE_STATE : public BASE_NODE { public: - const uint32_t queueFamilyIndex; - const VkDeviceQueueCreateFlags flags; - - uint64_t seq; - std::deque submissions; - QUEUE_STATE(VkQueue q, uint32_t index, VkDeviceQueueCreateFlags flags) - : BASE_NODE(q, kVulkanObjectTypeQueue), queueFamilyIndex(index), flags(flags), seq(0) {} + : BASE_NODE(q, kVulkanObjectTypeQueue), queueFamilyIndex(index), flags(flags), seq_(0) {} VkQueue Queue() const { return handle_.Cast(); } uint64_t Submit(CB_SUBMISSION &&submission); - void Retire(uint64_t next_seq); - void Retire() { Retire(seq + submissions.size()); } + void Retire(uint64_t until_seq = UINT64_MAX); + + const uint32_t queueFamilyIndex; + const VkDeviceQueueCreateFlags flags; + + std::deque submissions; + + private: + layer_data::optional NextSubmission(uint64_t until_seq); + + uint64_t seq_; }; diff --git a/layers/state_tracker.cpp b/layers/state_tracker.cpp index 8b6c929ecb2..f6c152f78f4 100644 --- a/layers/state_tracker.cpp +++ b/layers/state_tracker.cpp @@ -1251,7 +1251,7 @@ void ValidationStateTracker::PostCallRecordQueueSubmit(VkQueue queue, uint32_t s const VkSubmitInfo *submit = &pSubmits[submit_idx]; auto *timeline_semaphore_submit = LvlFindInChain(submit->pNext); for (uint32_t i = 0; i < submit->waitSemaphoreCount; ++i) { - uint64_t value = 0; + uint64_t value{0}; if (timeline_semaphore_submit && timeline_semaphore_submit->pWaitSemaphoreValues != nullptr && (i < timeline_semaphore_submit->waitSemaphoreValueCount)) { value = timeline_semaphore_submit->pWaitSemaphoreValues[i]; @@ -1260,7 +1260,7 @@ void ValidationStateTracker::PostCallRecordQueueSubmit(VkQueue queue, uint32_t s } for (uint32_t i = 0; i < submit->signalSemaphoreCount; ++i) { - uint64_t value = 0; + uint64_t value{0}; if (timeline_semaphore_submit && timeline_semaphore_submit->pSignalSemaphoreValues != nullptr && (i < timeline_semaphore_submit->signalSemaphoreValueCount)) { value = timeline_semaphore_submit->pSignalSemaphoreValues[i]; @@ -1438,21 +1438,18 @@ void ValidationStateTracker::PostCallRecordCreateSemaphore(VkDevice device, cons void ValidationStateTracker::RecordImportSemaphoreState(VkSemaphore semaphore, VkExternalSemaphoreHandleTypeFlagBits handle_type, VkSemaphoreImportFlags flags) { - auto sema_node = Get(semaphore); - if (sema_node && sema_node->scope != kSyncScopeExternalPermanent) { - if ((handle_type == VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT || flags & VK_SEMAPHORE_IMPORT_TEMPORARY_BIT) && - sema_node->scope == kSyncScopeInternal) { - sema_node->scope = kSyncScopeExternalTemporary; - } else { - sema_node->scope = kSyncScopeExternalPermanent; - } + auto semaphore_state = Get(semaphore); + if (semaphore_state) { + semaphore_state->Import(handle_type, flags); } } void ValidationStateTracker::PostCallRecordSignalSemaphoreKHR(VkDevice device, const VkSemaphoreSignalInfo *pSignalInfo, VkResult result) { auto semaphore_state = Get(pSignalInfo->semaphore); - semaphore_state->payload = pSignalInfo->value; + if (semaphore_state) { + semaphore_state->RetireTimeline(pSignalInfo->value); + } } void ValidationStateTracker::RecordMappedMemory(VkDeviceMemory mem, VkDeviceSize offset, VkDeviceSize size, void **ppData) { @@ -1486,10 +1483,14 @@ void ValidationStateTracker::RecordWaitSemaphores(VkDevice device, const VkSemap VkResult result) { if (VK_SUCCESS != result) return; - for (uint32_t i = 0; i < pWaitInfo->semaphoreCount; i++) { - auto semaphore_state = Get(pWaitInfo->pSemaphores[i]); - if (semaphore_state) { - semaphore_state->Retire(pWaitInfo->pValues[i]); + // Same logic as vkWaitForFences(). If some semaphores are not signaled, we will get their status when + // the application calls vkGetSemaphoreCounterValue() on each of them. + if ((pWaitInfo->flags & VK_SEMAPHORE_WAIT_ANY_BIT) == 0 || pWaitInfo->semaphoreCount == 1) { + for (uint32_t i = 0; i < pWaitInfo->semaphoreCount; i++) { + auto semaphore_state = Get(pWaitInfo->pSemaphores[i]); + if (semaphore_state) { + semaphore_state->RetireTimeline(pWaitInfo->pValues[i]); + } } } } @@ -1510,7 +1511,7 @@ void ValidationStateTracker::RecordGetSemaphoreCounterValue(VkDevice device, VkS auto semaphore_state = Get(semaphore); if (semaphore_state) { - semaphore_state->Retire(*pValue); + semaphore_state->RetireTimeline(*pValue); } } @@ -2924,9 +2925,8 @@ void ValidationStateTracker::PostCallRecordImportSemaphoreFdKHR(VkDevice device, void ValidationStateTracker::RecordGetExternalSemaphoreState(VkSemaphore semaphore, VkExternalSemaphoreHandleTypeFlagBits handle_type) { auto semaphore_state = Get(semaphore); - if (semaphore_state && handle_type != VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT) { - // Cannot track semaphore state once it is exported, except for Sync FD handle types which have copy transference - semaphore_state->scope = kSyncScopeExternalPermanent; + if (semaphore_state) { + semaphore_state->Export(handle_type); } } @@ -3046,12 +3046,12 @@ void ValidationStateTracker::PostCallRecordCreateDisplayModeKHR(VkPhysicalDevice } void ValidationStateTracker::PostCallRecordQueuePresentKHR(VkQueue queue, const VkPresentInfoKHR *pPresentInfo, VkResult result) { + auto queue_state = Get(queue); // Semaphore waits occur before error generation, if the call reached the ICD. (Confirm?) for (uint32_t i = 0; i < pPresentInfo->waitSemaphoreCount; ++i) { auto semaphore_state = Get(pPresentInfo->pWaitSemaphores[i]); if (semaphore_state) { - semaphore_state->signaler.queue = nullptr; - semaphore_state->signaled = false; + semaphore_state->EnqueuePresent(queue_state.get()); } } @@ -3072,8 +3072,6 @@ void ValidationStateTracker::PostCallRecordQueuePresentKHR(VkQueue queue, const } } } - // Note: even though presentation is directed to a queue, there is no direct ordering between QP and subsequent work, so QP (and - // its semaphore waits) /never/ participate in any completion proof. } void ValidationStateTracker::PostCallRecordCreateSharedSwapchainsKHR(VkDevice device, uint32_t swapchainCount, @@ -3100,11 +3098,10 @@ void ValidationStateTracker::RecordAcquireNextImageState(VkDevice device, VkSwap } auto semaphore_state = Get(semaphore); - if (semaphore_state && semaphore_state->scope == kSyncScopeInternal) { + if (semaphore_state) { // Treat as signaled since it is valid to wait on this semaphore, even in cases where it is technically a // temporary import - semaphore_state->signaled = true; - semaphore_state->signaler.queue = nullptr; + semaphore_state->EnqueueAcquire(); } // Mark the image as acquired.