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.

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

bp: Remove simple derived state objects #9486

Merged
merged 1 commit into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
101 changes: 1 addition & 100 deletions layers/best_practices/best_practices_validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,6 @@ typedef enum {
} BPVendorFlagBits;
typedef VkFlags BPVendorFlags;

enum CALL_STATE {
UNCALLED, // Function has not been called
QUERY_COUNT, // Function called once to query a count
QUERY_DETAILS, // Function called w/ a count to query details
};

enum IMAGE_SUBRESOURCE_USAGE_BP {
UNDEFINED, // If it has never been used
RENDER_PASS_CLEARED,
Expand All @@ -169,20 +163,12 @@ enum class ZcullDirection {
};

namespace bp_state {
class PhysicalDevice;
class CommandBuffer;
class Swapchain;
class Image;
class DescriptorPool;
class Pipeline;
} // namespace bp_state

VALSTATETRACK_DERIVED_STATE_OBJECT(VkPhysicalDevice, bp_state::PhysicalDevice, vvl::PhysicalDevice)
VALSTATETRACK_DERIVED_STATE_OBJECT(VkCommandBuffer, bp_state::CommandBuffer, vvl::CommandBuffer)
VALSTATETRACK_DERIVED_STATE_OBJECT(VkSwapchainKHR, bp_state::Swapchain, vvl::Swapchain)
VALSTATETRACK_DERIVED_STATE_OBJECT(VkImage, bp_state::Image, vvl::Image)
VALSTATETRACK_DERIVED_STATE_OBJECT(VkDescriptorPool, bp_state::DescriptorPool, vvl::DescriptorPool)
VALSTATETRACK_DERIVED_STATE_OBJECT(VkPipeline, bp_state::Pipeline, vvl::Pipeline)

namespace bp_state {
template <typename StateObject, typename Handle>
Expand Down Expand Up @@ -218,7 +204,6 @@ class Instance : public vvl::Instance {
using Field = vvl::Field;

Instance(vvl::dispatch::Instance* dispatch) : BaseClass(dispatch, LayerObjectTypeBestPractices) {}
std::shared_ptr<vvl::PhysicalDevice> CreatePhysicalDeviceState(VkPhysicalDevice handle) final;

bool VendorCheckEnabled(BPVendorFlags vendors) const { return bp_state::VendorCheckEnabled(enabled, vendors); }

Expand Down Expand Up @@ -258,61 +243,7 @@ class Instance : public vvl::Instance {
const ErrorObject& error_obj) const override;
bool ValidateCommonGetPhysicalDeviceQueueFamilyProperties(const vvl::PhysicalDevice& pd_state,
uint32_t requested_queue_family_property_count,
const CALL_STATE call_state, const Location& loc) const;
void CommonPostCallRecordGetPhysicalDeviceQueueFamilyProperties(CALL_STATE& call_state, bool no_pointer);
void PostCallRecordGetPhysicalDeviceQueueFamilyProperties(VkPhysicalDevice physicalDevice, uint32_t* pQueueFamilyPropertyCount,
VkQueueFamilyProperties* pQueueFamilyProperties,
const RecordObject& record_obj) override;

void PostCallRecordGetPhysicalDeviceQueueFamilyProperties2(VkPhysicalDevice physicalDevice, uint32_t* pQueueFamilyPropertyCount,
VkQueueFamilyProperties2* pQueueFamilyProperties,
const RecordObject& record_obj) override;

void PostCallRecordGetPhysicalDeviceQueueFamilyProperties2KHR(VkPhysicalDevice physicalDevice,
uint32_t* pQueueFamilyPropertyCount,
VkQueueFamilyProperties2* pQueueFamilyProperties,
const RecordObject& record_obj) override;

void PostCallRecordGetPhysicalDeviceFeatures(VkPhysicalDevice physicalDevice, VkPhysicalDeviceFeatures* pFeatures,
const RecordObject& record_obj) override;

void PostCallRecordGetPhysicalDeviceFeatures2(VkPhysicalDevice physicalDevice, VkPhysicalDeviceFeatures2* pFeatures,
const RecordObject& record_obj) override;

void PostCallRecordGetPhysicalDeviceFeatures2KHR(VkPhysicalDevice physicalDevice, VkPhysicalDeviceFeatures2* pFeatures,
const RecordObject& record_obj) override;

void ManualPostCallRecordGetPhysicalDeviceSurfaceCapabilitiesKHR(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface,
VkSurfaceCapabilitiesKHR* pSurfaceCapabilities,
const RecordObject& record_obj);

void ManualPostCallRecordGetPhysicalDeviceSurfaceCapabilities2KHR(VkPhysicalDevice physicalDevice,
const VkPhysicalDeviceSurfaceInfo2KHR* pSurfaceInfo,
VkSurfaceCapabilities2KHR* pSurfaceCapabilities,
const RecordObject& record_obj);

void ManualPostCallRecordGetPhysicalDeviceSurfaceCapabilities2EXT(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface,
VkSurfaceCapabilities2EXT* pSurfaceCapabilities,
const RecordObject& record_obj);

void ManualPostCallRecordGetPhysicalDeviceSurfacePresentModesKHR(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface,
uint32_t* pPresentModeCount, VkPresentModeKHR* pPresentModes,
const RecordObject& record_obj);

void ManualPostCallRecordGetPhysicalDeviceSurfaceFormatsKHR(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface,
uint32_t* pSurfaceFormatCount, VkSurfaceFormatKHR* pSurfaceFormats,
const RecordObject& record_obj);

void ManualPostCallRecordGetPhysicalDeviceSurfaceFormats2KHR(VkPhysicalDevice physicalDevice,
const VkPhysicalDeviceSurfaceInfo2KHR* pSurfaceInfo,
uint32_t* pSurfaceFormatCount,
VkSurfaceFormat2KHR* pSurfaceFormats,
const RecordObject& record_obj);

void ManualPostCallRecordGetPhysicalDeviceDisplayPlanePropertiesKHR(VkPhysicalDevice physicalDevice, uint32_t* pPropertyCount,
VkDisplayPlanePropertiesKHR* pProperties,
const RecordObject& record_obj);

const Location& loc) const;
// Include code-generated functions
#include "generated/best_practices_instance_methods.h"
};
Expand Down Expand Up @@ -365,11 +296,6 @@ class BestPractices : public vvl::Device {
bool PreCallValidateAllocateDescriptorSets(VkDevice device, const VkDescriptorSetAllocateInfo* pAllocateInfo,
VkDescriptorSet* pDescriptorSets, const ErrorObject& error_obj,
vvl::AllocateDescriptorSetsData& ads_state_data) const override;
void ManualPostCallRecordAllocateDescriptorSets(VkDevice device, const VkDescriptorSetAllocateInfo* pAllocateInfo,
VkDescriptorSet* pDescriptorSets, const RecordObject& record_obj,
vvl::AllocateDescriptorSetsData& ads_state);
void PostCallRecordFreeDescriptorSets(VkDevice device, VkDescriptorPool descriptorPool, uint32_t descriptorSetCount,
const VkDescriptorSet* pDescriptorSets, const RecordObject& record_obj) override;
bool PreCallValidateAllocateMemory(VkDevice device, const VkMemoryAllocateInfo* pAllocateInfo,
const VkAllocationCallbacks* pAllocator, VkDeviceMemory* pMemory,
const ErrorObject& error_obj) const override;
Expand All @@ -393,8 +319,6 @@ class BestPractices : public vvl::Device {
const ErrorObject& error_obj) const override;
bool PreCallValidateBindImageMemory2KHR(VkDevice device, uint32_t bindInfoCount, const VkBindImageMemoryInfo* pBindInfos,
const ErrorObject& error_obj) const override;
void PostCallRecordSetDeviceMemoryPriorityEXT(VkDevice device, VkDeviceMemory memory, float priority,
const RecordObject& record_obj) override;
bool PreCallValidateGetVideoSessionMemoryRequirementsKHR(VkDevice device, VkVideoSessionKHR videoSession,
uint32_t* pMemoryRequirementsCount,
VkVideoSessionMemoryRequirementsKHR* pMemoryRequirements,
Expand Down Expand Up @@ -771,9 +695,6 @@ class BestPractices : public vvl::Device {
bool PreCallValidateAcquireNextImageKHR(VkDevice device, VkSwapchainKHR swapchain, uint64_t timeout, VkSemaphore semaphore,
VkFence fence, uint32_t* pImageIndex, const ErrorObject& error_obj) const override;

void ManualPostCallRecordGetSwapchainImagesKHR(VkDevice device, VkSwapchainKHR swapchain, uint32_t* pSwapchainImageCount,
VkImage* pSwapchainImages, const RecordObject& record_obj);

void ManualPostCallRecordQueueSubmit(VkQueue queue, uint32_t submitCount, const VkSubmitInfo* pSubmits, VkFence fence,
const RecordObject& record_obj);

Expand Down Expand Up @@ -880,28 +801,12 @@ class BestPractices : public vvl::Device {
const VkCommandBufferAllocateInfo* allocate_info,
const vvl::CommandPool* pool) final;

std::shared_ptr<vvl::Swapchain> CreateSwapchainState(const VkSwapchainCreateInfoKHR* create_info, VkSwapchainKHR handle) final;

std::shared_ptr<vvl::Image> CreateImageState(VkImage handle, const VkImageCreateInfo* create_info,
VkFormatFeatureFlags2 features) final;

std::shared_ptr<vvl::Image> CreateImageState(VkImage handle, const VkImageCreateInfo* create_info, VkSwapchainKHR swapchain,
uint32_t swapchain_index, VkFormatFeatureFlags2 features) final;

std::shared_ptr<vvl::DescriptorPool> CreateDescriptorPoolState(VkDescriptorPool handle,
const VkDescriptorPoolCreateInfo* create_info) final;

std::shared_ptr<vvl::DeviceMemory> CreateDeviceMemoryState(VkDeviceMemory handle, const VkMemoryAllocateInfo* p_alloc_info,
uint64_t fake_address, const VkMemoryType& memory_type,
const VkMemoryHeap& memory_heap,
std::optional<vvl::DedicatedBinding>&& dedicated_binding,
uint32_t physical_device_count) final;

std::shared_ptr<vvl::Pipeline> CreateGraphicsPipelineState(
const VkGraphicsPipelineCreateInfo* create_info, std::shared_ptr<const vvl::PipelineCache> pipeline_cache,
std::shared_ptr<const vvl::RenderPass>&& render_pass, std::shared_ptr<const vvl::PipelineLayout>&& layout,
spirv::StatelessData stateless_data[kCommonMaxGraphicsShaderStages]) const final;

private:
// CacheEntry and PostTransformLRUCacheModel are used on the stack
struct CacheEntry {
Expand Down Expand Up @@ -930,10 +835,6 @@ class BestPractices : public vvl::Device {
void RecordCmdDrawTypeArm(bp_state::CommandBuffer& cb_state, uint32_t draw_count);
void RecordCmdDrawTypeNVIDIA(bp_state::CommandBuffer& cb_state);

// Get BestPractices-specific for the current instance
bp_state::PhysicalDevice* GetPhysicalDeviceState();
const bp_state::PhysicalDevice* GetPhysicalDeviceState() const;

void RecordAttachmentClearAttachments(bp_state::CommandBuffer& cb_state, uint32_t fb_attachment, uint32_t color_attachment,
VkImageAspectFlags aspects, uint32_t rectCount, const VkClearRect* pRects);
void RecordAttachmentAccess(bp_state::CommandBuffer& cb_state, uint32_t attachment, VkImageAspectFlags aspects);
Expand Down
36 changes: 2 additions & 34 deletions layers/best_practices/bp_descriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ bool BestPractices::PreCallValidateAllocateDescriptorSets(VkDevice device, const
skip |= BaseClass::PreCallValidateAllocateDescriptorSets(device, pAllocateInfo, pDescriptorSets, error_obj, ads_state_data);
if (skip) return skip;

const auto pool_state = Get<bp_state::DescriptorPool>(pAllocateInfo->descriptorPool);
const auto pool_state = Get<vvl::DescriptorPool>(pAllocateInfo->descriptorPool);
ASSERT_AND_RETURN_SKIP(pool_state);

// if the number of freed sets > 0, it implies they could be recycled instead if desirable
// this warning is specific to Arm
if (VendorCheckEnabled(kBPVendorArm) && (pool_state->freed_count > 0)) {
if (VendorCheckEnabled(kBPVendorArm) && (pool_state->GetFreedCount() > 0)) {
skip |= LogPerformanceWarning(
"BestPractices-Arm-vkAllocateDescriptorSets-suboptimal-reuse", device, error_obj.location,
"%s Descriptor set memory was allocated via vkAllocateDescriptorSets() for sets which were previously freed in the "
Expand Down Expand Up @@ -70,34 +70,6 @@ bool BestPractices::PreCallValidateAllocateDescriptorSets(VkDevice device, const
return skip;
}

void BestPractices::ManualPostCallRecordAllocateDescriptorSets(VkDevice device, const VkDescriptorSetAllocateInfo* pAllocateInfo,
VkDescriptorSet* pDescriptorSets, const RecordObject& record_obj,
vvl::AllocateDescriptorSetsData& ads_state) {
if (record_obj.result == VK_SUCCESS) {
if (auto pool_state = Get<bp_state::DescriptorPool>(pAllocateInfo->descriptorPool)) {
// we record successful allocations by subtracting the allocation count from the last recorded free count
const auto alloc_count = pAllocateInfo->descriptorSetCount;
// clamp the unsigned subtraction to the range [0, last_free_count]
if (pool_state->freed_count > alloc_count) {
pool_state->freed_count -= alloc_count;
} else {
pool_state->freed_count = 0;
}
}
}
}

void BestPractices::PostCallRecordFreeDescriptorSets(VkDevice device, VkDescriptorPool descriptorPool, uint32_t descriptorSetCount,
const VkDescriptorSet* pDescriptorSets, const RecordObject& record_obj) {
BaseClass::PostCallRecordFreeDescriptorSets(device, descriptorPool, descriptorSetCount, pDescriptorSets, record_obj);
if (record_obj.result == VK_SUCCESS) {
// we want to track frees because we're interested in suggesting re-use
if (auto pool_state = Get<bp_state::DescriptorPool>(descriptorPool)) {
pool_state->freed_count += descriptorSetCount;
}
}
}

bool BestPractices::PreCallValidateCreateSampler(VkDevice device, const VkSamplerCreateInfo* pCreateInfo,
const VkAllocationCallbacks* pAllocator, VkSampler* pSampler,
const ErrorObject& error_obj) const {
Expand Down Expand Up @@ -193,7 +165,3 @@ bool BestPractices::PreCallValidateCreateDescriptorUpdateTemplate(VkDevice devic
return skip;
}

std::shared_ptr<vvl::DescriptorPool> BestPractices::CreateDescriptorPoolState(VkDescriptorPool handle,
const VkDescriptorPoolCreateInfo* create_info) {
return std::static_pointer_cast<vvl::DescriptorPool>(std::make_shared<bp_state::DescriptorPool>(*this, handle, create_info));
}
15 changes: 1 addition & 14 deletions layers/best_practices/bp_device_memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,17 +284,11 @@ bool BestPractices::PreCallValidateBindImageMemory2KHR(VkDevice device, uint32_t
return PreCallValidateBindImageMemory2(device, bindInfoCount, pBindInfos, error_obj);
}

void BestPractices::PostCallRecordSetDeviceMemoryPriorityEXT(VkDevice device, VkDeviceMemory memory, float priority,
const RecordObject& record_obj) {
auto mem_info = std::static_pointer_cast<bp_state::DeviceMemory>(Get<vvl::DeviceMemory>(memory));
mem_info->dynamic_priority.emplace(priority);
}

bool BestPractices::ValidateBindMemory(VkDevice device, VkDeviceMemory memory, const Location& loc) const {
bool skip = false;

if (VendorCheckEnabled(kBPVendorNVIDIA) && IsExtEnabled(extensions.vk_ext_pageable_device_local_memory)) {
auto mem_info = std::static_pointer_cast<const bp_state::DeviceMemory>(Get<vvl::DeviceMemory>(memory));
auto mem_info = Get<vvl::DeviceMemory>(memory);
bool has_static_priority = vku::FindStructInPNextChain<VkMemoryPriorityAllocateInfoEXT>(mem_info->allocate_info.pNext);
if (!mem_info->dynamic_priority && !has_static_priority) {
skip |=
Expand All @@ -310,13 +304,6 @@ bool BestPractices::ValidateBindMemory(VkDevice device, VkDeviceMemory memory, c
return skip;
}

std::shared_ptr<vvl::DeviceMemory> BestPractices::CreateDeviceMemoryState(
VkDeviceMemory handle, const VkMemoryAllocateInfo* allocate_info, uint64_t fake_address, const VkMemoryType& memory_type,
const VkMemoryHeap& memory_heap, std::optional<vvl::DedicatedBinding>&& dedicated_binding, uint32_t physical_device_count) {
return std::static_pointer_cast<vvl::DeviceMemory>(std::make_shared<bp_state::DeviceMemory>(
handle, allocate_info, fake_address, memory_type, memory_heap, std::move(dedicated_binding), physical_device_count));
}

void BestPractices::ManualPostCallRecordBindBufferMemory2(VkDevice device, uint32_t bindInfoCount,
const VkBindBufferMemoryInfo* pBindInfos,
const RecordObject& record_obj) {
Expand Down
Loading
Loading