Skip to content

Commit ce631c1

Browse files
fix(naga): Evaluate compound assignment LHS before RHS (#9181)
1 parent a18abdd commit ce631c1

40 files changed

+2011
-1971
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ depth_stencil: Some(wgpu::DepthStencilState::stencil(
209209
- Naga now detects bitwise shifts by a constant exceeding the operand bit width at compile time, and disallows scalar-by-vector and vector-by-scalar shifts in constant evaluation. By @andyleiserson in [#8907](https://github.com/gfx-rs/wgpu/pull/8907).
210210
- Naga uses wrapping arithmetic when evaluating dot products on concrete integer types (`u32` and `i32`). By @BKDaugherty in [#9142](https://github.com/gfx-rs/wgpu/pull/9142).
211211
- Disallow negation of a matrix in WGSL. By @andyleiserson in [#9157](https://github.com/gfx-rs/wgpu/pull/9157).
212+
- Fix evaluation order of compound assignment (e.g. `+=`) LHS and RHS. By @andyleiserson in [#9181](https://github.com/gfx-rs/wgpu/pull/9181).
212213

213214
#### Validation
214215

cts_runner/test.lst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ webgpu:shader,execution,flow_control,return:*
324324
// Many other vertex_buffer_access subtests also passing, but there are too many to enumerate.
325325
// Fails on Metal in CI only, not when running locally.
326326
fails-if(metal) webgpu:shader,execution,robust_access_vertex:vertex_buffer_access:indexed=true;indirect=false;drawCallTestParameter="baseVertex";type="float32x4";additionalBuffers=4;partialLastNumber=false;offsetVertexBuffer=true
327+
webgpu:shader,execution,statement,compound:*
327328

328329
webgpu:shader,validation,const_assert,const_assert:*
329330
webgpu:shader,validation,decl,assignment_statement:*

naga/src/front/wgsl/lower/mod.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2153,6 +2153,13 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {
21532153
.pointer_automatically_convertible_scalar(&ectx.module.types),
21542154
};
21552155

2156+
// Need to emit the LHS _before_ the RHS so that it is evaluated first.
2157+
let op_assign = if let Some(op) = op {
2158+
Some((op, ectx.apply_load_rule(target)?))
2159+
} else {
2160+
None
2161+
};
2162+
21562163
let value = self.expression_for_abstract(value, &mut ectx)?;
21572164
let mut value = match target_scalar {
21582165
Some(target_scalar) => ectx.try_automatic_conversion_for_leaf_scalar(
@@ -2163,9 +2170,8 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {
21632170
None => value,
21642171
};
21652172

2166-
let value = match op {
2167-
Some(op) => {
2168-
let mut left = ectx.apply_load_rule(target)?;
2173+
let value = match op_assign {
2174+
Some((op, mut left)) => {
21692175
ectx.binary_op_splat(op, &mut left, &mut value)?;
21702176
ectx.append_expression(
21712177
ir::Expression::Binary {

naga/tests/out/glsl/wgsl-break-if.main.Compute.glsl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ void breakIfSeparateVariable() {
6868
}
6969
}
7070
loop_init_3 = false;
71-
uint _e3 = counter;
72-
counter = (_e3 + 1u);
71+
uint _e2 = counter;
72+
counter = (_e2 + 1u);
7373
}
7474
return;
7575
}

naga/tests/out/glsl/wgsl-image.texture_sample.Fragment.glsl

Lines changed: 69 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -14,75 +14,75 @@ void main() {
1414
vec2 _e1 = vec2(0.5);
1515
vec3 _e3 = vec3(0.5);
1616
ivec2 _e6 = ivec2(3, 1);
17-
vec4 _e11 = texture(_group_0_binding_0_fs, 0.5);
18-
vec4 _e12 = a;
19-
a = (_e12 + _e11);
20-
vec4 _e16 = texture(_group_0_binding_1_fs, vec2(_e1));
21-
vec4 _e17 = a;
22-
a = (_e17 + _e16);
23-
vec4 _e24 = textureOffset(_group_0_binding_1_fs, vec2(_e1), ivec2(3, 1));
24-
vec4 _e25 = a;
25-
a = (_e25 + _e24);
26-
vec4 _e29 = textureLod(_group_0_binding_1_fs, vec2(_e1), 2.3);
27-
vec4 _e30 = a;
28-
a = (_e30 + _e29);
29-
vec4 _e34 = textureLodOffset(_group_0_binding_1_fs, vec2(_e1), 2.3, ivec2(3, 1));
30-
vec4 _e35 = a;
31-
a = (_e35 + _e34);
32-
vec4 _e40 = textureOffset(_group_0_binding_1_fs, vec2(_e1), ivec2(3, 1), 2.0);
33-
vec4 _e41 = a;
34-
a = (_e41 + _e40);
35-
vec4 _e45 = textureLod(_group_0_binding_1_fs, vec2(_e1), 0.0);
36-
vec4 _e46 = a;
37-
a = (_e46 + _e45);
38-
vec4 _e51 = texture(_group_0_binding_4_fs, vec3(_e1, 0u));
39-
vec4 _e52 = a;
40-
a = (_e52 + _e51);
41-
vec4 _e57 = textureOffset(_group_0_binding_4_fs, vec3(_e1, 0u), ivec2(3, 1));
42-
vec4 _e58 = a;
43-
a = (_e58 + _e57);
44-
vec4 _e63 = textureLod(_group_0_binding_4_fs, vec3(_e1, 0u), 2.3);
45-
vec4 _e64 = a;
46-
a = (_e64 + _e63);
47-
vec4 _e69 = textureLodOffset(_group_0_binding_4_fs, vec3(_e1, 0u), 2.3, ivec2(3, 1));
48-
vec4 _e70 = a;
49-
a = (_e70 + _e69);
50-
vec4 _e76 = textureOffset(_group_0_binding_4_fs, vec3(_e1, 0u), ivec2(3, 1), 2.0);
51-
vec4 _e77 = a;
52-
a = (_e77 + _e76);
53-
vec4 _e82 = texture(_group_0_binding_4_fs, vec3(_e1, 0));
54-
vec4 _e83 = a;
55-
a = (_e83 + _e82);
56-
vec4 _e88 = textureOffset(_group_0_binding_4_fs, vec3(_e1, 0), ivec2(3, 1));
57-
vec4 _e89 = a;
58-
a = (_e89 + _e88);
59-
vec4 _e94 = textureLod(_group_0_binding_4_fs, vec3(_e1, 0), 2.3);
60-
vec4 _e95 = a;
61-
a = (_e95 + _e94);
62-
vec4 _e100 = textureLodOffset(_group_0_binding_4_fs, vec3(_e1, 0), 2.3, ivec2(3, 1));
63-
vec4 _e101 = a;
64-
a = (_e101 + _e100);
65-
vec4 _e107 = textureOffset(_group_0_binding_4_fs, vec3(_e1, 0), ivec2(3, 1), 2.0);
66-
vec4 _e108 = a;
67-
a = (_e108 + _e107);
68-
vec4 _e113 = texture(_group_0_binding_6_fs, vec4(_e3, 0u));
69-
vec4 _e114 = a;
70-
a = (_e114 + _e113);
71-
vec4 _e119 = textureLod(_group_0_binding_6_fs, vec4(_e3, 0u), 2.3);
72-
vec4 _e120 = a;
73-
a = (_e120 + _e119);
74-
vec4 _e126 = texture(_group_0_binding_6_fs, vec4(_e3, 0u), 2.0);
75-
vec4 _e127 = a;
76-
a = (_e127 + _e126);
77-
vec4 _e132 = texture(_group_0_binding_6_fs, vec4(_e3, 0));
78-
vec4 _e133 = a;
79-
a = (_e133 + _e132);
80-
vec4 _e138 = textureLod(_group_0_binding_6_fs, vec4(_e3, 0), 2.3);
81-
vec4 _e139 = a;
82-
a = (_e139 + _e138);
83-
vec4 _e145 = texture(_group_0_binding_6_fs, vec4(_e3, 0), 2.0);
84-
vec4 _e146 = a;
85-
a = (_e146 + _e145);
17+
vec4 _e9 = a;
18+
vec4 _e12 = texture(_group_0_binding_0_fs, 0.5);
19+
a = (_e9 + _e12);
20+
vec4 _e14 = a;
21+
vec4 _e17 = texture(_group_0_binding_1_fs, vec2(_e1));
22+
a = (_e14 + _e17);
23+
vec4 _e19 = a;
24+
vec4 _e25 = textureOffset(_group_0_binding_1_fs, vec2(_e1), ivec2(3, 1));
25+
a = (_e19 + _e25);
26+
vec4 _e27 = a;
27+
vec4 _e30 = textureLod(_group_0_binding_1_fs, vec2(_e1), 2.3);
28+
a = (_e27 + _e30);
29+
vec4 _e32 = a;
30+
vec4 _e35 = textureLodOffset(_group_0_binding_1_fs, vec2(_e1), 2.3, ivec2(3, 1));
31+
a = (_e32 + _e35);
32+
vec4 _e37 = a;
33+
vec4 _e41 = textureOffset(_group_0_binding_1_fs, vec2(_e1), ivec2(3, 1), 2.0);
34+
a = (_e37 + _e41);
35+
vec4 _e43 = a;
36+
vec4 _e46 = textureLod(_group_0_binding_1_fs, vec2(_e1), 0.0);
37+
a = (_e43 + _e46);
38+
vec4 _e48 = a;
39+
vec4 _e52 = texture(_group_0_binding_4_fs, vec3(_e1, 0u));
40+
a = (_e48 + _e52);
41+
vec4 _e54 = a;
42+
vec4 _e58 = textureOffset(_group_0_binding_4_fs, vec3(_e1, 0u), ivec2(3, 1));
43+
a = (_e54 + _e58);
44+
vec4 _e60 = a;
45+
vec4 _e64 = textureLod(_group_0_binding_4_fs, vec3(_e1, 0u), 2.3);
46+
a = (_e60 + _e64);
47+
vec4 _e66 = a;
48+
vec4 _e70 = textureLodOffset(_group_0_binding_4_fs, vec3(_e1, 0u), 2.3, ivec2(3, 1));
49+
a = (_e66 + _e70);
50+
vec4 _e72 = a;
51+
vec4 _e77 = textureOffset(_group_0_binding_4_fs, vec3(_e1, 0u), ivec2(3, 1), 2.0);
52+
a = (_e72 + _e77);
53+
vec4 _e79 = a;
54+
vec4 _e83 = texture(_group_0_binding_4_fs, vec3(_e1, 0));
55+
a = (_e79 + _e83);
56+
vec4 _e85 = a;
57+
vec4 _e89 = textureOffset(_group_0_binding_4_fs, vec3(_e1, 0), ivec2(3, 1));
58+
a = (_e85 + _e89);
59+
vec4 _e91 = a;
60+
vec4 _e95 = textureLod(_group_0_binding_4_fs, vec3(_e1, 0), 2.3);
61+
a = (_e91 + _e95);
62+
vec4 _e97 = a;
63+
vec4 _e101 = textureLodOffset(_group_0_binding_4_fs, vec3(_e1, 0), 2.3, ivec2(3, 1));
64+
a = (_e97 + _e101);
65+
vec4 _e103 = a;
66+
vec4 _e108 = textureOffset(_group_0_binding_4_fs, vec3(_e1, 0), ivec2(3, 1), 2.0);
67+
a = (_e103 + _e108);
68+
vec4 _e110 = a;
69+
vec4 _e114 = texture(_group_0_binding_6_fs, vec4(_e3, 0u));
70+
a = (_e110 + _e114);
71+
vec4 _e116 = a;
72+
vec4 _e120 = textureLod(_group_0_binding_6_fs, vec4(_e3, 0u), 2.3);
73+
a = (_e116 + _e120);
74+
vec4 _e122 = a;
75+
vec4 _e127 = texture(_group_0_binding_6_fs, vec4(_e3, 0u), 2.0);
76+
a = (_e122 + _e127);
77+
vec4 _e129 = a;
78+
vec4 _e133 = texture(_group_0_binding_6_fs, vec4(_e3, 0));
79+
a = (_e129 + _e133);
80+
vec4 _e135 = a;
81+
vec4 _e139 = textureLod(_group_0_binding_6_fs, vec4(_e3, 0), 2.3);
82+
a = (_e135 + _e139);
83+
vec4 _e141 = a;
84+
vec4 _e146 = texture(_group_0_binding_6_fs, vec4(_e3, 0), 2.0);
85+
a = (_e141 + _e146);
8686
vec4 _e148 = a;
8787
_fs2p_location0 = _e148;
8888
return;

naga/tests/out/glsl/wgsl-image.texture_sample_comparison.Fragment.glsl

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,30 +11,30 @@ void main() {
1111
float a_1 = 0.0;
1212
vec2 tc = vec2(0.5);
1313
vec3 tc3_ = vec3(0.5);
14-
float _e8 = texture(_group_1_binding_2_fs, vec3(tc, 0.5));
15-
float _e9 = a_1;
16-
a_1 = (_e9 + _e8);
17-
float _e14 = texture(_group_1_binding_3_fs, vec4(tc, 0u, 0.5));
18-
float _e15 = a_1;
19-
a_1 = (_e15 + _e14);
20-
float _e20 = texture(_group_1_binding_3_fs, vec4(tc, 0, 0.5));
21-
float _e21 = a_1;
22-
a_1 = (_e21 + _e20);
23-
float _e25 = texture(_group_1_binding_4_fs, vec4(tc3_, 0.5));
24-
float _e26 = a_1;
25-
a_1 = (_e26 + _e25);
26-
float _e30 = textureLod(_group_1_binding_2_fs, vec3(tc, 0.5), 0.0);
27-
float _e31 = a_1;
28-
a_1 = (_e31 + _e30);
29-
float _e36 = textureGrad(_group_1_binding_3_fs, vec4(tc, 0u, 0.5), vec2(0.0), vec2(0.0));
30-
float _e37 = a_1;
31-
a_1 = (_e37 + _e36);
32-
float _e42 = textureGrad(_group_1_binding_3_fs, vec4(tc, 0, 0.5), vec2(0.0), vec2(0.0));
33-
float _e43 = a_1;
34-
a_1 = (_e43 + _e42);
35-
float _e47 = textureGrad(_group_1_binding_4_fs, vec4(tc3_, 0.5), vec3(0.0), vec3(0.0));
36-
float _e48 = a_1;
37-
a_1 = (_e48 + _e47);
14+
float _e6 = a_1;
15+
float _e9 = texture(_group_1_binding_2_fs, vec3(tc, 0.5));
16+
a_1 = (_e6 + _e9);
17+
float _e11 = a_1;
18+
float _e15 = texture(_group_1_binding_3_fs, vec4(tc, 0u, 0.5));
19+
a_1 = (_e11 + _e15);
20+
float _e17 = a_1;
21+
float _e21 = texture(_group_1_binding_3_fs, vec4(tc, 0, 0.5));
22+
a_1 = (_e17 + _e21);
23+
float _e23 = a_1;
24+
float _e26 = texture(_group_1_binding_4_fs, vec4(tc3_, 0.5));
25+
a_1 = (_e23 + _e26);
26+
float _e28 = a_1;
27+
float _e31 = textureLod(_group_1_binding_2_fs, vec3(tc, 0.5), 0.0);
28+
a_1 = (_e28 + _e31);
29+
float _e33 = a_1;
30+
float _e37 = textureGrad(_group_1_binding_3_fs, vec4(tc, 0u, 0.5), vec2(0.0), vec2(0.0));
31+
a_1 = (_e33 + _e37);
32+
float _e39 = a_1;
33+
float _e43 = textureGrad(_group_1_binding_3_fs, vec4(tc, 0, 0.5), vec2(0.0), vec2(0.0));
34+
a_1 = (_e39 + _e43);
35+
float _e45 = a_1;
36+
float _e48 = textureGrad(_group_1_binding_4_fs, vec4(tc3_, 0.5), vec3(0.0), vec3(0.0));
37+
a_1 = (_e45 + _e48);
3838
float _e50 = a_1;
3939
_fs2p_location0 = _e50;
4040
return;

naga/tests/out/glsl/wgsl-operators.main.Compute.glsl

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ vec4 splat(float m, int n) {
3737

3838
vec2 splat_assignment() {
3939
vec2 a = vec2(2.0);
40-
vec2 _e4 = a;
41-
a = (_e4 + vec2(1.0));
42-
vec2 _e8 = a;
43-
a = (_e8 - vec2(3.0));
44-
vec2 _e12 = a;
45-
a = (_e12 / vec2(4.0));
40+
vec2 _e3 = a;
41+
a = (_e3 + vec2(1.0));
42+
vec2 _e7 = a;
43+
a = (_e7 - vec2(3.0));
44+
vec2 _e11 = a;
45+
a = (_e11 / vec2(4.0));
4646
vec2 _e15 = a;
4747
return _e15;
4848
}
@@ -291,10 +291,10 @@ void assignment() {
291291
a_1 = (_e7 - 1);
292292
int _e9 = a_1;
293293
int _e10 = a_1;
294-
a_1 = (_e10 * _e9);
294+
a_1 = (_e9 * _e10);
295295
int _e12 = a_1;
296296
int _e13 = a_1;
297-
a_1 = (_e13 / _e12);
297+
a_1 = (_e12 / _e13);
298298
int _e15 = a_1;
299299
a_1 = (_e15 % 1);
300300
int _e17 = a_1;

naga/tests/out/glsl/wgsl-shadow.fs_main.Fragment.glsl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ void main() {
7272
float _e23 = fetch_shadow(_e19, (light.proj * in_.world_position));
7373
vec3 light_dir = normalize((light.pos.xyz - in_.world_position.xyz));
7474
float diffuse = max(0.0, dot(normal_1, light_dir));
75-
vec3 _e37 = color;
76-
color = (_e37 + ((_e23 * diffuse) * light.color.xyz));
75+
vec3 _e33 = color;
76+
color = (_e33 + ((_e23 * diffuse) * light.color.xyz));
7777
}
7878
}
7979
vec3 _e42 = color;

naga/tests/out/glsl/wgsl-shadow.fs_main_without_storage.Fragment.glsl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ void main() {
7272
float _e23 = fetch_shadow(_e19, (light.proj * in_1.world_position));
7373
vec3 light_dir = normalize((light.pos.xyz - in_1.world_position.xyz));
7474
float diffuse = max(0.0, dot(normal_1, light_dir));
75-
vec3 _e37 = color_1;
76-
color_1 = (_e37 + ((_e23 * diffuse) * light.color.xyz));
75+
vec3 _e33 = color_1;
76+
color_1 = (_e33 + ((_e23 * diffuse) * light.color.xyz));
7777
}
7878
}
7979
vec3 _e42 = color_1;

0 commit comments

Comments
 (0)