layers: Remove QueueSubmissionValidator

Move timeline signal validation from QueueSubmissionValidator to
SubmitTimeTracker and remove QueueSubmissionValidator

Author: artem-lunarg

BUILD.gn

@@ -144,8 +144,6 @@ vvl_sources = [
   "layers/core_checks/cc_spirv.cpp",
   "layers/core_checks/cc_state_tracker.cpp",
   "layers/core_checks/cc_state_tracker.h",
-  "layers/core_checks/cc_submit.cpp",
-  "layers/core_checks/cc_submit.h",
   "layers/core_checks/cc_sync_vuid_maps.h",
   "layers/core_checks/cc_sync_vuid_maps.cpp",
   "layers/core_checks/cc_synchronization.cpp",

layers/CMakeLists.txt

@@ -244,8 +244,6 @@ target_sources(vvl PRIVATE
     core_checks/cc_shader_object.cpp
     core_checks/cc_state_tracker.h
     core_checks/cc_state_tracker.cpp
-    core_checks/cc_submit.h
-    core_checks/cc_submit.cpp
     core_checks/cc_sync_vuid_maps.h
     core_checks/cc_sync_vuid_maps.cpp
     core_checks/cc_synchronization.h

layers/core_checks/cc_queue.cpp

@@ -290,9 +290,11 @@ bool CoreChecks::PreCallValidateQueueSubmit(VkQueue queue, uint32_t submitCount,
                                  chained_device_group_struct->commandBufferCount, submit.commandBufferCount);
             }
         }
-        // Perform submit time validation at the end.
-        // If submit API is used incorrectly, we want those errors to be reported first
-        skip |= submit_time_tracker.ProcessSubmitInfo(submit, queue, submit_loc);
+        // Run submit time validation only if skip is not set. This is mostly for vvl testing,
+        // some tests expect that submit state is not updated if regular validation fails
+        if (!skip) {
+            skip |= submit_time_tracker.ProcessSubmitInfo(submit, queue, submit_loc);
+        }
     }
 
     return skip;
@@ -452,9 +454,11 @@ bool CoreChecks::ValidateQueueSubmit2(VkQueue queue, uint32_t submitCount, const
             skip |= LogError("VUID-VkSubmitInfo2-commandBuffer-06010", queue, submit_loc,
                              "has a suspended render pass instance that was not resumed.");
         }
-        // Perform submit time validation at the end.
-        // If submit API is used incorrectly, we want those errors to be reported first
-        skip |= submit_time_tracker.ProcessSubmitInfo(submit, queue, submit_loc);
+        // Run submit time validation only if skip is not set. This is mostly for vvl testing,
+        // some tests expect that submit state is not updated if regular validation fails
+        if (!skip) {
+            skip |= submit_time_tracker.ProcessSubmitInfo(submit, queue, submit_loc);
+        }
     }
 
     return skip;
@@ -783,8 +787,21 @@ bool CoreChecks::PreCallValidateQueueBindSparse(VkQueue queue, uint32_t bindInfo
     return skip;
 }
 
