Skip to content

Commit

Permalink
Parenthesize unary negations to avoid -- (gfx-rs#2087)
Browse files Browse the repository at this point in the history
* fix(glsl-out,hlsl-out,msl-out): parenthesize unary negations a la `wgsl` everywhere

Unify parenthesization of unary negations across all backends with what the `wgsl` backend does,
which is `<op>(<expr>)`. This avoids ambiguity with output languages for which `--` is a different
operation; in this case, we've been accidentally emitting prefix decrements.

* build: update `rspirv` 0.11 -> 0.12 (FIXME: use upstream release)

* test: add `operators::negation_avoids_prefix_decrement` test

Co-authored-by: Dzmitry Malyshau <kvark@fastmail.com>
  • Loading branch information
ErichDonGubler and kvark authored Nov 17, 2022
1 parent e056805 commit aa22301
Show file tree
Hide file tree
Showing 10 changed files with 666 additions and 581 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ diff = "0.1"
ron = "~0.7.1"
serde = { version = "1.0", features = ["derive"] }
spirv = { version = "0.2", features = ["deserialize"] }
rspirv = "0.11"
rspirv = { version = "0.11", git = "https://github.com/gfx-rs/rspirv", rev = "b969f175d5663258b4891e44b76c1544da9661ab" }
env_logger = "0.9"

[workspace]
Expand Down
2 changes: 1 addition & 1 deletion src/back/glsl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2519,7 +2519,7 @@ impl<'a, W: Write> Writer<'a, W> {
},
};

