Skip to content

Commit

Permalink
[naga valid]: Clean up validation of Statement::ImageStore.
Browse files Browse the repository at this point in the history
Ensure that the type we obtain for the `image` operand is correct:
"see through" a binding array to its element type only when `image` is
actually an `Access` or `AccessIndex` expression. (This changes the
set of programs the validator will pass, but it turns out not to
affect the set of WGSL programs that Naga will accept, since the WGSL
front end is already checking the types of the `texture` arguments to
`textureStore` function calls, in order to decide which overload
applies.)

Rename the variables to better reflect their values.
  • Loading branch information
jimblandy committed Dec 18, 2024
1 parent 87d9ffe commit a0344cc
Showing 1 changed file with 20 additions and 12 deletions.
32 changes: 20 additions & 12 deletions naga/src/valid/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1012,9 +1012,12 @@ impl super::Validator {
} => {
//Note: this code uses a lot of `FunctionError::InvalidImageStore`,
// and could probably be refactored.
let var = match *context.get_expression(image) {
let global_var;
let image_ty;
match *context.get_expression(image) {
crate::Expression::GlobalVariable(var_handle) => {
&context.global_vars[var_handle]
global_var = &context.global_vars[var_handle];
image_ty = global_var.ty;
}
// The `image` operand is indexing into a binding array,
// so punch through the `Access`* expression and look at
Expand All @@ -1029,7 +1032,18 @@ impl super::Validator {
)
.with_span_handle(image, context.expressions));
};
&context.global_vars[var_handle]
global_var = &context.global_vars[var_handle];

// The global variable must be a binding array.
let Ti::BindingArray { base, .. } = context.types[global_var.ty].inner
else {
return Err(FunctionError::InvalidImageStore(
ExpressionError::ExpectedBindingArrayType(global_var.ty),
)
.with_span_handle(global_var.ty, context.types));
};

image_ty = base;
}
_ => {
return Err(FunctionError::InvalidImageStore(
Expand All @@ -1039,24 +1053,18 @@ impl super::Validator {
}
};

// Punch through a binding array to get the underlying type.
let global_ty = match context.types[var.ty].inner {
Ti::BindingArray { base, .. } => &context.types[base].inner,
ref inner => inner,
};

// The `image` operand must be an `Image`.
let Ti::Image {
class,
arrayed,
dim,
} = *global_ty
} = context.types[image_ty].inner
else {
return Err(FunctionError::InvalidImageStore(
ExpressionError::ExpectedImageType(var.ty),
ExpressionError::ExpectedImageType(global_var.ty),
)
.with_span()
.with_handle(var.ty, context.types)
.with_handle(global_var.ty, context.types)
.with_handle(image, context.expressions));
};

Expand Down

0 comments on commit a0344cc

Please sign in to comment.