Make shader validation under a separate feature (#1437)

This commit is contained in:
Igor Shaposhnik
2021-10-05 22:11:32 +03:00
committed by GitHub
parent da00bf2be6
commit 3e1244c5cb
9 changed files with 105 additions and 30 deletions

View File

@@ -149,3 +149,14 @@ jobs:
echo "Result: $(expr $FILE_COUNT - $SUCCESS_RESULT_COUNT) / $FILE_COUNT" > counter
done
cat counter
check_snapshots_without_validation:
name: Check snapshots (without validation)
runs-on: ubuntu-latest
steps:
- uses: actions-rs/cargo@v1
name: Test all (without validation)
with:
command: test
args: --features wgsl-in,wgsl-out,glsl-in,glsl-out,spv-in,spv-out,msl-out,hlsl-out,dot-out --workspace
- name: Check snapshots (without validation)
run: git diff --exit-code -- tests/out

View File

@@ -45,6 +45,7 @@ wgsl-in = ["codespan-reporting", "hexf-parse"]
wgsl-out = []
hlsl-out = []
span = ["codespan-reporting"]
validate = []
[dev-dependencies]
diff = "0.1"

View File

@@ -14,7 +14,7 @@ name = "naga"
path = "src/main.rs"
[dependencies]
naga = { path = "../", features = ["span", "wgsl-in", "wgsl-out", "glsl-in", "glsl-out", "spv-in", "spv-out", "msl-out", "hlsl-out", "dot-out", "glsl-validate"] }
naga = { path = "../", features = ["validate", "span", "wgsl-in", "wgsl-out", "glsl-in", "glsl-out", "spv-in", "spv-out", "msl-out", "hlsl-out", "dot-out", "glsl-validate"] }
log = "0.4"
codespan-reporting = "0.11"
env_logger = "0.8"

View File

@@ -190,6 +190,7 @@ struct Sampling {
#[cfg_attr(feature = "deserialize", derive(serde::Deserialize))]
pub struct FunctionInfo {
/// Validation flags.
#[allow(dead_code)]
flags: ValidationFlags,
/// Set of shader stages where calling this function is valid.
pub available_stages: ShaderStages,
@@ -621,6 +622,7 @@ impl FunctionInfo {
let mut requirements = UniformityRequirements::empty();
for expr in range.clone() {
let req = self.expressions[expr.index()].uniformity.requirements;
#[cfg(feature = "validate")]
if self
.flags
.contains(super::ValidationFlags::CONTROL_FLOW_UNIFORMITY)
@@ -854,6 +856,7 @@ impl ModuleInfo {
}
#[test]
#[cfg(feature = "validate")]
fn uniform_control_flow() {
use crate::{Expression as E, Statement as S};

View File

@@ -1,8 +1,11 @@
#[cfg(feature = "validate")]
use crate::{
arena::{Arena, Handle, UniqueArena},
arena::{Arena, UniqueArena},
proc::TypeResolution,
};
use crate::Handle;
#[derive(Clone, Debug, thiserror::Error)]
#[cfg_attr(test, derive(PartialEq))]
pub enum ComposeError {
@@ -16,6 +19,7 @@ pub enum ComposeError {
ComponentType { index: u32 },
}
#[cfg(feature = "validate")]
pub fn validate_compose(
self_ty_handle: Handle<crate::Type>,
constant_arena: &Arena<crate::Constant>,

View File

@@ -1,6 +1,10 @@
use super::{compose::validate_compose, ComposeError, FunctionInfo, ShaderStages, TypeFlags};
#[cfg(feature = "validate")]
use super::{compose::validate_compose, FunctionInfo, ShaderStages, TypeFlags};
#[cfg(feature = "validate")]
use crate::arena::UniqueArena;
use crate::{
arena::{Handle, UniqueArena},
arena::Handle,
proc::{ProcError, ResolveError},
};
@@ -40,7 +44,7 @@ pub enum ExpressionError {
#[error("Swizzle component {0:?} is outside of vector size {1:?}")]
InvalidSwizzleComponent(crate::SwizzleComponent, crate::VectorSize),
#[error(transparent)]
Compose(#[from] ComposeError),
Compose(#[from] super::ComposeError),
#[error(transparent)]
Proc(#[from] ProcError),
#[error("Operation {0:?} can't work with {1:?}")]
@@ -111,12 +115,14 @@ pub enum ExpressionError {
InvalidAtomicResultType(crate::ScalarKind, crate::Bytes),
}
#[cfg(feature = "validate")]
struct ExpressionTypeResolver<'a> {
root: Handle<crate::Expression>,
types: &'a UniqueArena<crate::Type>,
info: &'a FunctionInfo,
}
#[cfg(feature = "validate")]
impl<'a> ExpressionTypeResolver<'a> {
fn resolve(
&self,
@@ -130,6 +136,7 @@ impl<'a> ExpressionTypeResolver<'a> {
}
}
#[cfg(feature = "validate")]
impl super::Validator {
pub(super) fn validate_expression(
&self,

View File

@@ -1,8 +1,13 @@
use crate::arena::Handle;
#[cfg(feature = "validate")]
use crate::arena::{Arena, UniqueArena};
use super::{
analyzer::{UniformityDisruptor, UniformityRequirements},
ExpressionError, FunctionInfo, ModuleInfo, ShaderStages, TypeFlags, ValidationFlags,
ExpressionError, FunctionInfo, ModuleInfo,
};
use crate::arena::{Arena, Handle, UniqueArena};
#[cfg(feature = "validate")]
use bit_set::BitSet;
#[derive(Clone, Debug, thiserror::Error)]
@@ -141,11 +146,13 @@ bitflags::bitflags! {
}
}
#[cfg(feature = "validate")]
struct BlockInfo {
stages: ShaderStages,
stages: super::ShaderStages,
finished: bool,
}
#[cfg(feature = "validate")]
struct BlockContext<'a> {
abilities: ControlFlowAbility,
info: &'a FunctionInfo,
@@ -157,6 +164,7 @@ struct BlockContext<'a> {
return_type: Option<Handle<crate::Type>>,
}
#[cfg(feature = "validate")]
impl<'a> BlockContext<'a> {
fn new(
fun: &'a crate::Function,
@@ -228,13 +236,14 @@ impl<'a> BlockContext<'a> {
}
impl super::Validator {
#[cfg(feature = "validate")]
fn validate_call(
&mut self,
function: Handle<crate::Function>,
arguments: &[Handle<crate::Expression>],
result: Option<Handle<crate::Expression>>,
context: &BlockContext,
) -> Result<ShaderStages, CallError> {
) -> Result<super::ShaderStages, CallError> {
let fun = context
.functions
.try_get(function)
@@ -278,6 +287,7 @@ impl super::Validator {
Ok(callee_info.available_stages)
}
#[cfg(feature = "validate")]
fn validate_atomic(
&mut self,
pointer: Handle<crate::Expression>,
@@ -334,6 +344,7 @@ impl super::Validator {
Ok(())
}
#[cfg(feature = "validate")]
fn validate_block_impl(
&mut self,
statements: &[crate::Statement],
@@ -341,7 +352,7 @@ impl super::Validator {
) -> Result<BlockInfo, FunctionError> {
use crate::{Statement as S, TypeInner as Ti};
let mut finished = false;
let mut stages = ShaderStages::all();
let mut stages = super::ShaderStages::all();
for statement in statements {
if finished {
return Err(FunctionError::InstructionsAfterReturn);
@@ -479,7 +490,7 @@ impl super::Validator {
finished = true;
}
S::Barrier(_) => {
stages &= ShaderStages::COMPUTE;
stages &= super::ShaderStages::COMPUTE;
}
S::Store { pointer, value } => {
let mut current = pointer;
@@ -628,6 +639,7 @@ impl super::Validator {
Ok(BlockInfo { stages, finished })
}
#[cfg(feature = "validate")]
fn validate_block(
&mut self,
statements: &[crate::Statement],
@@ -641,6 +653,7 @@ impl super::Validator {
Ok(info)
}
#[cfg(feature = "validate")]
fn validate_local_var(
&self,
var: &crate::LocalVariable,
@@ -650,7 +663,7 @@ impl super::Validator {
log::debug!("var {:?}", var);
if !self.types[var.ty.index()]
.flags
.contains(TypeFlags::DATA | TypeFlags::SIZED)
.contains(super::TypeFlags::DATA | super::TypeFlags::SIZED)
{
return Err(LocalVariableError::InvalidType(var.ty));
}
@@ -681,8 +694,13 @@ impl super::Validator {
module: &crate::Module,
mod_info: &ModuleInfo,
) -> Result<FunctionInfo, FunctionError> {
#[cfg(feature = "validate")]
let mut info = mod_info.process_function(fun, module, self.flags)?;
#[cfg(not(feature = "validate"))]
let info = mod_info.process_function(fun, module, self.flags)?;
#[cfg(feature = "validate")]
for (var_handle, var) in fun.local_variables.iter() {
self.validate_local_var(var, &module.types, &module.constants)
.map_err(|error| FunctionError::LocalVariable {
@@ -692,10 +710,11 @@ impl super::Validator {
})?;
}
#[cfg(feature = "validate")]
for (index, argument) in fun.arguments.iter().enumerate() {
if !self.types[argument.ty.index()]
.flags
.contains(TypeFlags::ARGUMENT)
.contains(super::TypeFlags::ARGUMENT)
{
return Err(FunctionError::InvalidArgumentType {
index,
@@ -723,7 +742,8 @@ impl super::Validator {
if expr.needs_pre_emit() {
self.valid_expression_set.insert(handle.index());
}
if self.flags.contains(ValidationFlags::EXPRESSIONS) {
#[cfg(feature = "validate")]
if self.flags.contains(super::ValidationFlags::EXPRESSIONS) {
match self.validate_expression(
handle,
expr,
@@ -738,7 +758,8 @@ impl super::Validator {
}
}
if self.flags.contains(ValidationFlags::BLOCKS) {
#[cfg(feature = "validate")]
if self.flags.contains(super::ValidationFlags::BLOCKS) {
let stages = self
.validate_block(
&fun.body,

View File

@@ -1,12 +1,12 @@
use super::{
analyzer::{FunctionInfo, GlobalUse},
Capabilities, Disalignment, FunctionError, ModuleInfo, ShaderStages, TypeFlags,
ValidationFlags,
Capabilities, Disalignment, FunctionError, ModuleInfo,
};
use crate::arena::{Handle, UniqueArena};
use bit_set::BitSet;
#[cfg(feature = "validate")]
const MAX_WORKGROUP_SIZE: u32 = 0x4000;
#[derive(Clone, Debug, thiserror::Error)]
@@ -17,8 +17,8 @@ pub enum GlobalVariableError {
InvalidType,
#[error("Type flags {seen:?} do not meet the required {required:?}")]
MissingTypeFlags {
required: TypeFlags,
seen: TypeFlags,
required: super::TypeFlags,
seen: super::TypeFlags,
},
#[error("Capability {0:?} is not supported")]
UnsupportedCapability(Capabilities),
@@ -78,6 +78,7 @@ pub enum EntryPointError {
Function(#[from] FunctionError),
}
#[cfg(feature = "validate")]
fn storage_usage(access: crate::StorageAccess) -> GlobalUse {
let mut storage_usage = GlobalUse::QUERY;
if access.contains(crate::StorageAccess::LOAD) {
@@ -317,11 +318,14 @@ impl VaryingContext<'_> {
}
impl super::Validator {
#[cfg(feature = "validate")]
pub(super) fn validate_global_var(
&self,
var: &crate::GlobalVariable,
types: &UniqueArena<crate::Type>,
) -> Result<(), GlobalVariableError> {
use super::TypeFlags;
log::debug!("var {:?}", var);
let type_info = &self.types[var.ty.index()];
@@ -329,7 +333,7 @@ impl super::Validator {
crate::StorageClass::Function => return Err(GlobalVariableError::InvalidUsage),
crate::StorageClass::Storage { .. } => {
if let Err((ty_handle, disalignment)) = type_info.storage_layout {
if self.flags.contains(ValidationFlags::STRUCT_LAYOUTS) {
if self.flags.contains(super::ValidationFlags::STRUCT_LAYOUTS) {
return Err(GlobalVariableError::Alignment(ty_handle, disalignment));
}
}
@@ -340,7 +344,7 @@ impl super::Validator {
}
crate::StorageClass::Uniform => {
if let Err((ty_handle, disalignment)) = type_info.uniform_layout {
if self.flags.contains(ValidationFlags::STRUCT_LAYOUTS) {
if self.flags.contains(super::ValidationFlags::STRUCT_LAYOUTS) {
return Err(GlobalVariableError::Alignment(ty_handle, disalignment));
}
}
@@ -396,9 +400,12 @@ impl super::Validator {
module: &crate::Module,
mod_info: &ModuleInfo,
) -> Result<FunctionInfo, EntryPointError> {
#[cfg(feature = "validate")]
if ep.early_depth_test.is_some() && ep.stage != crate::ShaderStage::Fragment {
return Err(EntryPointError::UnexpectedEarlyDepthTest);
}
#[cfg(feature = "validate")]
if ep.stage == crate::ShaderStage::Compute {
if ep
.workgroup_size
@@ -411,16 +418,21 @@ impl super::Validator {
return Err(EntryPointError::UnexpectedWorkgroupSize);
}
let stage_bit = match ep.stage {
crate::ShaderStage::Vertex => ShaderStages::VERTEX,
crate::ShaderStage::Fragment => ShaderStages::FRAGMENT,
crate::ShaderStage::Compute => ShaderStages::COMPUTE,
};
let info = self.validate_function(&ep.function, module, mod_info)?;
if !info.available_stages.contains(stage_bit) {
return Err(EntryPointError::ForbiddenStageOperations);
#[cfg(feature = "validate")]
{
use super::ShaderStages;
let stage_bit = match ep.stage {
crate::ShaderStage::Vertex => ShaderStages::VERTEX,
crate::ShaderStage::Fragment => ShaderStages::FRAGMENT,
crate::ShaderStage::Compute => ShaderStages::COMPUTE,
};
if !info.available_stages.contains(stage_bit) {
return Err(EntryPointError::ForbiddenStageOperations);
}
}
self.location_mask.clear();
@@ -458,6 +470,8 @@ impl super::Validator {
for bg in self.bind_group_masks.iter_mut() {
bg.clear();
}
#[cfg(feature = "validate")]
for (var_handle, var) in module.global_variables.iter() {
let usage = info[var_handle];
if usage.is_empty() {

View File

@@ -5,8 +5,11 @@ mod function;
mod interface;
mod r#type;
#[cfg(feature = "validate")]
use crate::arena::{Arena, UniqueArena};
use crate::{
arena::{Arena, Handle, UniqueArena},
arena::Handle,
proc::{InvalidBaseType, Layouter},
FastHashSet,
};
@@ -29,14 +32,19 @@ bitflags::bitflags! {
#[cfg_attr(feature = "deserialize", derive(serde::Deserialize))]
pub struct ValidationFlags: u8 {
/// Expressions.
#[cfg(feature = "validate")]
const EXPRESSIONS = 0x1;
/// Statements and blocks of them.
#[cfg(feature = "validate")]
const BLOCKS = 0x2;
/// Uniformity of control flow for operations that require it.
#[cfg(feature = "validate")]
const CONTROL_FLOW_UNIFORMITY = 0x4;
/// Host-shareable structure layouts.
#[cfg(feature = "validate")]
const STRUCT_LAYOUTS = 0x8;
/// Constants.
#[cfg(feature = "validate")]
const CONSTANTS = 0x10;
}
}
@@ -97,6 +105,7 @@ pub struct Validator {
layouter: Layouter,
location_mask: BitSet,
bind_group_masks: Vec<BitSet>,
#[allow(dead_code)]
select_cases: FastHashSet<i32>,
valid_expression_list: Vec<Handle<crate::Expression>>,
valid_expression_set: BitSet,
@@ -158,6 +167,7 @@ pub enum ValidationError {
}
impl crate::TypeInner {
#[cfg(feature = "validate")]
fn is_sized(&self) -> bool {
match *self {
Self::Scalar { .. }
@@ -176,6 +186,7 @@ impl crate::TypeInner {
}
/// Return the `ImageDimension` for which `self` is an appropriate coordinate.
#[cfg(feature = "validate")]
fn image_storage_coordinates(&self) -> Option<crate::ImageDimension> {
match *self {
Self::Scalar {
@@ -213,6 +224,7 @@ impl Validator {
}
}
#[cfg(feature = "validate")]
fn validate_constant(
&self,
handle: Handle<crate::Constant>,
@@ -257,6 +269,7 @@ impl Validator {
self.reset_types(module.types.len());
self.layouter.update(&module.types, &module.constants)?;
#[cfg(feature = "validate")]
if self.flags.contains(ValidationFlags::CONSTANTS) {
for (handle, constant) in module.constants.iter() {
self.validate_constant(handle, &module.constants, &module.types)
@@ -279,6 +292,7 @@ impl Validator {
self.types[handle.index()] = ty_info;
}
#[cfg(feature = "validate")]
for (var_handle, var) in module.global_variables.iter() {
self.validate_global_var(var, &module.types)
.map_err(|error| ValidationError::GlobalVariable {