Skip to content
Open
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
1 change: 1 addition & 0 deletions layers/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ target_sources(vvl PRIVATE
state_tracker/wsi_state.h
stateless/sl_buffer.cpp
stateless/sl_cmd_buffer_dynamic.cpp
stateless/sl_command_pool.cpp
stateless/sl_cmd_buffer.cpp
stateless/sl_data_graph.cpp
stateless/sl_descriptor.cpp
Expand Down
85 changes: 83 additions & 2 deletions layers/core_checks/cc_data_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,78 @@
#include "state_tracker/pipeline_state.h"
#include "utils/math_utils.h"

bool CoreChecks::ValidateDataGraphPipelineBuiltinModelCreateInfoQCOM(const VkDataGraphPipelineBuiltinModelCreateInfoQCOM& dg_model_ci,
const Location& loc,
const VkDataGraphProcessingEngineCreateInfoARM* engine_ci) const {
bool skip = false;
bool has_identical_op = false;
bool has_identical_paired_op_engine = false;

std::vector<uint32_t> data_graph_queue_families{};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::vector<uint32_t> data_graph_queue_families{};
vvl::unordered_set<uint32_t> data_graph_queue_families{};

for (size_t family_index = 0; family_index < physical_device_state->queue_family_properties.size(); ++family_index) {
if ((physical_device_state->queue_family_properties[family_index].queueFlags & VK_QUEUE_DATA_GRAPH_BIT_ARM) != 0) {
data_graph_queue_families.push_back(static_cast<uint32_t>(family_index));
}
}

for (size_t index = 0; index < data_graph_queue_families.size(); ++index) {
const auto data_graph_properties_arms =
physical_device_state->GetQueueFamilyDataGraphPropsARM(data_graph_queue_families[index]);

for (size_t prop_index = 0; prop_index < data_graph_properties_arms.size(); ++prop_index) {
const auto& retrieved_operation = data_graph_properties_arms[prop_index].operation;
const auto& retrieved_engine = data_graph_properties_arms[prop_index].engine;

ASSERT_AND_CONTINUE(dg_model_ci.pOperation);

// Check whether the operations are identical or not
if (!CompareDataGraphOperationSupportARM(*dg_model_ci.pOperation, retrieved_operation)) {
continue;
}

has_identical_op = true;

ASSERT_AND_CONTINUE(engine_ci);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just pass in const VkDataGraphProcessingEngineCreateInfoARM& engine_ci then


// Check whether the engines are identical or not
for (uint32_t engine_index = 0; engine_index < engine_ci->processingEngineCount; ++engine_index) {
const auto& processing_engine = engine_ci->pProcessingEngines[engine_index];
if (processing_engine == retrieved_engine) {
has_identical_paired_op_engine = true;
break;
}
}
}
}

if (!has_identical_paired_op_engine) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this suppose to still be an error if data_graph_queue_families is empty?

A comment saying (and an assert(!data_graph_queue_families.empty());) to say otherwise that if we are in this function, there is a guaranteed VK_QUEUE_DATA_GRAPH_BIT_ARM

std::string conditional_string{};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use std::stringstream ss; like we do for other errors we want to print a more complex, conditional based, error message


if (!has_identical_op) {
conditional_string = (!dg_model_ci.pOperation) ?
"VkPhysicalDeviceDataGraphOperationSupportARM object is a nullptr." :
std::string{ "with input members operationType = " } +
string_VkPhysicalDeviceDataGraphOperationTypeARM(dg_model_ci.pOperation->operationType) +
", name = " +
dg_model_ci.pOperation->name +
", version = " + std::to_string(dg_model_ci.pOperation->version) +
", failed to find any identical operation in all retrieved "
"VkQueueFamilyDataGraphPropertiesARM::operation results.";
} else {
conditional_string = ", found the same operation among the retrieved VkQueueFamilyDataGraphPropertiesARM::operation "
"results, but failed to found the same paired VkQueueFamilyDataGraphPropertiesARM::engine, "
"please check VkDataGraphProcessingEngineCreateInfoARM object in the pNext chain.";
}

skip |= LogError("VUID-VkDataGraphPipelineBuiltinModelCreateInfoQCOM-pOperation-11842",
device,
loc.dot(Field::pOperation),
conditional_string.c_str());
}

return skip;
}

