Skip to content

Commit

Permalink
layers: Remove safe_VkDeviceCreateInfo
Browse files Browse the repository at this point in the history
  • Loading branch information
@spencer-lunarg
spencer-lunarg committed Jun 10, 2024
1 parent 9cd7020 commit 8cca8e4
Show file tree
Hide file tree
Showing 16 changed files with 264 additions and 209 deletions.
4 changes: 2 additions & 2 deletions layers/core_checks/cc_device.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,9 +437,9 @@ bool CoreChecks::PreCallValidateCreateDevice(VkPhysicalDevice gpu, const VkDevic
return skip;
}

void CoreChecks::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) {
void CoreChecks::PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) {
// The state tracker sets up the device state (also if extension and/or features are enabled)
StateTracker::CreateDevice(pCreateInfo, loc);
StateTracker::PostCreateDevice(pCreateInfo, loc);

// Add the callback hooks for the functions that are either broadly or deeply used and that the ValidationStateTracker refactor
// would be messier without.
Expand Down
2 changes: 1 addition & 1 deletion layers/core_checks/core_validation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@ class CoreChecks : public ValidationStateTracker {
bool PreCallValidateCreateDevice(VkPhysicalDevice gpu, const VkDeviceCreateInfo* pCreateInfo,
const VkAllocationCallbacks* pAllocator, VkDevice* pDevice,
const ErrorObject& error_obj) const override;
void CreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) override;
void PostCreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) override;
bool PreCallValidateCmdUpdateBuffer(VkCommandBuffer commandBuffer, VkBuffer dstBuffer, VkDeviceSize dstOffset,
VkDeviceSize dataSize, const void* pData, const ErrorObject& error_obj) const override;
bool PreCallValidateGetDeviceQueue(VkDevice device, uint32_t queueFamilyIndex, uint32_t queueIndex, VkQueue* pQueue,
Expand Down
12 changes: 8 additions & 4 deletions layers/gpu/core/gpuav.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ class Validator : public gpu::GpuShaderInstrumentor {
public:
Validator() {
container_type = LayerObjectTypeGpuAssisted;
desired_features_.vertexPipelineStoresAndAtomics = true;
desired_features_.fragmentStoresAndAtomics = true;
desired_features_.shaderInt64 = true;
force_buffer_device_address_ = true;
}

Expand Down Expand Up @@ -110,7 +107,10 @@ class Validator : public gpu::GpuShaderInstrumentor {
std::shared_ptr<vvl::Queue> CreateQueue(VkQueue q, uint32_t index, VkDeviceQueueCreateFlags flags,
const VkQueueFamilyProperties& queueFamilyProperties) final;

void CreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) final;
void PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo* pCreateInfo,
const VkAllocationCallbacks* pAllocator, VkDevice* pDevice, const RecordObject& record_obj,
VkDeviceCreateInfo* modified_create_info) final;
void PostCreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) final;

// gpuav_record.cpp
// --------------
Expand Down Expand Up @@ -363,6 +363,10 @@ class Validator : public gpu::GpuShaderInstrumentor {

private:
std::string instrumented_shader_cache_path_{};

// Need to live for the scope of vkCreateDevice
VkPhysicalDeviceBufferDeviceAddressFeatures new_bda_features;
VkPhysicalDeviceTimelineSemaphoreFeatures new_ts_features;
};

} // namespace gpuav
123 changes: 113 additions & 10 deletions layers/gpu/core/gpuav_setup.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "generated/layer_chassis_dispatch.h"
#include "containers/custom_containers.h"
#include "gpu_shaders/gpu_error_header.h"
#include "generated/chassis.h"

