Rename BoundsCheckPolicy::UndefinedBehavior to Unchecked.

When we are leaning on robust buffer access to do the job for us, there's no
undefined behavior going on. So `UndefinedBehavior` suggests people are doing
something reckless even if they're not. The policy just says what Naga is doing,
and it shouldn't pretend to say what the rest of the system is doing.
This commit is contained in:
Jim Blandy
2021-08-24 17:44:16 -07:00
committed by Dzmitry Malyshau
parent 1e9f2b9287
commit bdf774aa8b
5 changed files with 27 additions and 15 deletions

View File

@@ -13,13 +13,13 @@ struct Args {
/// what policy to use for index bounds checking for arrays, vectors, and
/// matrices.
///
/// May be `Restrict`, `ReadZeroSkipWrite`, or `UndefinedBehavior`
/// May be `Restrict`, `ReadZeroSkipWrite`, or `Unchecked`
#[argh(option)]
index_bounds_check_policy: Option<BoundsCheckPolicyArg>,
/// what policy to use for texture bounds checking.
///
/// May be `Restrict`, `ReadZeroSkipWrite`, or `UndefinedBehavior`
/// May be `Restrict`, `ReadZeroSkipWrite`, or `Unchecked`
#[argh(option)]
image_bounds_check_policy: Option<BoundsCheckPolicyArg>,
@@ -62,7 +62,7 @@ impl FromStr for BoundsCheckPolicyArg {
Ok(Self(match s.to_lowercase().as_str() {
"restrict" => BoundsCheckPolicy::Restrict,
"readzeroskipwrite" => BoundsCheckPolicy::ReadZeroSkipWrite,
"undefinedbehavior" => BoundsCheckPolicy::UndefinedBehavior,
"unchecked" => BoundsCheckPolicy::Unchecked,
_ => {
return Err(format!(
"Invalid value for --index-bounds-check-policy: {}",

View File

@@ -105,23 +105,35 @@ impl<'a> FunctionCtx<'_> {
}
}
/// How should code generated by Naga do indexing bounds checks?
/// How should code generated by Naga do bounds checks?
///
/// When a vector, matrix, or array index is out of bounds—either negative, or
/// greater than or equal to the number of elements in the type—WGSL requires
/// that some other index of the implementation's choice that is in bounds is
/// used instead. (There are no types with zero elements.)
///
/// Similarly, when out-of-bounds coordinates, array indices, or sample indices
/// are presented to the WGSL `textureLoad` and `textureStore` operations, the
/// operation is redirected to do something safe.
///
/// Different users of Naga will prefer different defaults:
///
/// - When used as part of a WebGPU implementation, the WGSL specification
/// requires the `Restrict` behavior.
/// requires the `Restrict` behavior for array, vector, and matrix accesses,
/// and either the `Restrict` or `ReadZeroSkipWrite` behaviors for texture
/// accesses.
///
/// - When used by the `wgpu` crate for native development, `wgpu` selects
/// `ReadZeroSkipWrite` as its default.
///
/// - Naga's own default is `UndefinedBehavior`, so that shader translations
/// - Naga's own default is `Unchanged`, so that shader translations
/// are as faithful to the original as possible.
///
/// Sometimes the underlying hardware and drivers can perform bounds checks
/// themselves, in a way that performs better than the checks Naga would inject.
/// If you're using native checks like this, then having Naga inject its own
/// checks as well would be redundant, and the `Unchecked` policy is
/// appropriate.
#[derive(Clone, Copy, Debug)]
pub enum BoundsCheckPolicy {
/// Replace out-of-bounds indexes with some arbitrary in-bounds index.
@@ -135,16 +147,16 @@ pub enum BoundsCheckPolicy {
/// Out-of-bounds reads return zero, and writes have no effect.
ReadZeroSkipWrite,
/// Out-of-bounds indexes are undefined behavior. Generate the fastest code
/// Naga adds no checks to indexing operations. Generate the fastest code
/// possible. This is the default for Naga, as a translator, but consumers
/// should consider defaulting to a safer behavior.
UndefinedBehavior,
Unchecked,
}
/// The default `BoundsCheckPolicy` is `UndefinedBehavior`.
/// The default `BoundsCheckPolicy` is `Unchecked`.
impl Default for BoundsCheckPolicy {
fn default() -> Self {
BoundsCheckPolicy::UndefinedBehavior
BoundsCheckPolicy::Unchecked
}
}

View File

@@ -758,7 +758,7 @@ impl<'w> BlockContext<'w> {
block,
&access,
)?,
crate::back::BoundsCheckPolicy::UndefinedBehavior => access.generate(
crate::back::BoundsCheckPolicy::Unchecked => access.generate(
&mut self.writer.id_gen,
coordinates.value_id,
level_id,
@@ -1146,7 +1146,7 @@ impl<'w> BlockContext<'w> {
&write,
)?;
}
crate::back::BoundsCheckPolicy::UndefinedBehavior => {
crate::back::BoundsCheckPolicy::Unchecked => {
write.generate(
&mut self.writer.id_gen,
coordinates.value_id,

View File

@@ -347,7 +347,7 @@ impl<'w> BlockContext<'w> {
BoundsCheckPolicy::ReadZeroSkipWrite => {
self.write_index_comparison(base, index, block)?
}
BoundsCheckPolicy::UndefinedBehavior => BoundsCheckResult::Computed(self.cached[index]),
BoundsCheckPolicy::Unchecked => BoundsCheckResult::Computed(self.cached[index]),
})
}

View File

@@ -174,14 +174,14 @@ fn write_output_spv(
} else if params.bounds_check_read_zero_skip_write {
naga::back::BoundsCheckPolicy::ReadZeroSkipWrite
} else {
naga::back::BoundsCheckPolicy::UndefinedBehavior
naga::back::BoundsCheckPolicy::Unchecked
},
image_bounds_check_policy: if params.image_bounds_check_restrict {
naga::back::BoundsCheckPolicy::Restrict
} else if params.image_bounds_check_read_zero_skip_write {
naga::back::BoundsCheckPolicy::ReadZeroSkipWrite
} else {
naga::back::BoundsCheckPolicy::UndefinedBehavior
naga::back::BoundsCheckPolicy::Unchecked
},
..spv::Options::default()
};