From ae97a3f3164e832039f72500469fd20b3ab84b6e Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Thu, 16 Sep 2021 16:03:20 -0700 Subject: [PATCH] [wgsl-out] Correct handling of named pointer expressions. Treat expressions in `Function::named_expressions` like WGSL `let` declarations, assuming that the Load Rule was applied to the rhs of the declaration, meaning that their values are always `Indirection::Ordinary`. Split `write_expr_plain_form` out from `write_expr_with_indirection`, to clean up the parenthesis generation: no more `opened_paren` variable, just function calls. This makes the early return for named expressions neater. Fixes #1382. --- src/back/hlsl/writer.rs | 2 +- src/back/wgsl/writer.rs | 64 ++++++++++++++++++++++++----------- tests/in/pointers.param.ron | 7 ++++ tests/in/pointers.wgsl | 5 +++ tests/out/spv/pointers.spvasm | 29 ++++++++++++++++ tests/out/wgsl/pointers.wgsl | 8 +++++ tests/snapshots.rs | 1 + 7 files changed, 95 insertions(+), 21 deletions(-) create mode 100644 tests/in/pointers.param.ron create mode 100644 tests/in/pointers.wgsl create mode 100644 tests/out/spv/pointers.spvasm create mode 100644 tests/out/wgsl/pointers.wgsl diff --git a/src/back/hlsl/writer.rs b/src/back/hlsl/writer.rs index b0b0d1a98d..3888f608df 100644 --- a/src/back/hlsl/writer.rs +++ b/src/back/hlsl/writer.rs @@ -874,7 +874,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> { } _ => { return Err(Error::Unimplemented(format!( - "write_value_type {:?}", + "back::hlsl::Writer::write_value_type {:?}", inner ))) } diff --git a/src/back/wgsl/writer.rs b/src/back/wgsl/writer.rs index 41205e6c75..f6378215f5 100644 --- a/src/back/wgsl/writer.rs +++ b/src/back/wgsl/writer.rs @@ -642,7 +642,7 @@ impl Writer { } _ => { return Err(Error::Unimplemented(format!( - "write_value_type {:?}", + "back::wgsl::Writer::write_value_type {:?}", inner ))); } @@ -991,11 +991,20 @@ impl Writer { /// argument's value directly, so any pointer it produces is merely the value /// passed by the caller. fn plain_form_indirection( + &self, expr: Handle, module: &Module, func_ctx: &back::FunctionCtx<'_>, ) -> Indirection { use crate::Expression as Ex; + + // Named expressions are `let` expressions, which apply the Load Rule, + // so if their type is a Naga pointer, then that must be a WGSL pointer + // as well. + if self.named_expressions.contains_key(&expr) { + return Indirection::Ordinary; + } + match func_ctx.expressions[expr] { Ex::LocalVariable(_) => Indirection::Reference, Ex::GlobalVariable(handle) => { @@ -1073,27 +1082,46 @@ impl Writer { func_ctx: &back::FunctionCtx<'_>, requested: Indirection, ) -> BackendResult { - use crate::Expression; - - if let Some(name) = self.named_expressions.get(&expr) { - write!(self.out, "{}", name)?; - return Ok(()); - } - // If the plain form of the expression is not what we need, emit the // operator necessary to correct that. - let plain = Self::plain_form_indirection(expr, module, func_ctx); - let opened_paren = match (requested, plain) { + let plain = self.plain_form_indirection(expr, module, func_ctx); + match (requested, plain) { (Indirection::Ordinary, Indirection::Reference) => { write!(self.out, "(&")?; - true + self.write_expr_plain_form(module, expr, func_ctx, plain)?; + write!(self.out, ")")?; } (Indirection::Reference, Indirection::Ordinary) => { write!(self.out, "(*")?; - true + self.write_expr_plain_form(module, expr, func_ctx, plain)?; + write!(self.out, ")")?; } - (_, _) => false, - }; + (_, _) => self.write_expr_plain_form(module, expr, func_ctx, plain)?, + } + + Ok(()) + } + + /// Write the 'plain form' of `expr`. + /// + /// An expression's 'plain form' is the most general rendition of that + /// expression into WGSL, lacking `&` or `*` operators. The plain forms of + /// `LocalVariable(x)` and `GlobalVariable(g)` are simply `x` and `g`. Such + /// Naga expressions represent both WGSL pointers and references; it's the + /// caller's responsibility to distinguish those cases appropriately. + fn write_expr_plain_form( + &mut self, + module: &Module, + expr: Handle, + func_ctx: &back::FunctionCtx<'_>, + indirection: Indirection, + ) -> BackendResult { + use crate::Expression; + + if let Some(name) = self.named_expressions.get(&expr) { + write!(self.out, "{}", name)?; + return Ok(()); + } let expression = &func_ctx.expressions[expr]; @@ -1166,7 +1194,7 @@ impl Writer { } // TODO: copy-paste from glsl-out Expression::Access { base, index } => { - self.write_expr_with_indirection(module, base, func_ctx, plain)?; + self.write_expr_with_indirection(module, base, func_ctx, indirection)?; write!(self.out, "[")?; self.write_expr(module, index, func_ctx)?; write!(self.out, "]")? @@ -1176,7 +1204,7 @@ impl Writer { let base_ty_res = &func_ctx.info[base].ty; let mut resolved = base_ty_res.inner_with(&module.types); - self.write_expr_with_indirection(module, base, func_ctx, plain)?; + self.write_expr_with_indirection(module, base, func_ctx, indirection)?; let base_ty_handle = match *resolved { TypeInner::Pointer { base, class: _ } => { @@ -1598,10 +1626,6 @@ impl Writer { Expression::CallResult(_) | Expression::AtomicResult { .. } => {} } - if opened_paren { - write!(self.out, ")")? - }; - Ok(()) } diff --git a/tests/in/pointers.param.ron b/tests/in/pointers.param.ron new file mode 100644 index 0000000000..de304854bb --- /dev/null +++ b/tests/in/pointers.param.ron @@ -0,0 +1,7 @@ +( + spv: ( + version: (1, 2), + debug: true, + adjust_coordinate_space: false, + ), +) diff --git a/tests/in/pointers.wgsl b/tests/in/pointers.wgsl new file mode 100644 index 0000000000..8dd12442e5 --- /dev/null +++ b/tests/in/pointers.wgsl @@ -0,0 +1,5 @@ +fn f() { + var v: vec2; + let px = &v.x; + *px = 10; +} diff --git a/tests/out/spv/pointers.spvasm b/tests/out/spv/pointers.spvasm new file mode 100644 index 0000000000..b019f8b5c6 --- /dev/null +++ b/tests/out/spv/pointers.spvasm @@ -0,0 +1,29 @@ +; SPIR-V +; Version: 1.2 +; Generator: rspirv +; Bound: 16 +OpCapability Shader +OpCapability Linkage +%1 = OpExtInstImport "GLSL.std.450" +OpMemoryModel Logical GLSL450 +OpSource GLSL 450 +OpName %6 "v" +OpName %9 "f" +%2 = OpTypeVoid +%4 = OpTypeInt 32 1 +%3 = OpConstant %4 10 +%5 = OpTypeVector %4 2 +%7 = OpTypePointer Function %5 +%10 = OpTypeFunction %2 +%12 = OpTypePointer Function %4 +%14 = OpTypeInt 32 0 +%13 = OpConstant %14 0 +%9 = OpFunction %2 None %10 +%8 = OpLabel +%6 = OpVariable %7 Function +OpBranch %11 +%11 = OpLabel +%15 = OpAccessChain %12 %6 %13 +OpStore %15 %3 +OpReturn +OpFunctionEnd \ No newline at end of file diff --git a/tests/out/wgsl/pointers.wgsl b/tests/out/wgsl/pointers.wgsl new file mode 100644 index 0000000000..15a5157ae5 --- /dev/null +++ b/tests/out/wgsl/pointers.wgsl @@ -0,0 +1,8 @@ +fn f() { + var v: vec2; + + let px: ptr = (&v.x); + (*px) = 10; + return; +} + diff --git a/tests/snapshots.rs b/tests/snapshots.rs index 7a8873f376..181bd29794 100644 --- a/tests/snapshots.rs +++ b/tests/snapshots.rs @@ -480,6 +480,7 @@ fn convert_wgsl() { "access", Targets::SPIRV | Targets::METAL | Targets::GLSL | Targets::HLSL | Targets::WGSL, ), + ("pointers", Targets::SPIRV | Targets::WGSL), ( "control-flow", Targets::SPIRV | Targets::METAL | Targets::GLSL | Targets::HLSL | Targets::WGSL,