namespace gpuav {

Expand Down Expand Up @@ -74,8 +75,107 @@ std::shared_ptr<vvl::Queue> Validator::CreateQueue(VkQueue q, uint32_t index, Vk
return std::static_pointer_cast<vvl::Queue>(std::make_shared<Queue>(*this, q, index, flags, queueFamilyProperties));
}

static void AddExtension(VkDeviceCreateInfo &ci, const char *extension_name) {
char **exts = new char *[ci.enabledExtensionCount + 1];
memcpy(exts, ci.ppEnabledExtensionNames, sizeof(char *) * ci.enabledExtensionCount);
exts[ci.enabledExtensionCount] = vku::SafeStringCopy(extension_name);
delete[] ci.ppEnabledExtensionNames;
ci.ppEnabledExtensionNames = exts;
ci.enabledExtensionCount++;
return;
}

void Validator::PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo,
const VkAllocationCallbacks *pAllocator, VkDevice *pDevice,
const RecordObject &record_obj, VkDeviceCreateInfo *modified_create_info) {
BaseClass::PreCallRecordCreateDevice(physicalDevice, pCreateInfo, pAllocator, pDevice, record_obj, modified_create_info);

// Force enable required features
// If the features are not supported, can't internal error until post device creation
VkPhysicalDeviceFeatures supported_features{};
DispatchGetPhysicalDeviceFeatures(physicalDevice, &supported_features);

if (auto enabled_features = const_cast<VkPhysicalDeviceFeatures *>(modified_create_info->pEnabledFeatures)) {
if (supported_features.fragmentStoresAndAtomics) {
enabled_features->fragmentStoresAndAtomics = VK_TRUE;
}
if (supported_features.vertexPipelineStoresAndAtomics) {
enabled_features->vertexPipelineStoresAndAtomics = VK_TRUE;
}
}

auto add_missing_features = [this, modified_create_info]() {
if (force_buffer_device_address_) {
// Add buffer device address feature
auto *bda_features = const_cast<VkPhysicalDeviceBufferDeviceAddressFeatures *>(
vku::FindStructInPNextChain<VkPhysicalDeviceBufferDeviceAddressFeatures>(modified_create_info));
if (bda_features) {
bda_features->bufferDeviceAddress = VK_TRUE;
} else {
new_bda_features = vku::InitStructHelper();
new_bda_features.bufferDeviceAddress = VK_TRUE;
auto *chain_end = vku::FindLastStructInPNextChain((void *)modified_create_info->pNext);
chain_end->pNext = reinterpret_cast<VkBaseOutStructure *>(&new_bda_features);
}
}

// Add timeline semaphore feature
auto *ts_features = const_cast<VkPhysicalDeviceTimelineSemaphoreFeatures *>(
vku::FindStructInPNextChain<VkPhysicalDeviceTimelineSemaphoreFeatures>(modified_create_info));
if (ts_features) {
ts_features->timelineSemaphore = VK_TRUE;
} else {
new_ts_features = vku::InitStructHelper();
new_ts_features.timelineSemaphore = VK_TRUE;
auto *chain_end = vku::FindLastStructInPNextChain((void *)modified_create_info->pNext);
chain_end->pNext = reinterpret_cast<VkBaseOutStructure *>(&new_ts_features);
}
};

// TODO - multi-device is not supported
if (api_version > VK_API_VERSION_1_1) {
auto *features12 = const_cast<VkPhysicalDeviceVulkan12Features *>(
vku::FindStructInPNextChain<VkPhysicalDeviceVulkan12Features>(modified_create_info->pNext));
if (features12) {
features12->timelineSemaphore = VK_TRUE;
if (force_buffer_device_address_) {
features12->bufferDeviceAddress = VK_TRUE;
}
} else {
add_missing_features();
}
} else if (api_version == VK_API_VERSION_1_1) {
auto find_extension = [modified_create_info](const char *extension_name) {
for (uint32_t i = 0; i < modified_create_info->enabledExtensionCount; i++) {
if (strcmp(modified_create_info->ppEnabledExtensionNames[i], extension_name) == 0) {
return true;
}
}
return false;
};

const std::string_view bda_ext{"VK_KHR_buffer_device_address"};
const bool found_bda = find_extension(bda_ext.data());

const std::string_view ts_ext{"VK_KHR_timeline_semaphore"};
const bool found_ts = find_extension(ts_ext.data());

// Add our new extensions
if (!found_bda) {
AddExtension(*modified_create_info, bda_ext.data());
}
if (!found_ts) {
AddExtension(*modified_create_info, ts_ext.data());
}

add_missing_features();
} else {
force_buffer_device_address_ = false;
}
}

