Skip to content

Commit

Permalink
[glsl-in] Don't reinterpret function arguments twice in the normal case
Browse files Browse the repository at this point in the history
We already lowered the function expression if it's a normal in argument; no need to do it again. This eliminates an unused variable and expression. With chained function calls, this could lead to a lot of waste.

There's still an extra unused expression in the case of an out/inout argument; ideally we'd remove these expressions, but it might be tricky.

Fixes gfx-rs#6602
  • Loading branch information
magcius authored and jimblandy committed Nov 26, 2024
1 parent e2eeba7 commit 7fff667
Show file tree
Hide file tree
Showing 15 changed files with 1,184 additions and 1,344 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148]
#### Naga

- Show types of LHS and RHS in binary operation type mismatch errors. By @ErichDonGubler in [#6450](https://github.com/gfx-rs/wgpu/pull/6450).
- The GLSL parser now uses less expressions for function calls. By @magcius in [#6604](https://github.com/gfx-rs/wgpu/pull/6604).

#### General

Expand Down
10 changes: 1 addition & 9 deletions naga/src/front/glsl/ast.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{borrow::Cow, fmt};

use super::{builtins::MacroCall, context::ExprPos, Span};
use super::{builtins::MacroCall, Span};
use crate::{
AddressSpace, BinaryOperator, Binding, Constant, Expression, Function, GlobalVariable, Handle,
Interpolation, Literal, Sampling, StorageAccess, Type, UnaryOperator,
Expand Down Expand Up @@ -376,14 +376,6 @@ impl ParameterQualifier {
_ => false,
}
}

/// Converts from a parameter qualifier into a [`ExprPos`]
pub const fn as_pos(&self) -> ExprPos {
match *self {
ParameterQualifier::Out | ParameterQualifier::InOut => ExprPos::Lhs,
_ => ExprPos::Rhs,
}
}
}

/// The GLSL profile used by a shader.
Expand Down
8 changes: 5 additions & 3 deletions naga/src/front/glsl/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -785,10 +785,10 @@ impl Frontend {
.zip(raw_args)
.zip(&parameters)
{
let (mut handle, meta) =
ctx.lower_expect_inner(stmt, self, *expr, parameter_info.qualifier.as_pos())?;

if parameter_info.qualifier.is_lhs() {
// Reprocess argument in LHS position
let (handle, meta) = ctx.lower_expect_inner(stmt, self, *expr, ExprPos::Lhs)?;

self.process_lhs_argument(
ctx,
meta,
Expand All @@ -803,6 +803,8 @@ impl Frontend {
continue;
}

let (mut handle, meta) = *call_argument;

let scalar_comps = scalar_components(&ctx.module.types[*parameter].inner);

// Apply implicit conversions as needed
Expand Down
29 changes: 14 additions & 15 deletions naga/tests/out/wgsl/246-collatz.comp.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,25 @@ fn collatz_iterations(n: u32) -> u32 {
break;
}
{
let _e14 = n_1;
let _e15 = f32(_e14);
if ((_e15 - (floor((_e15 / 2f)) * 2f)) == 0f) {
let _e12 = n_1;
let _e14 = f32(_e12);
if ((_e14 - (floor((_e14 / 2f)) * 2f)) == 0f) {
{
let _e25 = n_1;
n_1 = (_e25 / 2u);
let _e23 = n_1;
n_1 = (_e23 / 2u);
}
} else {
{
let _e30 = n_1;
n_1 = ((3u * _e30) + 1u);
let _e28 = n_1;
n_1 = ((3u * _e28) + 1u);
}
}
let _e36 = i;
i = (_e36 + 1u);
let _e34 = i;
i = (_e34 + 1u);
}
}
let _e39 = i;
return _e39;
let _e37 = i;
return _e37;
}

fn main_1() {
Expand All @@ -45,10 +45,9 @@ fn main_1() {
index = _e3.x;
let _e6 = index;
let _e8 = index;
let _e11 = index;
let _e13 = global.indices[_e11];
let _e14 = collatz_iterations(_e13);
global.indices[_e6] = _e14;
let _e10 = global.indices[_e8];
let _e11 = collatz_iterations(_e10);
global.indices[_e6] = _e11;
return;
}

Expand Down
Loading

0 comments on commit 7fff667

Please sign in to comment.