-bool CoreChecks::ProcessSubmissionBatch(const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
-                                        const Location& submit_loc) {
+static Location GetSignaledSemaphoreLocation(const Location& submit_loc, uint32_t index) {
+    vvl::Field field = vvl::Field::Empty;
+    if (submit_loc.function == vvl::Func::vkQueueSubmit || submit_loc.function == vvl::Func::vkQueueBindSparse) {
+        field = vvl::Field::pSignalSemaphores;
+    } else if (submit_loc.function == vvl::Func::vkQueueSubmit2 || submit_loc.function == vvl::Func::vkQueueSubmit2KHR) {
+        field = vvl::Field::pSignalSemaphoreInfos;
+    } else {
+        assert(false && "Unhandled signaling function");
+    }
+    return submit_loc.dot(field, index);
+}
+
+bool CoreChecks::ProcessSubmissionBatch(const vvl::SubmitTimeTracker& tracker,
+                                        const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
+                                        vvl::span<const VkSemaphoreSubmitInfo> signal_semaphores, const Location& submit_loc) {
     bool skip = false;
     // Validate image layouts on the command buffer boundaries
     {
@@ -796,6 +813,27 @@ bool CoreChecks::ProcessSubmissionBatch(const std::vector<std::shared_ptr<vvl::C
             }
         }
     }
+    // Ensure that timeline signals are monotonically increasing values
+    for (uint32_t i = 0; i < (uint32_t)signal_semaphores.size(); ++i) {
+        const VkSemaphore semaphore = signal_semaphores[i].semaphore;
+        const std::optional<uint64_t> current_value = tracker.GetTimelineValue(semaphore);
+        if (!current_value.has_value()) {
+            continue;
+        }
+        // Validate the case where the signal value is less than the current payload.
+        // Equality, which is also invalid, is handled earlier by SemaphoreSubmitState::ValidateTimelineSignal.
+        // Equality can be checked early because it is invalid regardless submit order.
+        // For not equal values, the comparison depends on submit ordering and is handled here
+        const uint64_t signal_value = signal_semaphores[i].value;
+        const bool invlid_signal_value = signal_value < *current_value;
+        if (invlid_signal_value) {
+            const Location signal_semaphore_loc = GetSignaledSemaphoreLocation(submit_loc, i);
+            const auto& vuid = GetQueueSubmitVUID(signal_semaphore_loc, vvl::SubmitError::kTimelineSemSmallValue);
+            LogError(vuid, semaphore, signal_semaphore_loc,
+                     "(%s) signaled with value %" PRIu64 " which is smaller than the current value %" PRIu64,
+                     FormatHandle(semaphore).c_str(), signal_value, *current_value);
+        }
+    }
     if (!skip) {
         for (const auto& cb : command_buffers) {
             if (cb) {

layers/core_checks/cc_state_tracker.cpp

@@ -41,7 +41,7 @@ void CoreChecks::Created(vvl::CommandBuffer& cb) {
 }
 
 void CoreChecks::Created(vvl::Queue& queue) {
-    queue.SetSubState(container_type, std::make_unique<core::QueueSubState>(*this, queue));
+    queue.SetSubState(container_type, std::make_unique<core::QueueSubState>(queue));
 }
 
 namespace core {
@@ -1214,9 +1214,6 @@ void CommandBufferSubState::SubmitTimeValidate() {
     }
 }
 
-QueueSubState::QueueSubState(CoreChecks& core_checks, vvl::Queue& q)
-    : vvl::QueueSubState(q), queue_submission_validator_(core_checks) {}
-
 void QueueSubState::PreSubmit(std::vector<vvl::QueueSubmission>& submissions) {
     for (const auto& submission : submissions) {
         for (auto& cb : submission.cb_submissions) {
@@ -1228,8 +1225,6 @@ void QueueSubState::PreSubmit(std::vector<vvl::QueueSubmission>& submissions) {
 }
 
 void QueueSubState::Retire(vvl::QueueSubmission& submission) {
-    queue_submission_validator_.Validate(submission);
-
     auto is_query_updated_after = [this](const QueryObject& query_object) {
         auto guard = base.Lock();
         bool first_queue_submission = true;

layers/core_checks/cc_state_tracker.h

@@ -18,7 +18,6 @@
  * limitations under the License.
  */
 #pragma once
-#include "cc_submit.h"
 #include "state_tracker/state_tracker.h"
 #include "state_tracker/cmd_buffer_state.h"
 #include "state_tracker/queue_state.h"
@@ -245,15 +244,12 @@ static inline const CommandBufferSubState &SubState(const vvl::CommandBuffer &cb
 
 class QueueSubState : public vvl::QueueSubState {
   public:
-    QueueSubState(CoreChecks &core_checks, vvl::Queue& q);
+    QueueSubState(vvl::Queue& q) : vvl::QueueSubState(q) {}
 
     void PreSubmit(std::vector<vvl::QueueSubmission> &submissions) override;
 
     // Override Retire to validate submissions in the order defined by synchronization
     void Retire(vvl::QueueSubmission&) override;
-
-  private:
-    QueueSubmissionValidator queue_submission_validator_;
 };
 
 }  // namespace core

layers/core_checks/cc_submit.cpp

@@ -1648,9 +1648,11 @@ bool CoreChecks::PreCallValidateSignalSemaphore(VkDevice device, const VkSemapho
                          FormatHandle(*semaphore_state).c_str(), payload_type, *far_away_payload);
     }
 
-    // Perform submit time validation at the end.
-    // If signal semaphore API is used incorrectly, we want those errors to be reported first
-    skip |= submit_time_tracker.ProcessSignalSemaphore(*pSignalInfo);
+    // Run submit time validation only if skip is not set. This is mostly for vvl testing,
+    // some tests expect that submit state is not updated if regular validation fails
+    if (!skip) {
+        skip |= submit_time_tracker.ProcessSignalSemaphore(*pSignalInfo);
+    }
     return skip;
 }
 

layers/core_checks/cc_submit.h

@@ -1263,9 +1263,11 @@ bool CoreChecks::PreCallValidateQueuePresentKHR(VkQueue queue, const VkPresentIn
         skip |= ValidateSwapchainPresentFenceInfo(queue, *pPresentInfo, *swapchain_present_fence_info, present_info_loc);
     }
 
-    // Perform submit time validation at the end.
-    // If present API is used incorrectly, we want those errors to be reported first
-    skip |= submit_time_tracker.ProcessPresent(*pPresentInfo, present_info_loc);
+    // Run submit time validation only if skip is not set. This is mostly for vvl testing,
+    // some tests expect that submit state is not updated if regular validation fails
+    if (!skip) {
+        skip |= submit_time_tracker.ProcessPresent(*pPresentInfo, present_info_loc);
+    }
 
     return skip;
 }

layers/core_checks/cc_synchronization.cpp

@@ -1587,8 +1587,9 @@ class CoreChecks : public vvl::DeviceProxy {
                                        const RecordObject& record_obj) override;
     void PostCallRecordQueueSubmit2(VkQueue queue, uint32_t submitCount, const VkSubmitInfo2* pSubmits, VkFence fence,
                                     const RecordObject& record_obj) override;
-    bool ProcessSubmissionBatch(const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
-                                const Location& submit_loc) override;
+    bool ProcessSubmissionBatch(const vvl::SubmitTimeTracker& tracker,
+                                const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
+                                vvl::span<const VkSemaphoreSubmitInfo> signal_semaphores, const Location& submit_loc) override;
     bool ProcessPresentBatch(const vvl::Image& swapchain_image, const Location& present_info_loc) override;
     bool IgnoreAllocationSize(const VkMemoryAllocateInfo& allocate_info) const;
     bool HasExternalMemoryImportSupport(const vvl::Buffer& buffer, VkExternalMemoryHandleTypeFlagBits handle_type) const;