Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omit non referenced expressions #2378

Merged
merged 1 commit into from
Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/back/glsl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1803,8 +1803,6 @@ impl<'a, W: Write> Writer<'a, W> {
Some(self.namer.call(name))
} else if self.need_bake_expressions.contains(&handle) {
Some(format!("{}{}", back::BAKE_PREFIX, handle.index()))
} else if info.ref_count == 0 {
Some(self.namer.call(""))
} else {
None
};
Expand Down
2 changes: 0 additions & 2 deletions src/back/hlsl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1336,8 +1336,6 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
Some(self.namer.call(name))
} else if self.need_bake_expressions.contains(&handle) {
Some(format!("_expr{}", handle.index()))
} else if info.ref_count == 0 {
Some(self.namer.call(""))
} else {
None
};
Expand Down
2 changes: 0 additions & 2 deletions src/back/msl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2581,8 +2581,6 @@ impl<W: Write> Writer<W> {
// Don't assume the names in `named_expressions` are unique,
// or even valid. Use the `Namer`.
Some(self.namer.call(name))
} else if info.ref_count == 0 {
Some(self.namer.call(""))
} else {
// If this expression is an index that we're going to first compare
// against a limit, and then actually use as an index, then we may
Expand Down
9 changes: 9 additions & 0 deletions src/back/spv/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,15 @@ impl<'w> BlockContext<'w> {
expr_handle: Handle<crate::Expression>,
block: &mut Block,
) -> Result<(), Error> {
let is_named_expression = self
.ir_function
.named_expressions
.contains_key(&expr_handle);

if self.fun_info[expr_handle].ref_count == 0 && !is_named_expression {
return Ok(());
}

let result_type_id = self.get_expression_type_id(&self.fun_info[expr_handle].ty);
let id = match self.ir_function.expressions[expr_handle] {
crate::Expression::Access { base, index: _ } if self.is_intermediate(base) => {
Expand Down
5 changes: 0 additions & 5 deletions src/back/wgsl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -634,11 +634,6 @@ impl<W: Write> Writer<W> {
// Otherwise, we could accidentally write variable name instead of full expression.
// Also, we use sanitized names! It defense backend from generating variable with name from reserved keywords.
Some(self.namer.call(name))
} else if info.ref_count == 0 {
write!(self.out, "{level}_ = ")?;
self.write_expr(module, handle, func_ctx)?;
writeln!(self.out, ";")?;
continue;
} else {
let expr = &func_ctx.expressions[handle];
let min_ref_count = expr.bake_ref_count();
Expand Down
14 changes: 11 additions & 3 deletions src/valid/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,12 +675,17 @@ impl FunctionInfo {
requirements: UniformityRequirements::empty(),
},
E::Math {
arg, arg1, arg2, ..
fun: _,
arg,
arg1,
arg2,
arg3,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch. I think I've seen several of these fixed recently. I did a quick look and I think this must the last - the other places that use Math { .. } are sites that can't produce results at all without the arguments, so .. is probably okay there.

} => {
let arg1_nur = arg1.and_then(|h| self.add_ref(h));
let arg2_nur = arg2.and_then(|h| self.add_ref(h));
let arg3_nur = arg3.and_then(|h| self.add_ref(h));
Uniformity {
non_uniform_result: self.add_ref(arg).or(arg1_nur).or(arg2_nur),
non_uniform_result: self.add_ref(arg).or(arg1_nur).or(arg2_nur).or(arg3_nur),
requirements: UniformityRequirements::empty(),
}
}
Expand Down Expand Up @@ -868,7 +873,7 @@ impl FunctionInfo {
S::Loop {
ref body,
ref continuing,
break_if: _,
break_if,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another nice catch.

} => {
let body_uniformity =
self.process_block(body, other_functions, disruptor, expression_arena)?;
Expand All @@ -879,6 +884,9 @@ impl FunctionInfo {
continuing_disruptor,
expression_arena,
)?;
if let Some(expr) = break_if {
let _ = self.add_ref(expr);
}
body_uniformity | continuing_uniformity
}
S::Return { value } => FunctionUniformity {
Expand Down
32 changes: 16 additions & 16 deletions tests/in/access.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ fn test_matrix_within_struct_accesses() {
idx--;

// loads
_ = baz.m;
_ = baz.m[0];
_ = baz.m[idx];
_ = baz.m[0][1];
_ = baz.m[0][idx];
_ = baz.m[idx][1];
_ = baz.m[idx][idx];
let l0 = baz.m;
let l1 = baz.m[0];
let l2 = baz.m[idx];
let l3 = baz.m[0][1];
let l4 = baz.m[0][idx];
let l5 = baz.m[idx][1];
let l6 = baz.m[idx][idx];

var t = Baz(mat3x2<f32>(vec2<f32>(1.0), vec2<f32>(2.0), vec2<f32>(3.0)));

Expand Down Expand Up @@ -75,14 +75,14 @@ fn test_matrix_within_array_within_struct_accesses() {
idx--;

// loads
_ = nested_mat_cx2.am;
_ = nested_mat_cx2.am[0];
_ = nested_mat_cx2.am[0][0];
_ = nested_mat_cx2.am[0][idx];
_ = nested_mat_cx2.am[0][0][1];
_ = nested_mat_cx2.am[0][0][idx];
_ = nested_mat_cx2.am[0][idx][1];
_ = nested_mat_cx2.am[0][idx][idx];
let l0 = nested_mat_cx2.am;
let l1 = nested_mat_cx2.am[0];
let l2 = nested_mat_cx2.am[0][0];
let l3 = nested_mat_cx2.am[0][idx];
let l4 = nested_mat_cx2.am[0][0][1];
let l5 = nested_mat_cx2.am[0][0][idx];
let l6 = nested_mat_cx2.am[0][idx][1];
let l7 = nested_mat_cx2.am[0][idx][idx];

var t = MatCx2InArray(array<mat4x2<f32>, 2>());

Expand Down Expand Up @@ -134,7 +134,7 @@ fn foo_vert(@builtin(vertex_index) vi: u32) -> @builtin(position) vec4<f32> {
c2[vi + 1u] = 42;
let value = c2[vi];

_ = test_arr_as_arg(array<array<f32, 10>, 5>());
test_arr_as_arg(array<array<f32, 10>, 5>());

return vec4<f32>(_matrix * vec4<f32>(vec4<i32>(value)), 2.0);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/in/array-in-ctor.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ var<storage> ah: Ah;

@compute @workgroup_size(1)
fn cs_main() {
_ = ah;
let ah = ah;
}
16 changes: 8 additions & 8 deletions tests/in/atomicOps.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ fn cs_main(@builtin(local_invocation_id) id: vec3<u32>) {

workgroupBarrier();

atomicLoad(&storage_atomic_scalar);
atomicLoad(&storage_atomic_arr[1]);
atomicLoad(&storage_struct.atomic_scalar);
atomicLoad(&storage_struct.atomic_arr[1]);
atomicLoad(&workgroup_atomic_scalar);
atomicLoad(&workgroup_atomic_arr[1]);
atomicLoad(&workgroup_struct.atomic_scalar);
atomicLoad(&workgroup_struct.atomic_arr[1]);
let l0 = atomicLoad(&storage_atomic_scalar);
let l1 = atomicLoad(&storage_atomic_arr[1]);
let l2 = atomicLoad(&storage_struct.atomic_scalar);
let l3 = atomicLoad(&storage_struct.atomic_arr[1]);
let l4 = atomicLoad(&workgroup_atomic_scalar);
let l5 = atomicLoad(&workgroup_atomic_arr[1]);
let l6 = atomicLoad(&workgroup_struct.atomic_scalar);
let l7 = atomicLoad(&workgroup_struct.atomic_arr[1]);

workgroupBarrier();

Expand Down
12 changes: 6 additions & 6 deletions tests/in/globals.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,17 @@ fn test_msl_packed_vec3() {
let data = alignment;

// loads
_ = data.v3;
_ = data.v3.zx;
let l0 = data.v3;
let l1 = data.v3.zx;
test_msl_packed_vec3_as_arg(data.v3);

// matrix vector multiplication
_ = data.v3 * mat3x3<f32>();
_ = mat3x3<f32>() * data.v3;
let mvm0 = data.v3 * mat3x3<f32>();
let mvm1 = mat3x3<f32>() * data.v3;

// scalar vector multiplication
_ = data.v3 * 2.0;
_ = 2.0 * data.v3;
let svm0 = data.v3 * 2.0;
let svm1 = 2.0 * data.v3;
}

@compute @workgroup_size(1)
Expand Down
Loading