Fix SPIR-V validation of image ops with offsets (#2575)

* Fix SPIR-V validation of image op functions with offsets

Shader instructions like `textureLodOffset` would always fail validation.

* Rename signed `get_constant_*` functions
This commit is contained in:
Lachlan Deakin 2024-10-18 17:16:05 +11:00 committed by GitHub
parent 8c08a85a18
commit 123eda1a3b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 65 additions and 29 deletions

View File

@ -6,8 +6,9 @@ use crate::{
}, },
shader::{ shader::{
reflect::{ reflect::{
get_constant, get_constant_composite, get_constant_composite_composite, get_constant, get_constant_composite, get_constant_float_composite,
get_constant_float_composite, get_constant_maybe_composite, size_of_type, get_constant_signed_composite_composite, get_constant_signed_maybe_composite,
size_of_type,
}, },
spirv::{ spirv::{
Capability, Decoration, Dim, ExecutionMode, ExecutionModel, FunctionInfo, Id, Capability, Decoration, Dim, ExecutionMode, ExecutionModel, FunctionInfo, Id,
@ -2468,10 +2469,10 @@ impl<'a> RuntimeValidator<'a> {
if let Some(components) = image_operands if let Some(components) = image_operands
.const_offset .const_offset
.or(image_operands.offset) .or(image_operands.offset)
.and_then(|offset| get_constant_maybe_composite(self.spirv, offset)) .and_then(|offset| get_constant_signed_maybe_composite(self.spirv, offset))
{ {
for offset in components { for offset in components {
if offset < properties.min_texel_gather_offset as u64 { if offset < properties.min_texel_gather_offset as i64 {
return Err(Box::new(ValidationError { return Err(Box::new(ValidationError {
problem: "an `OpImage*Gather` instruction is performed, but \ problem: "an `OpImage*Gather` instruction is performed, but \
its `Offset`, `ConstOffset` or `ConstOffsets` \ its `Offset`, `ConstOffset` or `ConstOffsets` \
@ -2483,7 +2484,7 @@ impl<'a> RuntimeValidator<'a> {
})); }));
} }
if offset > properties.max_texel_gather_offset as u64 { if offset > properties.max_texel_gather_offset as i64 {
return Err(Box::new(ValidationError { return Err(Box::new(ValidationError {
problem: "an `OpImage*Gather` instruction is performed, but \ problem: "an `OpImage*Gather` instruction is performed, but \
its `Offset`, `ConstOffset` or `ConstOffsets` \ its `Offset`, `ConstOffset` or `ConstOffsets` \
@ -2497,11 +2498,11 @@ impl<'a> RuntimeValidator<'a> {
} }
} else if let Some(elements) = image_operands } else if let Some(elements) = image_operands
.const_offsets .const_offsets
.and_then(|id| get_constant_composite_composite(self.spirv, id)) .and_then(|id| get_constant_signed_composite_composite(self.spirv, id))
{ {
for components in elements { for components in elements {
for offset in components { for offset in components {
if offset < properties.min_texel_gather_offset as u64 { if offset < properties.min_texel_gather_offset as i64 {
return Err(Box::new(ValidationError { return Err(Box::new(ValidationError {
problem: "an `OpImage*Gather` instruction is performed, \ problem: "an `OpImage*Gather` instruction is performed, \
but its `Offset`, `ConstOffset` or `ConstOffsets` \ but its `Offset`, `ConstOffset` or `ConstOffsets` \
@ -2513,7 +2514,7 @@ impl<'a> RuntimeValidator<'a> {
})); }));
} }
if offset > properties.max_texel_gather_offset as u64 { if offset > properties.max_texel_gather_offset as i64 {
return Err(Box::new(ValidationError { return Err(Box::new(ValidationError {
problem: "an `OpImage*Gather` instruction is performed, \ problem: "an `OpImage*Gather` instruction is performed, \
but its `Offset`, `ConstOffset` or `ConstOffsets` \ but its `Offset`, `ConstOffset` or `ConstOffsets` \
@ -2534,10 +2535,10 @@ impl<'a> RuntimeValidator<'a> {
if let Some(image_operands) = instruction.image_operands() { if let Some(image_operands) = instruction.image_operands() {
if let Some(components) = image_operands if let Some(components) = image_operands
.const_offset .const_offset
.and_then(|offset| get_constant_maybe_composite(self.spirv, offset)) .and_then(|offset| get_constant_signed_maybe_composite(self.spirv, offset))
{ {
for offset in components { for offset in components {
if offset < properties.min_texel_offset as u64 { if offset < properties.min_texel_offset as i64 {
return Err(Box::new(ValidationError { return Err(Box::new(ValidationError {
problem: "an `OpImageSample*` or `OpImageFetch*` instruction \ problem: "an `OpImageSample*` or `OpImageFetch*` instruction \
is performed, but its `ConstOffset` image operand \ is performed, but its `ConstOffset` image operand \
@ -2549,7 +2550,7 @@ impl<'a> RuntimeValidator<'a> {
})); }));
} }
if offset > properties.max_texel_offset as u64 { if offset > properties.max_texel_offset as i64 {
return Err(Box::new(ValidationError { return Err(Box::new(ValidationError {
problem: "an `OpImageSample*` or `OpImageFetch*` instruction \ problem: "an `OpImageSample*` or `OpImageFetch*` instruction \
is performed, but its `ConstOffset` image operand \ is performed, but its `ConstOffset` image operand \

View File

@ -1214,22 +1214,57 @@ pub(crate) fn get_constant_float_composite(spirv: &Spirv, id: Id) -> Option<Smal
} }
} }
pub(crate) fn get_constant_maybe_composite(spirv: &Spirv, id: Id) -> Option<SmallVec<[u64; 4]>> { fn integer_constant_to_i64(spirv: &Spirv, value: &[u32], result_type_id: Id) -> i64 {
let type_id_instruction = spirv.id(result_type_id).instruction();
match type_id_instruction {
Instruction::TypeInt {
width, signedness, ..
} => {
if *width == 64 {
assert_eq!(value.len(), 2);
} else {
assert_eq!(value.len(), 1);
}
match (signedness, width) {
(0, 8) => value[0] as u8 as i64,
(0, 16) => value[0] as u16 as i64,
(0, 32) => value[0] as i64,
(0, 64) => i64::try_from((value[0] as u64) | ((value[1] as u64) << 32)).unwrap(),
(1, 8) => value[0] as i8 as i64,
(1, 16) => value[0] as i16 as i64,
(1, 32) => value[0] as i32 as i64,
(1, 64) => (value[0] as i64) | ((value[1] as i64) << 32),
_ => unimplemented!(),
}
}
_ => unreachable!(),
}
}
pub(crate) fn get_constant_signed_maybe_composite(
spirv: &Spirv,
id: Id,
) -> Option<SmallVec<[i64; 4]>> {
match spirv.id(id).instruction() { match spirv.id(id).instruction() {
Instruction::Constant { value, .. } => match value.len() { Instruction::Constant {
1 => Some(smallvec![value[0] as u64]), value,
2 => Some(smallvec![value[0] as u64 | (value[1] as u64) << 32]), result_type_id,
_ => panic!("constant {} is larger than 64 bits", id), ..
}, } => Some(smallvec![integer_constant_to_i64(
spirv,
value,
*result_type_id
)]),
Instruction::ConstantComposite { constituents, .. } => Some( Instruction::ConstantComposite { constituents, .. } => Some(
constituents constituents
.iter() .iter()
.map(|&id| match spirv.id(id).instruction() { .map(|&id| match spirv.id(id).instruction() {
Instruction::Constant { value, .. } => match value.len() { Instruction::Constant {
1 => value[0] as u64, value,
2 => value[0] as u64 | (value[1] as u64) << 32, result_type_id,
_ => panic!("constant {} is larger than 64 bits", id), ..
}, } => integer_constant_to_i64(spirv, value, *result_type_id),
_ => unreachable!(), _ => unreachable!(),
}) })
.collect(), .collect(),
@ -1238,10 +1273,10 @@ pub(crate) fn get_constant_maybe_composite(spirv: &Spirv, id: Id) -> Option<Smal
} }
} }
pub(crate) fn get_constant_composite_composite( pub(crate) fn get_constant_signed_composite_composite(
spirv: &Spirv, spirv: &Spirv,
id: Id, id: Id,
) -> Option<SmallVec<[SmallVec<[u64; 4]>; 4]>> { ) -> Option<SmallVec<[SmallVec<[i64; 4]>; 4]>> {
match spirv.id(id).instruction() { match spirv.id(id).instruction() {
Instruction::ConstantComposite { constituents, .. } => Some( Instruction::ConstantComposite { constituents, .. } => Some(
constituents constituents
@ -1250,11 +1285,11 @@ pub(crate) fn get_constant_composite_composite(
Instruction::ConstantComposite { constituents, .. } => constituents Instruction::ConstantComposite { constituents, .. } => constituents
.iter() .iter()
.map(|&id| match spirv.id(id).instruction() { .map(|&id| match spirv.id(id).instruction() {
Instruction::Constant { value, .. } => match value.len() { Instruction::Constant {
1 => value[0] as u64, value,
2 => value[0] as u64 | (value[1] as u64) << 32, result_type_id,
_ => panic!("constant {} is larger than 64 bits", id), ..
}, } => integer_constant_to_i64(spirv, value, *result_type_id),
_ => unreachable!(), _ => unreachable!(),
}) })
.collect(), .collect(),