Skip to content

Commit

Permalink
bp: Remove simple derived state objects
Browse files Browse the repository at this point in the history 
These are fairly simple and not likely to cause performance problems.
Just get rid of them so that it is easier to change how state tracking
works.

Image and CommandBuffer are a completely different story...
  • Loading branch information
@jeremyg-lunarg
jeremyg-lunarg committed Feb 19, 2025
1 parent 2ff1545 commit f441a3f
Show file tree
Hide file tree
Showing 18 changed files with 138 additions and 535 deletions.
98 changes: 1 addition & 97 deletions layers/best_practices/best_practices_validation.h
View file
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 @@ -880,28 +804,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 +838,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
View file
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
View file
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

0 comments on commit f441a3f

Please sign in to comment.