Skip to content

Commit

Permalink
layers: Move VK_EXT_robustness2 check to corechecks
Browse files Browse the repository at this point in the history
  • Loading branch information
@spencer-lunarg
spencer-lunarg committed Jan 9, 2025
1 parent 196627c commit c4bb2de
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 86 deletions.
39 changes: 35 additions & 4 deletions layers/core_checks/cc_pipeline.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* Copyright (c) 2015-2024 The Khronos Group Inc.
* Copyright (c) 2015-2024 Valve Corporation
* Copyright (c) 2015-2024 LunarG, Inc.
* Copyright (C) 2015-2024 Google Inc.
/* Copyright (c) 2015-2025 The Khronos Group Inc.
* Copyright (c) 2015-2025 Valve Corporation
* Copyright (c) 2015-2025 LunarG, Inc.
* Copyright (C) 2015-2025 Google Inc.
* Modifications Copyright (C) 2020-2022 Advanced Micro Devices, Inc. All rights reserved.
* Modifications Copyright (C) 2022 RasterGrid Kft.
*
Expand Down Expand Up @@ -126,6 +126,37 @@ bool CoreChecks::ValidatePipelineRobustnessCreateInfo(const vvl::Pipeline &pipel
"is VK_PIPELINE_ROBUSTNESS_IMAGE_BEHAVIOR_ROBUST_IMAGE_ACCESS "
"but robustImageAccess2 is not supported.");
}

if (!has_robust_buffer_access2) {
if (pipeline_robustness_info.storageBuffers == VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2) {
skip |= LogError(
"VUID-VkPipelineRobustnessCreateInfo-robustBufferAccess2-06931", device,
loc.pNext(Struct::VkPipelineRobustnessCreateInfo, Field::storageBuffers),
"is VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2, but robustBufferAccess2 is not supported.");
}
if (pipeline_robustness_info.uniformBuffers == VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2) {
skip |= LogError(
"VUID-VkPipelineRobustnessCreateInfo-robustBufferAccess2-06932", device,
loc.pNext(Struct::VkPipelineRobustnessCreateInfo, Field::uniformBuffers),
"is VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2, but robustBufferAccess2 is not supported.");
}
if (pipeline_robustness_info.vertexInputs == VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2) {
skip |= LogError(
"VUID-VkPipelineRobustnessCreateInfo-robustBufferAccess2-06933", device,
loc.pNext(Struct::VkPipelineRobustnessCreateInfo, Field::vertexInputs),
"is VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2, but robustBufferAccess2 is not supported.");
}
}

if (!has_robust_image_access2) {
if (pipeline_robustness_info.images == VK_PIPELINE_ROBUSTNESS_IMAGE_BEHAVIOR_ROBUST_IMAGE_ACCESS_2) {
skip |= LogError(
"VUID-VkPipelineRobustnessCreateInfo-robustImageAccess2-06934", device,
loc.pNext(Struct::VkPipelineRobustnessCreateInfo, Field::images),
"is VK_PIPELINE_ROBUSTNESS_IMAGE_BEHAVIOR_ROBUST_IMAGE_ACCESS_2, but robustImageAccess2 is not supported.");
}
}

return skip;
}

Expand Down
12 changes: 12 additions & 0 deletions layers/state_tracker/state_tracker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,18 @@ void ValidationStateTracker::PostCreateDevice(const VkDeviceCreateInfo *pCreateI
has_robust_image_access =
(api_version >= VK_API_VERSION_1_3 || IsExtEnabled(instance_extensions.vk_khr_get_physical_device_properties2)) &&
phys_dev_extensions.find(vvl::Extension::_VK_EXT_image_robustness) != phys_dev_extensions.end();

if (IsExtEnabled(instance_extensions.vk_khr_get_physical_device_properties2) &&
phys_dev_extensions.find(vvl::Extension::_VK_EXT_image_robustness) != phys_dev_extensions.end()) {
VkPhysicalDeviceRobustness2FeaturesEXT robustness_2_features = vku::InitStructHelper();
VkPhysicalDeviceFeatures2 features2 = vku::InitStructHelper(&robustness_2_features);
DispatchGetPhysicalDeviceFeatures2Helper(physical_device, &features2);
has_robust_image_access2 = robustness_2_features.robustImageAccess2;

This comment has been minimized.

Sign in to view

Copy link
@y-novikov

y-novikov Jan 10, 2025

Contributor

We are getting crashes in SwiftShader around this line:
https://chromium-review.googlesource.com/c/angle/angle/+/6164685
https://ci.chromium.org/ui/p/angle/builders/try/linux-test/25042/overview
https://chromium-swarm.appspot.com/task?id=6e6568d7ba46de10

[ RUN      ] VulkanDescriptorSetTest.AtomicCounterReadLimitedDescriptorPool/ES3_1_Vulkan_SwiftShader
../../third_party/SwiftShader/src/Vulkan/VkPhysicalDevice.cpp:684 ABORT: UNSUPPORTED: curExtension->sType: 1000286000
angle::PrintStackBacktrace() at crash_handler_posix.cpp:500
angle::Handler(int) at crash_handler_posix.cpp:661
killpg at ??:?
__GI_raise at raise.c:51 (discriminator 3)
__GI_abort at abort.c:81
sw::abort(char const*, ...) at Debug.cpp:?
__is_long at string:1892
ValidationStateTracker::PostCreateDevice(VkDeviceCreateInfo const*, Location const&) at state_tracker.cpp:760
CoreChecks::PostCreateDevice(VkDeviceCreateInfo const*, Location const&) at cc_device.cpp:346
~unique_lock at unique_lock.h:62
loader_create_device_chain at loader.c:5145
loader_layer_create_device at loader.c:4507
vkCreateDevice at trampoline.c:970
rx::vk::Renderer::createDeviceAndQueue(rx::vk::Context*, unsigned int) at vk_renderer.cpp:4076
rx::vk::Renderer::initialize(rx::vk::Context*, rx::vk::GlobalOps*, angle::vk::ICD, unsigned int, unsigned int, rx::vk::UseDebugLayers, char const*, char const*, angle::NativeWindowSystem, angle::FeatureOverrides const&) at vk_renderer.cpp:?
rx::DisplayVk::initialize(egl::Display*) at DisplayVk.cpp:178
rx::DisplayVkXcb::initialize(egl::Display*) at DisplayVkXcb.cpp:65
isError at Error.inc:82
isError at Error.inc:82
EGL_Initialize at entry_points_egl_autogen.cpp:587
EGLWindow::initializeDisplay(OSWindow*, angle::Library*, angle::GLESDriverType, EGLPlatformParameters const&) at EGLWindow.cpp:293
ANGLETestBase::ANGLETestSetUp() at ANGLETest.cpp:757
SetUp at ANGLETest.h:670
testing::Test::Run() at gtest.cc:2704
testing::TestInfo::Run() at gtest.cc:2888
testing::TestSuite::Run() at gtest.cc:3040
testing::internal::UnitTestImpl::RunAllTests() at gtest.cc:5898
testing::UnitTest::Run() at gtest.cc:5464
RUN_ALL_TESTS at gtest.h:2492
main at angle_generic_tests_main.cpp:17
__libc_start_main at libc-start.c:344

Are you missing some condition to enable this test?
Or does SwiftShader say that it supports the required extension, while in fact it doesn't?

@SoltiGG

This comment has been minimized.

Sign in to view

Copy link
@spencer-lunarg

spencer-lunarg Jan 10, 2025

Author Contributor

no, this is a bug, I mixed up VK_EXT_image_robustness with VK_EXT_robustness2 in the check
has_robust_buffer_access2 = robustness_2_features.robustBufferAccess2;
} else {
has_robust_image_access2 = false;
has_robust_buffer_access2 = false;
}
}

const auto &dev_ext = device_extensions;
Expand Down
7 changes: 4 additions & 3 deletions layers/state_tracker/state_tracker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1816,9 +1816,10 @@ class ValidationStateTracker : public ValidationObject {
// Enabling the other robustness features can reduce performance on GPU, so just the
// support is needed to check
bool has_robust_image_access; // VK_EXT_image_robustness
// TODO - Issue 5657
// bool has_robust_image_access2; // VK_EXT_robustness2
// bool has_robust_buffer_access2; // VK_EXT_robustness2
// Validation requires special handling for VkPhysicalDeviceRobustness2FeaturesEXT, because for some cases robustness features
// // need to only be supported, not enabled
bool has_robust_image_access2; // VK_EXT_robustness2
bool has_robust_buffer_access2; // VK_EXT_robustness2

// Device extension properties -- storing properties gathered from VkPhysicalDeviceProperties2::pNext chain
struct DeviceExtensionProperties {
Expand Down
14 changes: 4 additions & 10 deletions layers/stateless/sl_instance_device.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* Copyright (c) 2015-2024 The Khronos Group Inc.
* Copyright (c) 2015-2024 Valve Corporation
* Copyright (c) 2015-2024 LunarG, Inc.
* Copyright (C) 2015-2024 Google Inc.
/* Copyright (c) 2015-2025 The Khronos Group Inc.
* Copyright (c) 2015-2025 Valve Corporation
* Copyright (c) 2015-2025 LunarG, Inc.
* Copyright (C) 2015-2025 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -246,12 +246,6 @@ void StatelessValidation::CommonPostCallRecordEnumeratePhysicalDevice(const VkPh
discard_rectangles_extension_version = ext_props[j].specVersion;
} else if (extension == vvl::Extension::_VK_NV_scissor_exclusive) {
scissor_exclusive_extension_version = ext_props[j].specVersion;
} else if (extension == vvl::Extension::_VK_EXT_robustness2 &&
IsExtEnabled(instance_extensions.vk_khr_get_physical_device_properties2)) {
VkPhysicalDeviceRobustness2FeaturesEXT robustness_2_features = vku::InitStructHelper();
VkPhysicalDeviceFeatures2 features2 = vku::InitStructHelper(&robustness_2_features);
DispatchGetPhysicalDeviceFeatures2Helper(phys_device, &features2);
device_supported_robustness_2_features[phys_device] = robustness_2_features;
}
}

Expand Down
57 changes: 0 additions & 57 deletions layers/stateless/sl_pipeline.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,6 @@ bool StatelessValidation::ValidatePipelineShaderStageCreateInfoCommon(const VkPi
string_VkPipelineShaderStageCreateFlags(create_info.flags).c_str());
}
}
if (const auto *pipeline_robustness_info = vku::FindStructInPNextChain<VkPipelineRobustnessCreateInfo>(create_info.pNext)) {
skip |= ValidatePipelineRobustnessCreateInfo(*pipeline_robustness_info, loc);
}
return skip;
}

Expand Down Expand Up @@ -260,50 +257,6 @@ bool StatelessValidation::ValidatePipelineRenderingCreateInfo(const VkPipelineRe
return skip;
}

bool StatelessValidation::ValidatePipelineRobustnessCreateInfo(const VkPipelineRobustnessCreateInfo &robustness_create_info,
const Location &loc) const {
bool skip = false;

VkPhysicalDeviceRobustness2FeaturesEXT supported_robustness_2_features = vku::InitStructHelper();

const auto &dev_robustness_enumerated = device_supported_robustness_2_features.find(physical_device);
if (dev_robustness_enumerated != device_supported_robustness_2_features.end()) {
supported_robustness_2_features = dev_robustness_enumerated->second;
}

if (!supported_robustness_2_features.robustBufferAccess2) {
if (robustness_create_info.storageBuffers == VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2) {
skip |= LogError(
"VUID-VkPipelineRobustnessCreateInfo-robustBufferAccess2-06931", device,
loc.pNext(Struct::VkPipelineRobustnessCreateInfo, Field::storageBuffers),
"is VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2, but robustBufferAccess2 is not supported.");
}
if (robustness_create_info.uniformBuffers == VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2) {
skip |= LogError(
"VUID-VkPipelineRobustnessCreateInfo-robustBufferAccess2-06932", device,
loc.pNext(Struct::VkPipelineRobustnessCreateInfo, Field::uniformBuffers),
"is VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2, but robustBufferAccess2 is not supported.");
}
if (robustness_create_info.vertexInputs == VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2) {
skip |= LogError(
"VUID-VkPipelineRobustnessCreateInfo-robustBufferAccess2-06933", device,
loc.pNext(Struct::VkPipelineRobustnessCreateInfo, Field::vertexInputs),
"is VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_2, but robustBufferAccess2 is not supported.");
}
}

if (!supported_robustness_2_features.robustImageAccess2) {
if (robustness_create_info.images == VK_PIPELINE_ROBUSTNESS_IMAGE_BEHAVIOR_ROBUST_IMAGE_ACCESS_2) {
skip |= LogError(
"VUID-VkPipelineRobustnessCreateInfo-robustImageAccess2-06934", device,
loc.pNext(Struct::VkPipelineRobustnessCreateInfo, Field::images),
"is VK_PIPELINE_ROBUSTNESS_IMAGE_BEHAVIOR_ROBUST_IMAGE_ACCESS_2, but robustImageAccess2 is not supported.");
}
}

return skip;
}

bool StatelessValidation::ValidateCreateGraphicsPipelinesFlags(const VkPipelineCreateFlags2KHR flags,
const Location &flags_loc) const {
bool skip = false;
Expand Down Expand Up @@ -1278,11 +1231,6 @@ bool StatelessValidation::manual_PreCallValidateCreateGraphicsPipelines(
}

skip |= ValidatePipelineBinaryInfo(create_info.pNext, create_info.flags, pipelineCache, create_info_loc);

if (const auto *pipeline_robustness_info =
vku::FindStructInPNextChain<VkPipelineRobustnessCreateInfo>(pCreateInfos[i].pNext)) {
skip |= ValidatePipelineRobustnessCreateInfo(*pipeline_robustness_info, create_info_loc);
}
}

return skip;
Expand Down Expand Up @@ -1420,11 +1368,6 @@ bool StatelessValidation::manual_PreCallValidateCreateComputePipelines(VkDevice
skip |= ValidatePipelineShaderStageCreateInfoCommon(create_info.stage, create_info_loc.dot(Field::stage));

skip |= ValidatePipelineBinaryInfo(create_info.pNext, create_info.flags, pipelineCache, create_info_loc);

if (const auto *pipeline_robustness_info =
vku::FindStructInPNextChain<VkPipelineRobustnessCreateInfo>(pCreateInfos[i].pNext)) {
skip |= ValidatePipelineRobustnessCreateInfo(*pipeline_robustness_info, create_info_loc);
}
}
return skip;
}
Expand Down
5 changes: 0 additions & 5 deletions layers/stateless/sl_ray_tracing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -655,11 +655,6 @@ bool StatelessValidation::manual_PreCallValidateCreateRayTracingPipelinesKHR(
}

skip |= ValidatePipelineBinaryInfo(create_info.pNext, create_info.flags, pipelineCache, create_info_loc);

if (const auto *pipeline_robustness_info =
vku::FindStructInPNextChain<VkPipelineRobustnessCreateInfo>(pCreateInfos[i].pNext)) {
skip |= ValidatePipelineRobustnessCreateInfo(*pipeline_robustness_info, create_info_loc);
}
}

return skip;
Expand Down
8 changes: 1 addition & 7 deletions layers/stateless/stateless_validation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,13 @@ class StatelessValidation : public ValidationObject {

public:
StatelessValidation(vvl::dispatch::Device *dev, StatelessValidation *instance_vo)
: BaseClass(dev, LayerObjectTypeParameterValidation),
device_supported_robustness_2_features(instance_vo->device_supported_robustness_2_features) {}
: BaseClass(dev, LayerObjectTypeParameterValidation) {}
StatelessValidation(vvl::dispatch::Instance *inst) : BaseClass(inst, LayerObjectTypeParameterValidation) {}
~StatelessValidation() {}

VkPhysicalDeviceLimits device_limits = {};
vvl::unordered_map<VkPhysicalDevice, VkPhysicalDeviceProperties *> physical_device_properties_map;
vvl::unordered_map<VkPhysicalDevice, vvl::unordered_set<vvl::Extension>> device_extensions_enumerated{};
// Validation requires special handling for VkPhysicalDeviceRobustness2FeaturesEXT, because for some cases robustness features
// need to only be supported, not enabled
vvl::unordered_map<VkPhysicalDevice, VkPhysicalDeviceRobustness2FeaturesEXT> device_supported_robustness_2_features{};
// We have a copy of this in Stateless and ValidationStateTracker, could move the ValidationObject, but we don't have a way to
// set it at the ValidationObject level
DeviceFeatures enabled_features = {};
Expand Down Expand Up @@ -462,8 +458,6 @@ class StatelessValidation : public ValidationObject {
bool ValidatePipelineBinaryInfo(const void *next, VkPipelineCreateFlags flags, VkPipelineCache pipelineCache,
const Location &loc) const;
bool ValidatePipelineRenderingCreateInfo(const VkPipelineRenderingCreateInfo &rendering_struct, const Location &loc) const;
bool ValidatePipelineRobustnessCreateInfo(const VkPipelineRobustnessCreateInfo &robustness_create_info,
const Location &loc) const;
bool ValidateCreateGraphicsPipelinesFlags(const VkPipelineCreateFlags2KHR flags, const Location &flags_loc) const;
bool manual_PreCallValidateCreateGraphicsPipelines(VkDevice device, VkPipelineCache pipelineCache, uint32_t createInfoCount,
const VkGraphicsPipelineCreateInfo *pCreateInfos,
Expand Down

0 comments on commit c4bb2de

Please sign in to comment.