[glsl-in] Don't reinterpret function arguments twice in the normal case

[magcius]

Connections
Fixes #6602

Description
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.

Testing
See shader changes.

Checklist

• Run cargo fmt.
• Run taplo format.
• Run cargo clippy. If applicable, add:
  • --target wasm32-unknown-unknown
  • --target wasm32-unknown-emscripten
• Run cargo xtask test to run tests.
• Add change to CHANGELOG.md. See simple instructions inside file.

[jimblandy]

The code changes look good to me, but I think it should have a test that clearly shows what's being fixed. The effects I saw on the snapshot output files were:

• In the (handful of) GLSL snapshot inputs that do use out or inout parameters, there were no changes.

• In those that do not it cleaned up some unused temporaries, but I didn't notice any code going from incorrect to correct.

So unless I missed it, that means we don't currently have any snapshot inputs that demonstrate why this fix is necessary.

[magcius]

Yeah, the code generated was technically fine before, but it was a lot of code to chew through, and with a lot of functions, you could easily get a lot of temporaries emitted when none were necessary. In particular, it generated code that caused Chrome to take 20s or so to compile the pipeline.

Before this change, I had a shader that emitted a lot of unused temporaries: https://gist.github.com/magcius/95298da1c344376cd9090c12a44bffdb#file-test-wgsl-L542-L618

With this change, that now gets reduced to this, which is still a lot of code, but way less before, and now at least every temporary is used:

    let _e85 = global_1.u_LightParams[1];
    let _e88 = v_Position;
    t_LightDelta = (_e85.Position.xyz - _e88.xyz);
    let _e91 = t_LightDelta;
    let _e92 = t_LightDelta;
    t_LightDeltaDist2_ = dot(_e91, _e92);
    let _e94 = t_LightDeltaDist2_;
    t_LightDeltaDist = sqrt(_e94);
    let _e96 = t_LightDelta;
    let _e97 = t_LightDeltaDist;
    t_LightDeltaDir = (_e96 / vec3(_e97));
    let _e104 = global_1.u_LightParams[1];
    let _e108 = t_LightDeltaDir;
    let _e111 = global_1.u_LightParams[1];
    let _e116 = ApplyAttenuation(_e104.CosAtten.xyz, max(0f, dot(_e108, _e111.Direction.xyz)));
    let _e120 = global_1.u_LightParams[1];
    let _e125 = t_LightDeltaDist;
    let _e126 = t_LightDeltaDist2_;
    t_Attenuation = max(0f, (max(0f, _e116) / dot(normalize(_e120.DistAtten.xyz), vec3<f32>(1f, _e125, _e126))));
    let _e131 = t_LightAccum;
    let _e132 = t_Normal;
    let _e133 = t_LightDeltaDir;
    let _e137 = t_Attenuation;
    let _e141 = global_1.u_LightParams[1];
    t_LightAccum = (_e131 + ((max(dot(_e132, _e133), 0f) * _e137) * _e141.Color));

In the (handful of) GLSL snapshot inputs that do use out or inout parameters, there were no changes.

Yes, there aren't any changes for inout/out, those will still generate unused temporaries; the function code first generates an expression for the parameter assuming it's in an RHS position, and it doesn't look easy to fix that. So it's not a full fix, yet, but inout/out are much more rare by comparison.

[jimblandy]

Okay, given that this is a cleanup, not a correction, this looks fine.