bool CoreChecks::ValidateDataGraphPipelineShaderModuleCreateInfo(VkDevice device,
const VkDataGraphPipelineShaderModuleCreateInfoARM& dg_shader_ci,
const Location& dg_shader_ci_loc,
Expand Down Expand Up @@ -152,6 +224,7 @@ bool CoreChecks::PreCallValidateCreateDataGraphPipelinesARM(VkDevice device, VkD
const ErrorObject& error_obj, PipelineStates& pipeline_states,
chassis::CreateDataGraphPipelinesARM& chassis_state) const {
bool skip = ValidateDeviceQueueSupport(error_obj.location);
bool use_dg_pipeline_identifier = false;

for (uint32_t i = 0; i < createInfoCount; i++) {
const VkDataGraphPipelineCreateInfoARM& create_info = pCreateInfos[i];
Expand Down Expand Up @@ -228,15 +301,23 @@ bool CoreChecks::PreCallValidateCreateDataGraphPipelinesARM(VkDevice device, VkD
skip |= ValidateDataGraphPipelineShaderModuleCreateInfo(device, *dg_shader_ci, dg_shader_ci_loc, *pipeline);
skip |= ValidateDataGraphPipelineShaderModuleSpirv(device, create_info, create_info_loc, *dg_shader_ci, *pipeline);
} else if (dg_pipeline_identifier_ci) {
// TODO: add here validation for datagraph defined as cache object
use_dg_pipeline_identifier = true;
} else if (qcom_model_ci) {
// TODO: add here validation for datagraph defined as QCOM model object
const Location built_in_model_loc = create_info_loc.pNext(Struct::VkDataGraphPipelineBuiltinModelCreateInfoQCOM);
skip |= ValidateDataGraphPipelineBuiltinModelCreateInfoQCOM(*qcom_model_ci, built_in_model_loc,
vku::FindStructInPNextChain<VkDataGraphProcessingEngineCreateInfoARM>(create_info.pNext));
}

// common checks
skip |= ValidateDataGraphPipelineCreateInfo(device, create_info, create_info_loc, *pipeline);
}

// Defer validate VkPipelineCacheHeaderVersionDataGraphQCOM of VkPipelineCache
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe say

Suggested change
// Defer validate VkPipelineCacheHeaderVersionDataGraphQCOM of VkPipelineCache
// Only need to validate the pipeline cache once even if multiple pipelines are being created

// TODO: may exist potential issues
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are "potential issues" here, please elaborate, if it is a spec issue, please provide a link to it here in the comment

if ((use_dg_pipeline_identifier) && (pipelineCache)) {
skip |= ValidatePipelineCacheHeaderVersionDataGraphQCOM(pipelineCache, error_obj.location.dot(Field::pipelineCache));
}

return skip;
}

Expand Down
60 changes: 60 additions & 0 deletions layers/core_checks/cc_descriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,50 @@
using DescriptorSetLayoutDef = vvl::DescriptorSetLayoutDef;
using DescriptorSetLayoutId = vvl::DescriptorSetLayoutId;

bool CoreChecks::ValidateDataGraphProcessingEngineForDescriptorPool(const VkDataGraphProcessingEngineCreateInfoARM& engine_ci,
const Location& loc) const {
bool skip = false;

ASSERT_AND_RETURN_SKIP(engine_ci.pProcessingEngines);

std::vector<uint32_t> data_graph_queue_families{};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::vector<uint32_t> data_graph_queue_families{};
vvl::unordered_set<uint32_t> data_graph_queue_families{};

for (size_t family_index = 0; family_index < physical_device_state->queue_family_properties.size(); ++family_index) {
if ((physical_device_state->queue_family_properties[family_index].queueFlags & VK_QUEUE_DATA_GRAPH_BIT_ARM) != 0) {
data_graph_queue_families.push_back(static_cast<uint32_t>(family_index));
}
}

vvl::unordered_set<VkPhysicalDeviceDataGraphProcessingEngineARM, HashCombineDataGraphProcessingEngineARMInfo> retrieved_engine_set{};
for (size_t index = 0; index < data_graph_queue_families.size(); ++index) {
const auto data_graph_properties_arms =
physical_device_state->GetQueueFamilyDataGraphPropsARM(data_graph_queue_families[index]);

for (size_t prop_index = 0; prop_index < data_graph_properties_arms.size(); ++prop_index) {
retrieved_engine_set.insert(data_graph_properties_arms[prop_index].engine);
}
}

for (uint32_t engine_index = 0; engine_index < engine_ci.processingEngineCount; ++engine_index) {
const auto& processing_engine = engine_ci.pProcessingEngines[engine_index];

if (!retrieved_engine_set.contains(processing_engine)) {
skip |= LogError("VUID-VkDescriptorPoolCreateInfo-pNext-09946",
device,
loc,
"for VkDataGraphProcessingEngineCreateInfoARM in the pNext chain, "
"pProcessingEngines[%" PRIu32 "]::type = %s, pProcessingEngines[%" PRIu32 "]::isForeign = %u, "
"failed to find any identical VkQueueFamilyDataGraphPropertiesARM::engine element from "
"vkGetPhysicalDeviceQueueFamilyDataGraphPropertiesARM.",
engine_index,
string_VkPhysicalDeviceDataGraphProcessingEngineTypeARM(processing_engine.type),
engine_index,
processing_engine.isForeign);
}
}

return skip;
}

// Check if the |reference_dsl| (from PipelineLayout) is compatibile with |to_bind_dsl|
// For GPL this is also used, but we don't care which DSL is which
bool CoreChecks::VerifyDescriptorSetLayoutIsCompatibile(const vvl::DescriptorSetLayout &reference_dsl,
Expand Down Expand Up @@ -3712,6 +3756,22 @@ bool CoreChecks::PreCallValidateGetDescriptorEXT(VkDevice device, const VkDescri
return skip;
}

bool CoreChecks::PreCallValidateCreateDescriptorPool(VkDevice device, const VkDescriptorPoolCreateInfo* pCreateInfo,
const VkAllocationCallbacks* pAllocator, VkDescriptorPool* pDescriptorPool,
const ErrorObject& error_obj) const {
bool skip = false;

ASSERT_AND_RETURN_SKIP(pCreateInfo);

if (const auto* processing_engine_info =
vku::FindStructInPNextChain<VkDataGraphProcessingEngineCreateInfoARM>(pCreateInfo->pNext); processing_engine_info) {
const Location processing_engine_ci_loc = error_obj.location.pNext(Struct::VkDataGraphProcessingEngineCreateInfoARM);
skip |= ValidateDataGraphProcessingEngineForDescriptorPool(*processing_engine_info, processing_engine_ci_loc);
}

return skip;
}

bool CoreChecks::PreCallValidateResetDescriptorPool(VkDevice device, VkDescriptorPool descriptorPool,
VkDescriptorPoolResetFlags flags, const ErrorObject &error_obj) const {
// Make sure sets being destroyed are not currently in-use
Expand Down
Loading