github/wgpu
1
1
Fork 0
mirror of https://github.com/gfx-rs/wgpu.git synced 2026-04-22 03:02:01 -04:00
Code Issues Packages Projects Releases Wiki Activity

Update WGSL grammar for pointer access. (#1312)

Browse Source 
* Update WGSL grammar for pointer access.

Comes with a small test, which revealed a number of issues in the backends.

* Validate pointer arguments to functions to only have function/private/workgroup classes.

Comes with a small test. Also, "pointer-access.spv" test is temporarily disabled.
This commit is contained in:
Dzmitry Malyshau
2021-09-27 18:49:28 -04:00
committed by GitHub
parent 38d74a7f0a
commit 21324b8fea
19 changed files with 312 additions and 197 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/back/glsl/mod.rs
View File

@@ -715,7 +715,9 @@ impl<'a, W: Write> Writer<'a, W> {
TypeInner::Pointer { .. }
| TypeInner::Struct { .. }
| TypeInner::Image { .. }
| TypeInner::Sampler { .. } => unreachable!(),
| TypeInner::Sampler { .. } => {
return Err(Error::Custom(format!("Unable to write type {:?}", inner)))
}
}
Ok(())
@@ -1332,7 +1334,14 @@ impl<'a, W: Write> Writer<'a, W> {
// This is where we can generate intermediate constants for some expression types.
Statement::Emit(ref range) => {
for handle in range.clone() {
let expr_name = if let Some(name) = ctx.named_expressions.get(&handle) {
let info = &ctx.info[handle];
let ptr_class = info.ty.inner_with(&self.module.types).pointer_class();
let expr_name = if ptr_class.is_some() {
// GLSL can't save a pointer-valued expression in a variable,
// but we shouldn't ever need to: they should never be named expressions,
// and none of the expression types flagged by bake_ref_count can be pointer-valued.
None
} else if let Some(name) = ctx.named_expressions.get(&handle) {
// Front end provides names for all variables at the start of writing.
// But we write them to step by step. We need to recache them
// Otherwise, we could accidentally write variable name instead of full expression.
@@ -1340,7 +1349,7 @@ impl<'a, W: Write> Writer<'a, W> {
Some(self.namer.call_unique(name))
} else {
let min_ref_count = ctx.expressions[handle].bake_ref_count();
if min_ref_count <= ctx.info[handle].ref_count {
if min_ref_count <= info.ref_count {
Some(format!("{}{}", super::BAKE_PREFIX, handle.index()))
} else {
None

11
src/back/hlsl/writer.rs
View File

@@ -1057,7 +1057,14 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
match *stmt {
Statement::Emit(ref range) => {
for handle in range.clone() {
let expr_name = if let Some(name) = func_ctx.named_expressions.get(&handle) {
let info = &func_ctx.info[handle];
let ptr_class = info.ty.inner_with(&module.types).pointer_class();
let expr_name = if ptr_class.is_some() {
// HLSL can't save a pointer-valued expression in a variable,
// but we shouldn't ever need to: they should never be named expressions,
// and none of the expression types flagged by bake_ref_count can be pointer-valued.
None
} else if let Some(name) = func_ctx.named_expressions.get(&handle) {
// Front end provides names for all variables at the start of writing.
// But we write them to step by step. We need to recache them
// Otherwise, we could accidentally write variable name instead of full expression.
@@ -1065,7 +1072,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
Some(self.namer.call_unique(name))
} else {
let min_ref_count = func_ctx.expressions[handle].bake_ref_count();
if min_ref_count <= func_ctx.info[handle].ref_count {
if min_ref_count <= info.ref_count {
Some(format!("_expr{}", handle.index()))
} else {
None

13
src/back/msl/writer.rs
View File

@@ -1355,7 +1355,7 @@ impl<W: Write> Writer<W> {
)?;
}
TypeResolution::Value(ref other) => {
log::error!("Type {:?} isn't a known local", other);
log::warn!("Type {:?} isn't a known local", other); //TEMP!
return Err(Error::FeatureNotImplemented("weird local type".to_string()));
}
}
@@ -1383,7 +1383,14 @@ impl<W: Write> Writer<W> {
match *statement {
crate::Statement::Emit(ref range) => {
for handle in range.clone() {
let expr_name = if let Some(name) =
let info = &context.expression.info[handle];
let ptr_class = info
.ty
.inner_with(&context.expression.module.types)
.pointer_class();
let expr_name = if ptr_class.is_some() {
None // don't bake pointer expressions (just yet)
} else if let Some(name) =
context.expression.function.named_expressions.get(&handle)
{
// Front end provides names for all variables at the start of writing.
@@ -1394,7 +1401,7 @@ impl<W: Write> Writer<W> {
} else {
let min_ref_count =
context.expression.function.expressions[handle].bake_ref_count();
if min_ref_count <= context.expression.info[handle].ref_count {
if min_ref_count <= info.ref_count {
Some(format!("{}{}", back::BAKE_PREFIX, handle.index()))
} else {
None

20
src/back/wgsl/writer.rs
View File

@@ -525,10 +525,13 @@ impl<W: Write> Writer<W> {
"storage_",
"",
storage_format_str(format),
if access.contains(crate::StorageAccess::STORE) {
",write"
if access.contains(crate::StorageAccess::LOAD | crate::StorageAccess::STORE)
{
",read_write"
} else if access.contains(crate::StorageAccess::LOAD) {
",read"
} else {
""
",write"
},
),
};
@@ -639,6 +642,7 @@ impl<W: Write> Writer<W> {
inner
)));
}
write!(self.out, ">")?;
}
_ => {
return Err(Error::Unimplemented(format!(
@@ -666,6 +670,7 @@ impl<W: Write> Writer<W> {
match *stmt {
Statement::Emit(ref range) => {
for handle in range.clone() {
let info = &func_ctx.info[handle];
let expr_name = if let Some(name) = func_ctx.named_expressions.get(&handle) {
// Front end provides names for all variables at the start of writing.
// But we write them to step by step. We need to recache them
@@ -682,8 +687,7 @@ impl<W: Write> Writer<W> {
| Expression::ImageSample { .. } => true,
_ => false,
};
if min_ref_count <= func_ctx.info[handle].ref_count || required_baking_expr
{
if min_ref_count <= info.ref_count || required_baking_expr {
// If expression contains unsupported builtin we should skip it
if let Expression::Load { pointer } = func_ctx.expressions[handle] {
if let Expression::AccessIndex { base, index } =
@@ -809,8 +813,8 @@ impl<W: Write> Writer<W> {
}
let func_name = &self.names[&NameKey::Function(function)];
write!(self.out, "{}(", func_name)?;
for (index, argument) in arguments.iter().enumerate() {
self.write_expr(module, *argument, func_ctx)?;
for (index, &argument) in arguments.iter().enumerate() {
self.write_expr(module, argument, func_ctx)?;
// Only write a comma if isn't the last element
if index != arguments.len().saturating_sub(1) {
// The leading space is for readability only
@@ -1199,14 +1203,12 @@ impl<W: Write> Writer<W> {
self.write_expr(module, right, func_ctx)?;
write!(self.out, ")")?;
}
// TODO: copy-paste from glsl-out
Expression::Access { base, index } => {
self.write_expr_with_indirection(module, base, func_ctx, indirection)?;
write!(self.out, "[")?;
self.write_expr(module, index, func_ctx)?;
write!(self.out, "]")?
}
// TODO: copy-paste from glsl-out
Expression::AccessIndex { base, index } => {
let base_ty_res = &func_ctx.info[base].ty;
let mut resolved = base_ty_res.inner_with(&module.types);

24
src/front/wgsl/lexer.rs
View File

@@ -558,24 +558,24 @@ impl<'a> Lexer<'a> {
Ok(pair)
}
// TODO relocate storage texture specifics
pub(super) fn next_storage_access(&mut self) -> Result<crate::StorageAccess, Error<'a>> {
let (ident, span) = self.next_ident_with_span()?;
match ident {
"read" => Ok(crate::StorageAccess::LOAD),
"write" => Ok(crate::StorageAccess::STORE),
"read_write" => Ok(crate::StorageAccess::LOAD | crate::StorageAccess::STORE),
_ => Err(Error::UnknownAccess(span)),
}
}
pub(super) fn next_format_generic(
&mut self,
) -> Result<(crate::StorageFormat, crate::StorageAccess), Error<'a>> {
self.expect(Token::Paren('<'))?;
let (ident, ident_span) = self.next_ident_with_span()?;
let format = conv::map_storage_format(ident, ident_span)?;
let access = if self.skip(Token::Separator(',')) {
let (raw, span) = self.next_ident_with_span()?;
match raw {
"read" => crate::StorageAccess::LOAD,
"write" => crate::StorageAccess::STORE,
"read_write" => crate::StorageAccess::all(),
_ => return Err(Error::UnknownAccess(span)),
}
} else {
crate::StorageAccess::LOAD
};
self.expect(Token::Separator(','))?;
let access = self.next_storage_access()?;
self.expect(Token::Paren('>'))?;
Ok((format, access))
}

17
src/front/wgsl/mod.rs
View File

@@ -2587,13 +2587,7 @@ impl Parser {
class = Some(match class_str {
"storage" => {
let access = if lexer.skip(Token::Separator(',')) {
let (ident, span) = lexer.next_ident_with_span()?;
match ident {
"read" => crate::StorageAccess::LOAD,
"write" => crate::StorageAccess::STORE,
"read_write" => crate::StorageAccess::all(),
_ => return Err(Error::UnknownAccess(span)),
}
lexer.next_storage_access()?
} else {
// defaulting to `read`
crate::StorageAccess::LOAD
@@ -2836,9 +2830,16 @@ impl Parser {
"ptr" => {
lexer.expect_generic_paren('<')?;
let (ident, span) = lexer.next_ident_with_span()?;
let class = conv::map_storage_class(ident, span)?;
let mut class = conv::map_storage_class(ident, span)?;
lexer.expect(Token::Separator(','))?;
let (base, _access) = self.parse_type_decl(lexer, None, type_arena, const_arena)?;
if let crate::StorageClass::Storage { ref mut access } = class {
*access = if lexer.skip(Token::Separator(',')) {
lexer.next_storage_access()?
} else {
crate::StorageAccess::LOAD
};
}
lexer.expect_generic_paren('>')?;
crate::TypeInner::Pointer { base, class }
}

4
src/front/wgsl/tests.rs
View File

@@ -92,7 +92,7 @@ fn parse_types() {
parse_str("var t: texture_cube_array<i32>;").unwrap();
parse_str("var t: texture_multisampled_2d<u32>;").unwrap();
parse_str("var t: texture_storage_1d<rgba8uint,write>;").unwrap();
parse_str("var t: texture_storage_3d<r32float>;").unwrap();
parse_str("var t: texture_storage_3d<r32float,read>;").unwrap();
}
#[test]
@@ -305,7 +305,7 @@ fn parse_texture_load() {
.unwrap();
parse_str(
"
var t: texture_storage_1d_array<r32float>;
var t: texture_storage_1d_array<r32float,read>;
fn foo() {
let r: vec4<f32> = textureLoad(t, 10, 2);
}

19
src/valid/function.rs
View File

@@ -76,6 +76,12 @@ pub enum FunctionError {
},
#[error("Argument '{name}' at index {index} has a type that can't be passed into functions.")]
InvalidArgumentType { index: usize, name: String },
#[error("Argument '{name}' at index {index} is a pointer of class {class:?}, which can't be passed into functions.")]
InvalidArgumentPointerClass {
index: usize,
name: String,
class: crate::StorageClass,
},
#[error("There are instructions after `return`/`break`/`continue`")]
InstructionsAfterReturn,
#[error("The `break` is used outside of a `loop` or `switch` context")]
@@ -696,6 +702,19 @@ impl super::Validator {
name: argument.name.clone().unwrap_or_default(),
});
}
match module.types[argument.ty].inner.pointer_class() {
Some(crate::StorageClass::Private)
| Some(crate::StorageClass::Function)
| Some(crate::StorageClass::WorkGroup)
| None => {}
Some(other) => {
return Err(FunctionError::InvalidArgumentPointerClass {
index,
name: argument.name.clone().unwrap_or_default(),
class: other,
})
}
}
}
self.valid_expression_set.clear();