[spv-out] Support a separate bounds check policy for buffers.

This commit is contained in:
Jim Blandy
2021-08-24 18:26:22 -07:00
committed by Dzmitry Malyshau
parent 140e3223e2
commit 1b95e023e7
9 changed files with 261 additions and 2 deletions

View File

@@ -17,6 +17,14 @@ struct Args {
#[argh(option)]
index_bounds_check_policy: Option<BoundsCheckPolicyArg>,
/// what policy to use for index bounds checking for arrays, vectors, and
/// matrices, when they are stored in globals in the `storage` or `uniform`
/// storage classes.
///
/// May be `Restrict`, `ReadZeroSkipWrite`, or `Unchecked`
#[argh(option)]
buffer_bounds_check_policy: Option<BoundsCheckPolicyArg>,
/// what policy to use for texture bounds checking.
///
/// May be `Restrict`, `ReadZeroSkipWrite`, or `Unchecked`
@@ -114,6 +122,7 @@ impl FromStr for GlslProfileArg {
struct Parameters {
validation_flags: naga::valid::ValidationFlags,
index_bounds_check_policy: naga::back::BoundsCheckPolicy,
buffer_bounds_check_policy: naga::back::BoundsCheckPolicy,
image_bounds_check_policy: naga::back::BoundsCheckPolicy,
entry_point: Option<String>,
spv_adjust_coordinate_space: bool,
@@ -194,6 +203,9 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
if let Some(policy) = args.index_bounds_check_policy {
params.index_bounds_check_policy = policy.0;
}
if let Some(policy) = args.buffer_bounds_check_policy {
params.buffer_bounds_check_policy = policy.0;
}
if let Some(policy) = args.image_bounds_check_policy {
params.image_bounds_check_policy = policy.0;
}
@@ -318,6 +330,7 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
use naga::back::spv;
params.spv.index_bounds_check_policy = params.index_bounds_check_policy;
params.spv.buffer_bounds_check_policy = params.buffer_bounds_check_policy;
params.spv.image_bounds_check_policy = params.image_bounds_check_policy;
let spv = spv::write_vec(

View File

@@ -332,7 +332,11 @@ impl<'w> BlockContext<'w> {
selection.finish(self, loaded_value)
}
/// Emit code for bounds checks, per self.index_bounds_check_policy.
/// Emit code for bounds checks for an array, vector, or matrix access.
///
/// This implements either `index_bounds_check_policy` or
/// `buffer_bounds_check_policy`, depending on the storage class of the
/// pointer being accessed.
///
/// Return a `BoundsCheckResult` indicating how the index should be
/// consumed. See that type's documentation for details.
@@ -342,7 +346,24 @@ impl<'w> BlockContext<'w> {
index: Handle<crate::Expression>,
block: &mut Block,
) -> Result<BoundsCheckResult, Error> {
Ok(match self.writer.index_bounds_check_policy {
// Should this access be covered by `index_bounds_check_policy` or
// `buffer_bounds_check_policy`?
let is_buffer = match *self.fun_info[base].ty.inner_with(&self.ir_module.types) {
crate::TypeInner::Pointer { class, .. }
| crate::TypeInner::ValuePointer { class, .. } => match class {
crate::StorageClass::Storage { access: _ } | crate::StorageClass::Uniform => true,
_ => false,
},
_ => false,
};
let policy = if is_buffer {
self.writer.buffer_bounds_check_policy
} else {
self.writer.index_bounds_check_policy
};
Ok(match policy {
BoundsCheckPolicy::Restrict => self.write_restricted_index(base, index, block)?,
BoundsCheckPolicy::ReadZeroSkipWrite => {
self.write_index_comparison(base, index, block)?

View File

@@ -419,6 +419,7 @@ pub struct Writer {
flags: WriterFlags,
index_bounds_check_policy: BoundsCheckPolicy,
image_bounds_check_policy: BoundsCheckPolicy,
buffer_bounds_check_policy: BoundsCheckPolicy,
void_type: Word,
//TODO: convert most of these into vectors, addressable by handle indices
lookup_type: crate::FastHashMap<LookupType, Word>,
@@ -463,6 +464,42 @@ pub struct Options {
/// How should the generated code handle array, vector, or matrix indices
/// that are out of range?
pub index_bounds_check_policy: BoundsCheckPolicy,
/// How should the generated code handle array, vector, or matrix indices
/// that are out of range, when those values live in a [`GlobalVariable`] in
/// the [`Storage`] or [`Uniform`] storage classes?
///
/// Some graphics hardware provides "robust buffer access", a feature that
/// ensures that using a pointer cannot access memory outside the 'buffer'
/// that it was derived from. In Naga terms, this means that the hardware
/// ensures that pointers computed by applying [`Access`] and
/// [`AccessIndex`] expressions to a [`GlobalVariable`] whose [`class`] is
/// [`Storage`] or [`Uniform`] will never read or write memory outside that
/// global variable.
///
/// When hardware offers such a feature, it is probably undesirable to have
/// Naga inject bounds checking code for such accesses, since the hardware
/// can probably provide the same protection more efficiently. However,
/// bounds checks are still needed on accesses to indexable values that do
/// not live in buffers, like local variables.
///
/// So, this option provides a separate policy that applies only to accesses
/// to storage and uniform globals. When depending on hardware bounds
/// checking, this policy can be `Unchecked` to avoid unnecessary overhead.
///
/// When special hardware support is not available, this should probably be
/// the same as `index_bounds_check_policy`.
///
/// [`GlobalVariable`]: crate::GlobalVariable
/// [`class`]: crate::GlobalVariable::class
/// [`Restrict`]: crate::back::BoundsCheckPolicy::Restrict
/// [`ReadZeroSkipWrite`]: crate::back::BoundsCheckPolicy::ReadZeroSkipWrite
/// [`Access`]: crate::Expression::Access
/// [`AccessIndex`]: crate::Expression::AccessIndex
/// [`Storage`]: crate::StorageClass::Storage
/// [`Uniform`]: crate::StorageClass::Uniform
pub buffer_bounds_check_policy: BoundsCheckPolicy,
/// How should the generated code handle image references that are out of
/// range?
pub image_bounds_check_policy: BoundsCheckPolicy,
@@ -480,6 +517,7 @@ impl Default for Options {
capabilities: None,
index_bounds_check_policy: super::BoundsCheckPolicy::default(),
image_bounds_check_policy: super::BoundsCheckPolicy::default(),
buffer_bounds_check_policy: super::BoundsCheckPolicy::default(),
}
}
}

View File

@@ -68,6 +68,7 @@ impl Writer {
flags: options.flags,
index_bounds_check_policy: options.index_bounds_check_policy,
image_bounds_check_policy: options.image_bounds_check_policy,
buffer_bounds_check_policy: options.buffer_bounds_check_policy,
void_type,
lookup_type: crate::FastHashMap::default(),
lookup_function: crate::FastHashMap::default(),
@@ -105,6 +106,7 @@ impl Writer {
flags: self.flags,
index_bounds_check_policy: self.index_bounds_check_policy,
image_bounds_check_policy: self.image_bounds_check_policy,
buffer_bounds_check_policy: self.buffer_bounds_check_policy,
capabilities_available: take(&mut self.capabilities_available),
// Initialized afresh:

View File

@@ -1,4 +1,5 @@
(
index_bounds_check_policy: ReadZeroSkipWrite,
buffer_bounds_check_policy: ReadZeroSkipWrite,
spv_version: (1, 1),
)

View File

@@ -0,0 +1,7 @@
(
index_bounds_check_policy: Restrict,
buffer_bounds_check_policy: Unchecked,
image_bounds_check_policy: ReadZeroSkipWrite,
spv_version: (1, 1),
spv_debug: true,
)

35
tests/in/policy-mix.wgsl Normal file
View File

@@ -0,0 +1,35 @@
// Tests that the index, buffer, and texture bounds checks policies are
// implemented separately.
// Storage and Uniform storage classes
[[block]]
struct InStorage {
a: array<vec4<f32>, 10>;
};
[[group(0), binding(0)]] var<storage> in_storage: InStorage;
[[block]]
struct InUniform {
a: array<vec4<f32>, 20>;
};
[[group(0), binding(1)]] var<uniform> in_uniform: InUniform;
// Textures automatically land in the `handle` storage class.
[[group(0), binding(2)]] var image_2d_array: texture_2d_array<f32>;
// None of the above.
var<workgroup> in_workgroup: array<f32, 30>;
var<private> in_private: array<f32, 40>;
fn mock_function(c: vec2<i32>, i: i32, l: i32) -> vec4<f32> {
var in_function: array<vec4<f32>, 2> =
array<vec4<f32>, 2>(vec4<f32>(0.707, 0.0, 0.0, 1.0),
vec4<f32>(0.0, 0.707, 0.0, 1.0));
return (in_storage.a[i] +
in_uniform.a[i] +
textureLoad(image_2d_array, c, i, l) +
in_workgroup[i] +
in_private[i] +
in_function[i]);
}

View File

@@ -0,0 +1,137 @@
; SPIR-V
; Version: 1.1
; Generator: rspirv
; Bound: 93
OpCapability Shader
OpCapability ImageQuery
OpCapability Linkage
OpExtension "SPV_KHR_storage_buffer_storage_class"
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpSource GLSL 450
OpName %15 "InStorage"
OpMemberName %15 0 "a"
OpName %17 "InUniform"
OpMemberName %17 0 "a"
OpName %23 "in_storage"
OpName %25 "in_uniform"
OpName %27 "image_2d_array"
OpName %29 "in_workgroup"
OpName %31 "in_private"
OpName %33 "in_function"
OpName %39 "mock_function"
OpDecorate %14 ArrayStride 16
OpDecorate %15 Block
OpMemberDecorate %15 0 Offset 0
OpDecorate %16 ArrayStride 16
OpDecorate %17 Block
OpMemberDecorate %17 0 Offset 0
OpDecorate %19 ArrayStride 4
OpDecorate %20 ArrayStride 4
OpDecorate %22 ArrayStride 16
OpDecorate %23 NonWritable
OpDecorate %23 DescriptorSet 0
OpDecorate %23 Binding 0
OpDecorate %25 DescriptorSet 0
OpDecorate %25 Binding 1
OpDecorate %27 DescriptorSet 0
OpDecorate %27 Binding 2
%2 = OpTypeVoid
%4 = OpTypeInt 32 1
%3 = OpConstant %4 10
%5 = OpConstant %4 20
%6 = OpConstant %4 30
%7 = OpConstant %4 40
%8 = OpConstant %4 2
%10 = OpTypeFloat 32
%9 = OpConstant %10 0.707
%11 = OpConstant %10 0.0
%12 = OpConstant %10 1.0
%13 = OpTypeVector %10 4
%14 = OpTypeArray %13 %3
%15 = OpTypeStruct %14
%16 = OpTypeArray %13 %5
%17 = OpTypeStruct %16
%18 = OpTypeImage %10 2D 0 1 0 1 Unknown
%19 = OpTypeArray %10 %6
%20 = OpTypeArray %10 %7
%21 = OpTypeVector %4 2
%22 = OpTypeArray %13 %8
%24 = OpTypePointer StorageBuffer %15
%23 = OpVariable %24 StorageBuffer
%26 = OpTypePointer Uniform %17
%25 = OpVariable %26 Uniform
%28 = OpTypePointer UniformConstant %18
%27 = OpVariable %28 UniformConstant
%30 = OpTypePointer Workgroup %19
%29 = OpVariable %30 Workgroup
%32 = OpTypePointer Private %20
%31 = OpVariable %32 Private
%34 = OpTypePointer Function %22
%40 = OpTypeFunction %13 %21 %4 %4
%46 = OpTypePointer StorageBuffer %14
%47 = OpTypePointer StorageBuffer %13
%49 = OpTypeInt 32 0
%48 = OpConstant %49 0
%52 = OpTypePointer Uniform %16
%53 = OpTypePointer Uniform %13
%57 = OpTypeVector %4 3
%59 = OpTypeBool
%60 = OpConstantNull %13
%66 = OpTypeVector %59 3
%73 = OpTypePointer Workgroup %10
%74 = OpConstant %49 29
%80 = OpTypePointer Private %10
%81 = OpConstant %49 39
%87 = OpTypePointer Function %13
%88 = OpConstant %49 1
%39 = OpFunction %13 None %40
%36 = OpFunctionParameter %21
%37 = OpFunctionParameter %4
%38 = OpFunctionParameter %4
%35 = OpLabel
%33 = OpVariable %34 Function
%41 = OpLoad %18 %27
OpBranch %42
%42 = OpLabel
%43 = OpCompositeConstruct %13 %9 %11 %11 %12
%44 = OpCompositeConstruct %13 %11 %9 %11 %12
%45 = OpCompositeConstruct %22 %43 %44
OpStore %33 %45
%50 = OpAccessChain %47 %23 %48 %37
%51 = OpLoad %13 %50
%54 = OpAccessChain %53 %25 %48 %37
%55 = OpLoad %13 %54
%56 = OpFAdd %13 %51 %55
%58 = OpCompositeConstruct %57 %36 %37
%61 = OpImageQueryLevels %4 %41
%62 = OpULessThan %59 %38 %61
OpSelectionMerge %63 None
OpBranchConditional %62 %64 %63
%64 = OpLabel
%65 = OpImageQuerySizeLod %57 %41 %38
%67 = OpULessThan %66 %58 %65
%68 = OpAll %59 %67
OpBranchConditional %68 %69 %63
%69 = OpLabel
%70 = OpImageFetch %13 %41 %58 Lod %38
OpBranch %63
%63 = OpLabel
%71 = OpPhi %13 %60 %42 %60 %64 %70 %69
%72 = OpFAdd %13 %56 %71
%75 = OpExtInst %49 %1 UMin %37 %74
%76 = OpAccessChain %73 %29 %75
%77 = OpLoad %10 %76
%78 = OpCompositeConstruct %13 %77 %77 %77 %77
%79 = OpFAdd %13 %72 %78
%82 = OpExtInst %49 %1 UMin %37 %81
%83 = OpAccessChain %80 %31 %82
%84 = OpLoad %10 %83
%85 = OpCompositeConstruct %13 %84 %84 %84 %84
%86 = OpFAdd %13 %79 %85
%89 = OpExtInst %49 %1 UMin %37 %88
%90 = OpAccessChain %87 %33 %89
%91 = OpLoad %13 %90
%92 = OpFAdd %13 %86 %91
OpReturnValue %92
OpFunctionEnd

View File

@@ -53,6 +53,9 @@ struct Parameters {
index_bounds_check_policy: BoundsCheckPolicyArg,
#[allow(dead_code)]
#[serde(default)]
buffer_bounds_check_policy: BoundsCheckPolicyArg,
#[allow(dead_code)]
#[serde(default)]
image_bounds_check_policy: BoundsCheckPolicyArg,
#[cfg_attr(not(feature = "spv-out"), allow(dead_code))]
@@ -187,6 +190,7 @@ fn write_output_spv(
},
index_bounds_check_policy: params.index_bounds_check_policy.into(),
buffer_bounds_check_policy: params.buffer_bounds_check_policy.into(),
image_bounds_check_policy: params.image_bounds_check_policy.into(),
..spv::Options::default()
@@ -462,6 +466,7 @@ fn convert_wgsl() {
("bounds-check-zero", Targets::SPIRV),
("bounds-check-image-restrict", Targets::SPIRV),
("bounds-check-image-rzsw", Targets::SPIRV),
("policy-mix", Targets::SPIRV),
(
"texture-arg",
Targets::SPIRV | Targets::METAL | Targets::GLSL | Targets::HLSL | Targets::WGSL,