From 38d74a7f0aa723355a35a3011d044f59d38e79a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Capucho?= Date: Thu, 23 Sep 2021 22:20:08 +0100 Subject: [PATCH] [glsl-in] Use intermediate local if storage class isn't function Automatically spills to a local variable function call arguments to parameters expecting a pointer where the argument storage class isn't function since the storage classes wouldn't match. --- src/front/glsl/context.rs | 7 +- src/front/glsl/functions.rs | 126 +++++++++++++++++---------- tests/in/glsl/expressions.frag | 14 +-- tests/out/wgsl/expressions-frag.wgsl | 24 +++-- 4 files changed, 114 insertions(+), 57 deletions(-) diff --git a/src/front/glsl/context.rs b/src/front/glsl/context.rs index dcbac22768..6729c6f796 100644 --- a/src/front/glsl/context.rs +++ b/src/front/glsl/context.rs @@ -433,7 +433,12 @@ impl Context { HirExprKind::Access { base, index } => { let (index, index_meta) = self.lower_expect_inner(stmt, parser, index, ExprPos::Rhs, body)?; - let maybe_constant_index = parser.solve_constant(self, index, index_meta).ok(); + let maybe_constant_index = match pos { + // Don't try to generate `AccessIndex` if in a LHS position, since it + // wouldn't produce a pointer. + ExprPos::Lhs => None, + _ => parser.solve_constant(self, index, index_meta).ok(), + }; let base = self .lower_expect_inner( diff --git a/src/front/glsl/functions.rs b/src/front/glsl/functions.rs index 1b84a10bb6..e8693fb207 100644 --- a/src/front/glsl/functions.rs +++ b/src/front/glsl/functions.rs @@ -9,7 +9,8 @@ use super::{ use crate::{ front::glsl::types::type_power, proc::ensure_block_returns, Arena, Block, Constant, ConstantInner, EntryPoint, Expression, FastHashMap, Function, FunctionArgument, FunctionResult, - Handle, LocalVariable, ScalarKind, ScalarValue, Span, Statement, StructMember, Type, TypeInner, + Handle, LocalVariable, ScalarKind, ScalarValue, Span, Statement, StorageClass, StructMember, + Type, TypeInner, }; use std::iter; @@ -549,47 +550,85 @@ impl Parser { ctx.lower_expect_inner(stmt, self, *expr, parameter_info.qualifier.as_pos(), body)?; if parameter_info.qualifier.is_lhs() { - // If the argument is to be passed as a pointer but the type of the - // expression returns a vector it must mean that it was for example - // swizzled and it must be spilled into a local before calling - if let TypeInner::Vector { size, kind, width } = - *self.resolve_type(ctx, handle, meta)? - { - let ty = self.module.types.insert( - Type { - name: None, - inner: TypeInner::Vector { size, kind, width }, - }, - Span::default(), - ); - let temp_var = ctx.locals.append( - LocalVariable { - name: None, - ty, - init: None, - }, - Span::default(), - ); - let temp_expr = ctx.add_expression( - Expression::LocalVariable(temp_var), - Span::default(), - body, - ); + let (ty, value) = match *self.resolve_type(ctx, handle, meta)? { + // If the argument is to be passed as a pointer but the type of the + // expression returns a vector it must mean that it was for example + // swizzled and it must be spilled into a local before calling + // TODO: this part doesn't work because of #1385 once that's sorted out + // revisit this part. + TypeInner::Vector { size, kind, width } => ( + self.module.types.insert( + Type { + name: None, + inner: TypeInner::Vector { size, kind, width }, + }, + Span::default(), + ), + handle, + ), + // If the argument is a pointer whose storage class isn't `Function` an + // indirection trough a local variable is needed to align the storage + // classes of the call argument and the overload parameter + TypeInner::Pointer { base, class } if class != StorageClass::Function => ( + base, + ctx.add_expression( + Expression::Load { pointer: handle }, + Span::default(), + body, + ), + ), + TypeInner::ValuePointer { + size, + kind, + width, + class, + } if class != StorageClass::Function => { + let inner = match size { + Some(size) => TypeInner::Vector { size, kind, width }, + None => TypeInner::Scalar { kind, width }, + }; + + ( + self.module + .types + .insert(Type { name: None, inner }, Span::default()), + ctx.add_expression( + Expression::Load { pointer: handle }, + Span::default(), + body, + ), + ) + } + _ => { + arguments.push(handle); + continue; + } + }; - body.push( - Statement::Store { - pointer: temp_expr, - value: handle, - }, - Span::default(), - ); + let temp_var = ctx.locals.append( + LocalVariable { + name: None, + ty, + init: None, + }, + Span::default(), + ); + let temp_expr = + ctx.add_expression(Expression::LocalVariable(temp_var), Span::default(), body); - arguments.push(temp_expr); - // Register the temporary local to be written back to it's original - // place after the function call - proxy_writes.push((*expr, temp_expr)); - continue; - } + body.push( + Statement::Store { + pointer: temp_expr, + value, + }, + Span::default(), + ); + + arguments.push(temp_expr); + // Register the temporary local to be written back to it's original + // place after the function call + proxy_writes.push((handle, temp_expr)); + continue; } // Apply implicit conversions as needed @@ -623,18 +662,15 @@ impl Parser { ctx.emit_start(); // Write back all the variables that were scheduled to their original place - for (tgt, pointer) in proxy_writes { + for (original, pointer) in proxy_writes { let value = ctx.add_expression(Expression::Load { pointer }, meta, body); - let target = ctx - .lower_expect_inner(stmt, self, tgt, ExprPos::Rhs, body)? - .0; ctx.emit_flush(body); ctx.emit_start(); body.push( Statement::Store { - pointer: target, + pointer: original, value, }, meta, diff --git a/tests/in/glsl/expressions.frag b/tests/in/glsl/expressions.frag index 58176f5aea..105d6116d8 100644 --- a/tests/in/glsl/expressions.frag +++ b/tests/in/glsl/expressions.frag @@ -63,18 +63,22 @@ void testBinOpUintUVec(uint a, uvec4 b) { } void testStructConstructor() { - struct BST { - int data; - }; + struct BST { + int data; + }; - BST tree = BST(1); + BST tree = BST(1); } void testArrayConstructor() { - float tree[1] = float[1](0.0); + float tree[1] = float[1](0.0); } +float global; +void privatePointer(inout float a) {} + out vec4 o_color; void main() { + privatePointer(global); o_color.rgba = vec4(1.0); } diff --git a/tests/out/wgsl/expressions-frag.wgsl b/tests/out/wgsl/expressions-frag.wgsl index f7514881ee..8bed050c12 100644 --- a/tests/out/wgsl/expressions-frag.wgsl +++ b/tests/out/wgsl/expressions-frag.wgsl @@ -2,6 +2,7 @@ struct BST { data: i32; }; +var global: f32; var o_color: vec4; fn testBinOpVecFloat(a: vec4, b: f32) { @@ -184,13 +185,24 @@ fn testArrayConstructor() { } +fn privatePointer(a12: ptr) { + return; +} + fn main1() { - let e1: vec4 = o_color; - let e4: vec4 = vec4(1.0); - o_color.x = e4.x; - o_color.y = e4.y; - o_color.z = e4.z; - o_color.w = e4.w; + var local: f32; + + let e3: f32 = global; + local = e3; + privatePointer((&local)); + let e5: f32 = local; + global = e5; + let e6: vec4 = o_color; + let e9: vec4 = vec4(1.0); + o_color.x = e9.x; + o_color.y = e9.y; + o_color.z = e9.z; + o_color.w = e9.w; return; }