write!(self.out, "({}", operator)?;
write!(self.out, "{}(", operator)?;
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/back/hlsl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2395,8 +2395,9 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
}
},
};
write!(self.out, "{}", op_str)?;
write!(self.out, "{}(", op_str)?;
self.write_expr(module, expr, func_ctx)?;
write!(self.out, ")")?;
}
Expression::As {
expr,
Expand Down
3 changes: 2 additions & 1 deletion src/back/msl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1500,8 +1500,9 @@ impl<W: Write> Writer<W> {
_ => return Err(Error::Validation),
},
};
write!(self.out, "{}", op_str)?;
write!(self.out, "{}(", op_str)?;
self.put_expression(expr, context, false)?;
write!(self.out, ")")?;
}
crate::Expression::Binary { op, left, right } => {
let op_str = crate::back::binary_operation_str(op);
Expand Down
13 changes: 11 additions & 2 deletions tests/in/operators.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,6 @@ fn logical() {

fn arithmetic() {
// unary
// TODO: uncomment when we get the changes from https://github.com/gfx-rs/rspirv/pull/231
// _ = -1;
_ = -1.0;
_ = -vec2(1);
_ = -vec2(1.0);
Expand Down Expand Up @@ -314,3 +312,14 @@ fn main() {
comparison();
assignment();
}

fn negation_avoids_prefix_decrement() {
_ = -1;
_ = - -2;
_ = -(-3);
_ = -(- 4);
_ = - - -5;
_ = - - - - 6;
_ = - - -(- -7);
_ = (- - - - -8);
}
24 changes: 17 additions & 7 deletions tests/out/glsl/operators.main.Compute.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ float constructors() {
}

void logical() {
bool unnamed_11 = (!true);
bool unnamed_11 = !(true);
bvec2 unnamed_12 = not(bvec2(true));
bool unnamed_13 = (true || false);
bool unnamed_14 = (true && false);
Expand All @@ -78,8 +78,8 @@ void logical() {
}

void arithmetic() {
ivec2 unnamed_19 = (-ivec2(1));
vec2 unnamed_20 = (-vec2(1.0));
ivec2 unnamed_19 = -(ivec2(1));
vec2 unnamed_20 = -(vec2(1.0));
int unnamed_21 = (2 + 1);
uint unnamed_22 = (2u + 1u);
float unnamed_23 = (2.0 + 1.0);
Expand Down Expand Up @@ -150,10 +150,10 @@ void arithmetic() {
}

void bit() {
int unnamed_88 = (~1);
uint unnamed_89 = (~1u);
ivec2 unnamed_90 = (~ivec2(1));
uvec3 unnamed_91 = (~uvec3(1u));
int unnamed_88 = ~(1);
uint unnamed_89 = ~(1u);
ivec2 unnamed_90 = ~(ivec2(1));
uvec3 unnamed_91 = ~(uvec3(1u));
int unnamed_92 = (2 | 1);
uint unnamed_93 = (2u | 1u);
ivec2 unnamed_94 = (ivec2(2) | ivec2(1));
Expand Down Expand Up @@ -251,6 +251,16 @@ void assignment() {
return;
}

void negation_avoids_prefix_decrement() {
int unnamed_148 = -(-2);
int unnamed_149 = -(-3);
int unnamed_150 = -(-(4));
int unnamed_151 = -(-(-5));
int unnamed_152 = -(-(-(-(6))));
int unnamed_153 = -(-(-(-(-7))));
int unnamed_154 = -(-(-(-(-8))));
}

void main() {
vec4 _e4 = builtins();
vec4 _e5 = splat();
Expand Down
27 changes: 19 additions & 8 deletions tests/out/hlsl/operators.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ float constructors()

void logical()
{
bool unnamed_11 = !true;
bool2 unnamed_12 = !(true).xx;
bool unnamed_11 = !(true);
bool2 unnamed_12 = !((true).xx);
bool unnamed_13 = (true || false);
bool unnamed_14 = (true && false);
bool unnamed_15 = (true | false);
Expand All @@ -107,8 +107,8 @@ void logical()

void arithmetic()
{
int2 unnamed_19 = -(1).xx;
float2 unnamed_20 = -(1.0).xx;
int2 unnamed_19 = -((1).xx);
float2 unnamed_20 = -((1.0).xx);
int unnamed_21 = (2 + 1);
uint unnamed_22 = (2u + 1u);
float unnamed_23 = (2.0 + 1.0);
Expand Down Expand Up @@ -180,10 +180,10 @@ void arithmetic()

void bit()
{
int unnamed_88 = ~1;
uint unnamed_89 = ~1u;
int2 unnamed_90 = ~(1).xx;
uint3 unnamed_91 = ~(1u).xxx;
int unnamed_88 = ~(1);
uint unnamed_89 = ~(1u);
int2 unnamed_90 = ~((1).xx);
uint3 unnamed_91 = ~((1u).xxx);
int unnamed_92 = (2 | 1);
uint unnamed_93 = (2u | 1u);
int2 unnamed_94 = ((2).xx | (1).xx);
Expand Down Expand Up @@ -284,6 +284,17 @@ void assignment()
return;
}

void negation_avoids_prefix_decrement()
{
int unnamed_148 = -(-2);
int unnamed_149 = -(-3);
int unnamed_150 = -(-(4));
int unnamed_151 = -(-(-5));
int unnamed_152 = -(-(-(-(6))));
int unnamed_153 = -(-(-(-(-7))));
int unnamed_154 = -(-(-(-(-8))));
}

[numthreads(1, 1, 1)]
void main()
{
Expand Down
27 changes: 19 additions & 8 deletions tests/out/msl/operators.msl
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ float constructors(

void logical(
) {
bool unnamed_11 = !true;
metal::bool2 unnamed_12 = !metal::bool2(true);
bool unnamed_11 = !(true);
metal::bool2 unnamed_12 = !(metal::bool2(true));
bool unnamed_13 = true || false;
bool unnamed_14 = true && false;
bool unnamed_15 = true | false;
Expand All @@ -107,8 +107,8 @@ void logical(

void arithmetic(
) {
metal::int2 unnamed_19 = -metal::int2(1);
metal::float2 unnamed_20 = -metal::float2(1.0);
metal::int2 unnamed_19 = -(metal::int2(1));
metal::float2 unnamed_20 = -(metal::float2(1.0));
int unnamed_21 = 2 + 1;
uint unnamed_22 = 2u + 1u;
float unnamed_23 = 2.0 + 1.0;
Expand Down Expand Up @@ -180,10 +180,10 @@ void arithmetic(

void bit(
) {
int unnamed_88 = ~1;
uint unnamed_89 = ~1u;
metal::int2 unnamed_90 = ~metal::int2(1);
metal::uint3 unnamed_91 = ~metal::uint3(1u);
int unnamed_88 = ~(1);
uint unnamed_89 = ~(1u);
metal::int2 unnamed_90 = ~(metal::int2(1));
metal::uint3 unnamed_91 = ~(metal::uint3(1u));
int unnamed_92 = 2 | 1;
uint unnamed_93 = 2u | 1u;
metal::int2 unnamed_94 = metal::int2(2) | metal::int2(1);
Expand Down Expand Up @@ -283,6 +283,17 @@ void assignment(
return;
}

void negation_avoids_prefix_decrement(
) {
int unnamed_148 = -(-2);
int unnamed_149 = -(-3);
int unnamed_150 = -(-(4));
int unnamed_151 = -(-(-5));
int unnamed_152 = -(-(-(-(6))));
int unnamed_153 = -(-(-(-(-7))));
int unnamed_154 = -(-(-(-(-8))));
}

kernel void main_(
) {
metal::float4 _e4 = builtins();
Expand Down
Loading

0 comments on commit aa22301

Please sign in to comment.