// Perform initializations that can be done at Create Device time.
void Validator::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) {
void Validator::PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) {
// Add the callback hooks for the functions that are either broadly or deeply used and that the ValidationStateTracker refactor
// would be messier without.
// TODO: Find a good way to do this hooklessly.
Expand Down Expand Up @@ -104,28 +204,31 @@ void Validator::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Locati

// TODO: Such a call is expected to be the first thing happening in this function,
// but moving it at the top breaks GPU-AV. Try to fix it
BaseClass::CreateDevice(pCreateInfo, loc);
BaseClass::PostCreateDevice(pCreateInfo, loc);

if (api_version < VK_API_VERSION_1_1) {
InternalError(device, loc, "GPU-Assisted validation requires Vulkan 1.1 or later. Aborting GPU-AV.");
return;
}

DispatchGetPhysicalDeviceFeatures(physical_device, &supported_features_);
if (!supported_features_.fragmentStoresAndAtomics || !supported_features_.vertexPipelineStoresAndAtomics) {
InternalError(
device, loc,
"GPU-Assisted validation requires fragmentStoresAndAtomics and vertexPipelineStoresAndAtomics. Aborting GPU-AV.");
VkPhysicalDeviceFeatures supported_features{};
DispatchGetPhysicalDeviceFeatures(physical_device, &supported_features);

if (!supported_features.fragmentStoresAndAtomics) {
InternalError(device, loc, "GPU-Assisted validation requires fragmentStoresAndAtomics. Aborting GPU-AV.");
return;
}
if (!supported_features.vertexPipelineStoresAndAtomics) {
InternalError(device, loc, "GPU-Assisted validation requires vertexPipelineStoresAndAtomics. Aborting GPU-AV.");
return;
}

const VkBool32 shaderInt64 = supported_features_.shaderInt64;
bda_validation_possible = ((IsExtEnabled(device_extensions.vk_ext_buffer_device_address) ||
IsExtEnabled(device_extensions.vk_khr_buffer_device_address)) &&
shaderInt64 && enabled_features.bufferDeviceAddress);
supported_features.shaderInt64 && enabled_features.bufferDeviceAddress);
if (!bda_validation_possible) {
if (gpuav_settings.validate_bda) {
if (!shaderInt64) {
if (!supported_features.shaderInt64) {
LogWarning("WARNING-GPU-Assisted-Validation", device, loc,
"Buffer device address validation option was enabled, but required features shaderInt64 is not enabled. "
"Disabling option.");
Expand Down
31 changes: 26 additions & 5 deletions layers/gpu/debug_printf/debug_printf.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,28 @@

namespace debug_printf {

void Validator::PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo,
const VkAllocationCallbacks *pAllocator, VkDevice *pDevice,
const RecordObject &record_obj, VkDeviceCreateInfo *modified_create_info) {
BaseClass::PreCallRecordCreateDevice(physicalDevice, pCreateInfo, pAllocator, pDevice, record_obj, modified_create_info);

// Force enable required features
// If the features are not supported, can't internal error until post device creation
VkPhysicalDeviceFeatures supported_features{};
DispatchGetPhysicalDeviceFeatures(physicalDevice, &supported_features);

if (auto enabled_features = const_cast<VkPhysicalDeviceFeatures *>(modified_create_info->pEnabledFeatures)) {
if (supported_features.fragmentStoresAndAtomics) {
enabled_features->fragmentStoresAndAtomics = VK_TRUE;
}
if (supported_features.vertexPipelineStoresAndAtomics) {
enabled_features->vertexPipelineStoresAndAtomics = VK_TRUE;
}
}
}

// Perform initializations that can be done at Create Device time.
void Validator::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) {
void Validator::PostCreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Location &loc) {
if (enabled[gpu_validation]) {
InternalError(device, loc, "Debug Printf cannot be enabled when gpu assisted validation is enabled.");
return;
Expand All @@ -49,19 +69,20 @@ void Validator::CreateDevice(const VkDeviceCreateInfo *pCreateInfo, const Locati
nullptr};
instrumentation_bindings_.push_back(binding);

BaseClass::CreateDevice(pCreateInfo, loc); // will set up bindings
BaseClass::PostCreateDevice(pCreateInfo, loc); // will set up bindings

if (api_version < VK_API_VERSION_1_1) {
InternalError(device, loc, "Debug Printf requires Vulkan 1.1 or later.");
return;
}

DispatchGetPhysicalDeviceFeatures(physical_device, &supported_features_);
if (!supported_features_.fragmentStoresAndAtomics) {
VkPhysicalDeviceFeatures supported_features{};
DispatchGetPhysicalDeviceFeatures(physical_device, &supported_features);
if (!supported_features.fragmentStoresAndAtomics) {
InternalError(device, loc, "Debug Printf requires fragmentStoresAndAtomics.");
return;
}
if (!supported_features_.vertexPipelineStoresAndAtomics) {
if (!supported_features.vertexPipelineStoresAndAtomics) {
InternalError(device, loc, "Debug Printf requires vertexPipelineStoresAndAtomics.");
return;
}
Expand Down
11 changes: 5 additions & 6 deletions layers/gpu/debug_printf/debug_printf.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,12 @@ namespace debug_printf {
class Validator : public gpu::GpuShaderInstrumentor {
public:
using BaseClass = gpu::GpuShaderInstrumentor;
Validator() {
container_type = LayerObjectTypeDebugPrintf;
desired_features_.vertexPipelineStoresAndAtomics = true;
desired_features_.fragmentStoresAndAtomics = true;
}
Validator() { container_type = LayerObjectTypeDebugPrintf; }

void CreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) override;
void PreCallRecordCreateDevice(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo* pCreateInfo,
const VkAllocationCallbacks* pAllocator, VkDevice* pDevice, const RecordObject& record_obj,
VkDeviceCreateInfo* modified_create_info) override;
void PostCreateDevice(const VkDeviceCreateInfo* pCreateInfo, const Location& loc) override;
bool InstrumentShader(const vvl::span<const uint32_t>& input, uint32_t unique_shader_id, const Location& loc,
std::vector<uint32_t>& out_instrumented_spirv) override;
void PreCallRecordCreateShaderModule(VkDevice device, const VkShaderModuleCreateInfo* pCreateInfo,
Expand Down
Loading

0 comments on commit 8cca8e4

Please sign in to comment.