Initial support for VK_EXT_pipeline_robustness
In this initial implementation, enabling robustBufferAccess on any of
storage, uniform or vertex buffers makes it enabled for all of them.
This can be optimized in a future change, by appropriately tracking
separate robustBufferAccess flags for storage, uniform and vertex
buffers and using the right flag for the right buffer.
Bug: b/185122256
Tests: dEQP-VK.robustness.pipeline_robustness*
Change-Id: Idb9f841e2a850b866cdfbc1d558efb2d33203c4d
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/67948
Presubmit-Ready: Shahbaz Youssefi <syoussefi@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Shahbaz Youssefi <syoussefi@google.com>
Commit-Queue: Shahbaz Youssefi <syoussefi@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/Device/Context.cpp b/src/Device/Context.cpp
index a338198..65b9d97 100644
--- a/src/Device/Context.cpp
+++ b/src/Device/Context.cpp
@@ -382,9 +382,8 @@
}
GraphicsState::GraphicsState(const Device *device, const VkGraphicsPipelineCreateInfo *pCreateInfo,
- const PipelineLayout *layout, bool robustBufferAccess)
+ const PipelineLayout *layout)
: pipelineLayout(layout)
- , robustBufferAccess(robustBufferAccess)
, dynamicStateFlags(ParseDynamicStateFlags(pCreateInfo->pDynamicState))
{
if((pCreateInfo->flags &
diff --git a/src/Device/Context.hpp b/src/Device/Context.hpp
index f3173b5..cbd4a0b 100644
--- a/src/Device/Context.hpp
+++ b/src/Device/Context.hpp
@@ -154,12 +154,11 @@
struct GraphicsState
{
- GraphicsState(const Device *device, const VkGraphicsPipelineCreateInfo *pCreateInfo, const PipelineLayout *layout, bool robustBufferAccess);
+ GraphicsState(const Device *device, const VkGraphicsPipelineCreateInfo *pCreateInfo, const PipelineLayout *layout);
const GraphicsState combineStates(const DynamicState &dynamicState) const;
inline const PipelineLayout *getPipelineLayout() const { return pipelineLayout; }
- inline bool getRobustBufferAccess() const { return robustBufferAccess; }
inline VkPrimitiveTopology getTopology() const { return topology; }
inline VkProvokingVertexModeEXT getProvokingVertexMode() const { return provokingVertexMode; }
@@ -255,7 +254,6 @@
bool colorWriteActive(const Attachments &attachments) const;
const PipelineLayout *pipelineLayout = nullptr;
- const bool robustBufferAccess = false;
const DynamicStateFlags dynamicStateFlags = {};
VkPrimitiveTopology topology = VK_PRIMITIVE_TOPOLOGY_POINT_LIST;
diff --git a/src/Device/VertexProcessor.cpp b/src/Device/VertexProcessor.cpp
index 42df2d2..23e8e9d 100644
--- a/src/Device/VertexProcessor.cpp
+++ b/src/Device/VertexProcessor.cpp
@@ -71,7 +71,7 @@
state.shaderID = vertexShader->getIdentifier();
state.pipelineLayoutIdentifier = pipelineState.getPipelineLayout()->identifier;
- state.robustBufferAccess = pipelineState.getRobustBufferAccess();
+ state.robustBufferAccess = vertexShader->getRobustBufferAccess();
state.isPoint = pipelineState.getTopology() == VK_PRIMITIVE_TOPOLOGY_POINT_LIST;
state.depthClipEnable = pipelineState.getDepthClipEnable();
state.depthClipNegativeOneToOne = pipelineState.getDepthClipNegativeOneToOne();
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 74dc1c6..ceaa72f 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -951,6 +951,7 @@
uint32_t getWorkgroupSizeZ() const;
bool containsImageWrite() const { return imageWriteEmitted; }
+ bool getRobustBufferAccess() const { return robustBufferAccess; }
using BuiltInHash = std::hash<std::underlying_type<spv::BuiltIn>::type>;
std::unordered_map<spv::BuiltIn, BuiltinMapping, BuiltInHash> inputBuiltins;
diff --git a/src/Vulkan/VkPhysicalDevice.cpp b/src/Vulkan/VkPhysicalDevice.cpp
index 499f1ee..a7352a3 100644
--- a/src/Vulkan/VkPhysicalDevice.cpp
+++ b/src/Vulkan/VkPhysicalDevice.cpp
@@ -377,6 +377,12 @@
}
template<typename T>
+static void getPhysicalDevicePipelineRobustnessFeatures(T *features)
+{
+ features->pipelineRobustness = VK_TRUE;
+}
+
+template<typename T>
static void getPhysicalDeviceVulkan12Features(T *features)
{
features->samplerMirrorClampToEdge = VK_TRUE;
@@ -601,6 +607,9 @@
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PRIMITIVE_TOPOLOGY_LIST_RESTART_FEATURES_EXT:
getPhysicalDevicePrimitiveTopologyListRestartFeatures(reinterpret_cast<struct VkPhysicalDevicePrimitiveTopologyListRestartFeaturesEXT *>(curExtension));
break;
+ case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PIPELINE_ROBUSTNESS_FEATURES_EXT:
+ getPhysicalDevicePipelineRobustnessFeatures(reinterpret_cast<struct VkPhysicalDevicePipelineRobustnessFeaturesEXT *>(curExtension));
+ break;
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_FEATURES_EXT:
getPhysicalDeviceRasterizationOrderAttachmentAccessFeaturesEXT(reinterpret_cast<struct VkPhysicalDeviceRasterizationOrderAttachmentAccessFeaturesEXT *>(curExtension));
break;
@@ -1338,6 +1347,24 @@
getTimelineSemaphoreProperties(properties);
}
+template<typename T>
+static void getPipelineRobustnessProperties(T *properties)
+{
+ // Buffer access is not robust by default.
+ properties->defaultRobustnessStorageBuffers = VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_DISABLED_EXT;
+ properties->defaultRobustnessUniformBuffers = VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_DISABLED_EXT;
+ properties->defaultRobustnessVertexInputs = VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_DISABLED_EXT;
+ // SwiftShader currently provides robustImageAccess robustness unconditionally.
+ // robustImageAccess2 is not supported.
+ // TODO(b/162327166): Only provide robustness when requested.
+ properties->defaultRobustnessImages = VK_PIPELINE_ROBUSTNESS_IMAGE_BEHAVIOR_ROBUST_IMAGE_ACCESS_EXT;
+}
+
+void PhysicalDevice::getProperties(VkPhysicalDevicePipelineRobustnessPropertiesEXT *properties) const
+{
+ getPipelineRobustnessProperties(properties);
+}
+
void PhysicalDevice::getProperties(VkPhysicalDeviceVulkan12Properties *properties) const
{
getDriverProperties(properties);
@@ -1618,6 +1645,13 @@
CheckFeature(requested, supported, descriptorBindingVariableDescriptorCount) &&
CheckFeature(requested, supported, runtimeDescriptorArray);
}
+
+bool PhysicalDevice::hasExtendedFeatures(const VkPhysicalDevicePipelineRobustnessFeaturesEXT *requested) const
+{
+ auto supported = getSupportedFeatures(requested);
+
+ return CheckFeature(requested, supported, pipelineRobustness);
+}
#undef CheckFeature
void PhysicalDevice::GetFormatProperties(Format format, VkFormatProperties *pFormatProperties)
diff --git a/src/Vulkan/VkPhysicalDevice.hpp b/src/Vulkan/VkPhysicalDevice.hpp
index 21a519d..40acb1f 100644
--- a/src/Vulkan/VkPhysicalDevice.hpp
+++ b/src/Vulkan/VkPhysicalDevice.hpp
@@ -56,6 +56,7 @@
bool hasExtendedFeatures(const VkPhysicalDeviceZeroInitializeWorkgroupMemoryFeatures *requested) const;
bool hasExtendedFeatures(const VkPhysicalDevicePrimitiveTopologyListRestartFeaturesEXT *requested) const;
bool hasExtendedFeatures(const VkPhysicalDeviceDescriptorIndexingFeatures *requested) const;
+ bool hasExtendedFeatures(const VkPhysicalDevicePipelineRobustnessFeaturesEXT *requested) const;
const VkPhysicalDeviceProperties &getProperties() const;
void getProperties(VkPhysicalDeviceIDProperties *properties) const;
@@ -82,8 +83,6 @@
void getProperties(VkPhysicalDeviceFloatControlsProperties *) const;
void getProperties(VkPhysicalDeviceSamplerFilterMinmaxProperties *properties) const;
void getProperties(VkPhysicalDeviceTimelineSemaphoreProperties *properties) const;
- void getProperties(VkPhysicalDeviceVulkan12Properties *properties) const;
- void getProperties(VkPhysicalDeviceVulkan13Properties *properties) const;
void getProperties(VkPhysicalDeviceDescriptorIndexingProperties *properties) const;
void getProperties(VkPhysicalDeviceDepthStencilResolveProperties *properties) const;
void getProperties(VkPhysicalDeviceCustomBorderColorPropertiesEXT *properties) const;
@@ -92,7 +91,10 @@
void getProperties(VkPhysicalDeviceInlineUniformBlockProperties *properties) const;
void getProperties(VkPhysicalDeviceTexelBufferAlignmentProperties *properties) const;
void getProperties(VkPhysicalDeviceShaderIntegerDotProductProperties *properties) const;
+ void getProperties(VkPhysicalDevicePipelineRobustnessPropertiesEXT *properties) const;
void getProperties(VkPhysicalDeviceVulkan11Properties *properties) const;
+ void getProperties(VkPhysicalDeviceVulkan12Properties *properties) const;
+ void getProperties(VkPhysicalDeviceVulkan13Properties *properties) const;
static void GetFormatProperties(Format format, VkFormatProperties *pFormatProperties);
static void GetFormatProperties(Format format, VkFormatProperties3 *pFormatProperties);
diff --git a/src/Vulkan/VkPipeline.cpp b/src/Vulkan/VkPipeline.cpp
index 66a3c21..07701c5 100644
--- a/src/Vulkan/VkPipeline.cpp
+++ b/src/Vulkan/VkPipeline.cpp
@@ -240,14 +240,85 @@
const VkPipelineCreationFeedbackCreateInfo *pipelineCreationFeedback = nullptr;
};
+bool getRobustBufferAccess(VkPipelineRobustnessBufferBehaviorEXT behavior, bool inheritRobustBufferAccess)
+{
+ // Based on behavior:
+ // - <not provided>:
+ // * For pipelines, use device's robustBufferAccess
+ // * For shaders, use pipeline's robustBufferAccess
+ // Note that pipeline's robustBufferAccess is already set to device's if not overriden.
+ // - Default: Use device's robustBufferAccess
+ // - Disabled / Enabled: Override to disabled or enabled
+ //
+ // This function is passed "DEFAULT" when override is not provided, and
+ // inheritRobustBufferAccess is appropriately set to the device or pipeline's
+ // robustBufferAccess
+ switch(behavior)
+ {
+ case VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_DEVICE_DEFAULT_EXT:
+ return inheritRobustBufferAccess;
+ case VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_DISABLED_EXT:
+ return false;
+ case VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_EXT:
+ return true;
+ default:
+ UNSUPPORTED("Unsupported robustness behavior");
+ return true;
+ }
+}
+
+bool getRobustBufferAccess(const VkPipelineRobustnessCreateInfoEXT *overrideRobustness, bool deviceRobustBufferAccess, bool inheritRobustBufferAccess)
+{
+ VkPipelineRobustnessBufferBehaviorEXT storageBehavior = VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_DEVICE_DEFAULT_EXT;
+ VkPipelineRobustnessBufferBehaviorEXT uniformBehavior = VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_DEVICE_DEFAULT_EXT;
+ VkPipelineRobustnessBufferBehaviorEXT vertexBehavior = VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_DEVICE_DEFAULT_EXT;
+
+ if(overrideRobustness)
+ {
+ storageBehavior = overrideRobustness->storageBuffers;
+ uniformBehavior = overrideRobustness->uniformBuffers;
+ vertexBehavior = overrideRobustness->vertexInputs;
+ }
+ else
+ {
+ inheritRobustBufferAccess = deviceRobustBufferAccess;
+ }
+
+ bool storageRobustBufferAccess = getRobustBufferAccess(storageBehavior, inheritRobustBufferAccess);
+ bool uniformRobustBufferAccess = getRobustBufferAccess(uniformBehavior, inheritRobustBufferAccess);
+ bool vertexRobustBufferAccess = getRobustBufferAccess(vertexBehavior, inheritRobustBufferAccess);
+
+ // Note: in the initial implementation, enabling robust access for any buffer enables it for
+ // all. TODO(b/185122256) split robustBufferAccess in the pipeline and shaders into three
+ // categories and provide robustness for storage, uniform and vertex buffers accordingly.
+ return storageRobustBufferAccess || uniformRobustBufferAccess || vertexRobustBufferAccess;
+}
+
+bool getPipelineRobustBufferAccess(const void *pNext, vk::Device *device)
+{
+ const VkPipelineRobustnessCreateInfoEXT *overrideRobustness = vk::GetExtendedStruct<VkPipelineRobustnessCreateInfoEXT>(pNext, VK_STRUCTURE_TYPE_PIPELINE_ROBUSTNESS_CREATE_INFO_EXT);
+ const bool deviceRobustBufferAccess = device->getEnabledFeatures().robustBufferAccess;
+
+ // For pipelines, there's no robustBufferAccess to inherit from. Default and no-override
+ // both lead to using the device's robustBufferAccess.
+ return getRobustBufferAccess(overrideRobustness, deviceRobustBufferAccess, deviceRobustBufferAccess);
+}
+
+bool getPipelineStageRobustBufferAccess(const void *pNext, vk::Device *device, bool pipelineRobustBufferAccess)
+{
+ const VkPipelineRobustnessCreateInfoEXT *overrideRobustness = vk::GetExtendedStruct<VkPipelineRobustnessCreateInfoEXT>(pNext, VK_STRUCTURE_TYPE_PIPELINE_ROBUSTNESS_CREATE_INFO_EXT);
+ const bool deviceRobustBufferAccess = device->getEnabledFeatures().robustBufferAccess;
+
+ return getRobustBufferAccess(overrideRobustness, deviceRobustBufferAccess, pipelineRobustBufferAccess);
+}
+
} // anonymous namespace
namespace vk {
-
-Pipeline::Pipeline(PipelineLayout *layout, Device *device)
+Pipeline::Pipeline(PipelineLayout *layout, Device *device, bool robustBufferAccess)
: layout(layout)
, device(device)
- , robustBufferAccess(device->getEnabledFeatures().robustBufferAccess)
+ , robustBufferAccess(robustBufferAccess)
{
layout->incRefCount();
}
@@ -260,8 +331,8 @@
}
GraphicsPipeline::GraphicsPipeline(const VkGraphicsPipelineCreateInfo *pCreateInfo, void *mem, Device *device)
- : Pipeline(vk::Cast(pCreateInfo->layout), device)
- , state(device, pCreateInfo, layout, robustBufferAccess)
+ : Pipeline(vk::Cast(pCreateInfo->layout), device, getPipelineRobustBufferAccess(pCreateInfo->pNext, device))
+ , state(device, pCreateInfo, layout)
, inputs(pCreateInfo->pVertexInputState)
{
}
@@ -376,9 +447,11 @@
}
}
+ const bool stageRobustBufferAccess = getPipelineStageRobustBufferAccess(stageInfo.pNext, device, robustBufferAccess);
+
// TODO(b/201798871): use allocator.
auto shader = std::make_shared<sw::SpirvShader>(stageInfo.stage, stageInfo.pName, spirv,
- vk::Cast(pCreateInfo->renderPass), pCreateInfo->subpass, robustBufferAccess, dbgctx, getOrCreateSpirvProfiler());
+ vk::Cast(pCreateInfo->renderPass), pCreateInfo->subpass, stageRobustBufferAccess, dbgctx, getOrCreateSpirvProfiler());
setShader(stageInfo.stage, shader);
@@ -389,7 +462,7 @@
}
ComputePipeline::ComputePipeline(const VkComputePipelineCreateInfo *pCreateInfo, void *mem, Device *device)
- : Pipeline(vk::Cast(pCreateInfo->layout), device)
+ : Pipeline(vk::Cast(pCreateInfo->layout), device, getPipelineRobustBufferAccess(pCreateInfo->pNext, device))
{
}
@@ -450,9 +523,11 @@
}
}
+ const bool stageRobustBufferAccess = getPipelineStageRobustBufferAccess(stage.pNext, device, robustBufferAccess);
+
// TODO(b/201798871): use allocator.
shader = std::make_shared<sw::SpirvShader>(stage.stage, stage.pName, spirv,
- nullptr, 0, robustBufferAccess, dbgctx, getOrCreateSpirvProfiler());
+ nullptr, 0, stageRobustBufferAccess, dbgctx, getOrCreateSpirvProfiler());
const PipelineCache::ComputeProgramKey programKey(shader->getIdentifier(), layout->identifier);
diff --git a/src/Vulkan/VkPipeline.hpp b/src/Vulkan/VkPipeline.hpp
index 39e605d..153329b 100644
--- a/src/Vulkan/VkPipeline.hpp
+++ b/src/Vulkan/VkPipeline.hpp
@@ -37,7 +37,7 @@
class Pipeline
{
public:
- Pipeline(PipelineLayout *layout, Device *device);
+ Pipeline(PipelineLayout *layout, Device *device, bool robustBufferAccess);
virtual ~Pipeline() = default;
operator VkPipeline()
diff --git a/src/Vulkan/libVulkan.cpp b/src/Vulkan/libVulkan.cpp
index 43fb688..c1b8e63 100644
--- a/src/Vulkan/libVulkan.cpp
+++ b/src/Vulkan/libVulkan.cpp
@@ -424,6 +424,7 @@
{ { VK_EXT_BLEND_OPERATION_ADVANCED_EXTENSION_NAME, VK_EXT_BLEND_OPERATION_ADVANCED_SPEC_VERSION } },
// Used by ANGLE to implement triangle/etc list restarts as possible in OpenGL
{ { VK_EXT_PRIMITIVE_TOPOLOGY_LIST_RESTART_EXTENSION_NAME, VK_EXT_PRIMITIVE_TOPOLOGY_LIST_RESTART_SPEC_VERSION } },
+ { { VK_EXT_PIPELINE_ROBUSTNESS_EXTENSION_NAME, VK_EXT_PIPELINE_ROBUSTNESS_SPEC_VERSION } },
{ { VK_EXT_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_EXTENSION_NAME, VK_EXT_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_SPEC_VERSION } }
};
@@ -1064,7 +1065,7 @@
}
}
break;
- // These structs are supported, but no behavior changes based on their feature bools
+ // These structs are supported, but no behavior changes based on their feature flags
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_UNIFORM_BUFFER_STANDARD_LAYOUT_FEATURES:
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_SUBGROUP_EXTENDED_TYPES_FEATURES:
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_4444_FORMATS_FEATURES_EXT:
@@ -1073,6 +1074,7 @@
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_MAINTENANCE_4_FEATURES:
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_FEATURES_EXT:
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_DEPTH_CLIP_CONTROL_FEATURES_EXT:
+ case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PIPELINE_ROBUSTNESS_FEATURES_EXT:
break;
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_TEXEL_BUFFER_ALIGNMENT_FEATURES_EXT:
// Workaround for a test bug (see https://gitlab.khronos.org/Tracker/vk-gl-cts/-/issues/3564)
@@ -3685,6 +3687,12 @@
vk::Cast(physicalDevice)->getProperties(properties);
}
break;
+ case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PIPELINE_ROBUSTNESS_PROPERTIES_EXT:
+ {
+ auto properties = reinterpret_cast<VkPhysicalDevicePipelineRobustnessPropertiesEXT *>(extensionProperties);
+ vk::Cast(physicalDevice)->getProperties(properties);
+ }
+ break;
default:
// "the [driver] must skip over, without processing (other than reading the sType and pNext members) any structures in the chain with sType values not defined by [supported extenions]"
UNSUPPORTED("pProperties->pNext sType = %s", vk::Stringify(extensionProperties->sType).c_str());
