From b9170c65fc01d4e2be20055cd2c74012f0034650 Mon Sep 17 00:00:00 2001 From: Stephen White Date: Tue, 16 Nov 2021 16:16:56 +0000 Subject: [PATCH] GLSL: fix vector relational ops. In GLSL, relational operators are only valid for scalar operands. For vector operands, lessThan, greaterThan, etc must be used. Bug: tint:1303 Change-Id: Ia800f89111630c756dc1b30ef0c6858fb520fb16 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/69561 Kokoro: Kokoro Commit-Queue: Stephen White Reviewed-by: Ben Clayton --- src/writer/glsl/generator_impl.cc | 52 ++++++++++++++++++++++++++ src/writer/glsl/generator_impl.h | 10 ++++- test/bug/dawn/947.wgsl.expected.glsl | 11 +----- test/bug/tint/913.wgsl.expected.glsl | 2 +- test/bug/tint/942.wgsl.expected.glsl | 2 +- test/bug/tint/943.spvasm.expected.glsl | 4 +- 6 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/writer/glsl/generator_impl.cc b/src/writer/glsl/generator_impl.cc index b7ea9e27c9..6fbc70fa99 100644 --- a/src/writer/glsl/generator_impl.cc +++ b/src/writer/glsl/generator_impl.cc @@ -52,6 +52,19 @@ #include "src/writer/append_vector.h" #include "src/writer/float_to_string.h" +namespace { + +bool IsRelational(tint::ast::BinaryOp op) { + return op == tint::ast::BinaryOp::kEqual || + op == tint::ast::BinaryOp::kNotEqual || + op == tint::ast::BinaryOp::kLessThan || + op == tint::ast::BinaryOp::kGreaterThan || + op == tint::ast::BinaryOp::kLessThanEqual || + op == tint::ast::BinaryOp::kGreaterThanEqual; +} + +} // namespace + namespace tint { namespace writer { namespace glsl { @@ -212,8 +225,47 @@ bool GeneratorImpl::EmitAssign(const ast::AssignmentStatement* stmt) { return true; } +bool GeneratorImpl::EmitVectorRelational(std::ostream& out, + const ast::BinaryExpression* expr) { + switch (expr->op) { + case ast::BinaryOp::kEqual: + out << "equal"; + break; + case ast::BinaryOp::kNotEqual: + out << "notEqual"; + break; + case ast::BinaryOp::kLessThan: + out << "lessThan"; + break; + case ast::BinaryOp::kGreaterThan: + out << "greaterThan"; + break; + case ast::BinaryOp::kLessThanEqual: + out << "lessThanEqual"; + break; + case ast::BinaryOp::kGreaterThanEqual: + out << "greaterThanEqual"; + break; + default: + break; + } + out << "("; + if (!EmitExpression(out, expr->lhs)) { + return false; + } + out << ", "; + if (!EmitExpression(out, expr->rhs)) { + return false; + } + out << ")"; + return true; +} + bool GeneratorImpl::EmitBinary(std::ostream& out, const ast::BinaryExpression* expr) { + if (IsRelational(expr->op) && !TypeOf(expr->lhs)->UnwrapRef()->is_scalar()) { + return EmitVectorRelational(out, expr); + } if (expr->op == ast::BinaryOp::kLogicalAnd || expr->op == ast::BinaryOp::kLogicalOr) { auto name = UniqueIdentifier(kTempNamePrefix); diff --git a/src/writer/glsl/generator_impl.h b/src/writer/glsl/generator_impl.h index c28154c398..8595e08a41 100644 --- a/src/writer/glsl/generator_impl.h +++ b/src/writer/glsl/generator_impl.h @@ -78,8 +78,14 @@ class GeneratorImpl : public TextGenerator { bool EmitBinary(std::ostream& out, const ast::BinaryExpression* expr); /// Handles generating a bitcast expression /// @param out the output of the expression stream - /// @param expr the as expression - /// @returns true if the bitcast was emitted + /// @param expr the expression + /// @returns true if the binary expression was emitted + bool EmitVectorRelational(std::ostream& out, + const ast::BinaryExpression* expr); + /// Handles generating a vector relational expression + /// @param out the output of the expression stream + /// @param expr the expression + /// @returns true if the vector relational expression was emitted bool EmitBitcast(std::ostream& out, const ast::BitcastExpression* expr); /// Emits a list of statements /// @param stmts the statement list diff --git a/test/bug/dawn/947.wgsl.expected.glsl b/test/bug/dawn/947.wgsl.expected.glsl index d9d9455717..b46d8fc875 100644 --- a/test/bug/dawn/947.wgsl.expected.glsl +++ b/test/bug/dawn/947.wgsl.expected.glsl @@ -1,5 +1,3 @@ -SKIP: FAILED - #version 310 es precision mediump float; @@ -87,7 +85,7 @@ struct tint_symbol_6 { vec4 fs_main_inner(vec2 texcoord) { vec2 clampedTexcoord = clamp(texcoord, vec2(0.0f, 0.0f), vec2(1.0f, 1.0f)); - if (!(all((clampedTexcoord == texcoord)))) { + if (!(all(equal(clampedTexcoord, texcoord)))) { discard; } vec4 srcColor = texture(myTexture, texcoord); @@ -111,10 +109,3 @@ void main() { } -Error parsing GLSL shader: -ERROR: 0:28: 'all' : no matching overloaded function found -ERROR: 0:28: '' : compilation terminated -ERROR: 2 compilation errors. No code generated. - - - diff --git a/test/bug/tint/913.wgsl.expected.glsl b/test/bug/tint/913.wgsl.expected.glsl index 0830bdf9c3..3043ed02ff 100644 --- a/test/bug/tint/913.wgsl.expected.glsl +++ b/test/bug/tint/913.wgsl.expected.glsl @@ -46,7 +46,7 @@ void tint_symbol_1_inner(uvec3 GlobalInvocationID) { if ((tint_tmp)) { bool tint_tmp_3 = success; if (tint_tmp_3) { - tint_tmp_3 = all((texelFetch(dst, ivec3(ivec2(dstTexCoord), 0)) == nonCoveredColor)); + tint_tmp_3 = all(equal(texelFetch(dst, ivec3(ivec2(dstTexCoord), 0)), nonCoveredColor)); } success = (tint_tmp_3); } else { diff --git a/test/bug/tint/942.wgsl.expected.glsl b/test/bug/tint/942.wgsl.expected.glsl index 594731251b..a011f7e5f0 100644 --- a/test/bug/tint/942.wgsl.expected.glsl +++ b/test/bug/tint/942.wgsl.expected.glsl @@ -64,7 +64,7 @@ void tint_symbol_inner(uvec3 WorkGroupID, uvec3 LocalInvocationID, uint local_in } bool tint_tmp = (tint_tmp_1); if (tint_tmp) { - tint_tmp = all((writeIndex < dims)); + tint_tmp = all(lessThan(writeIndex, dims)); } if ((tint_tmp)) { vec3 acc = vec3(0.0f, 0.0f, 0.0f); diff --git a/test/bug/tint/943.spvasm.expected.glsl b/test/bug/tint/943.spvasm.expected.glsl index da855d390b..c79643d8f1 100644 --- a/test/bug/tint/943.spvasm.expected.glsl +++ b/test/bug/tint/943.spvasm.expected.glsl @@ -33,12 +33,12 @@ bool coordsInBounds_vi2_vi2_(inout ivec2 coord, inout ivec2 shape) { bool x_87 = false; bool x_88_phi = false; ivec2 x_76 = coord; - bool x_81 = all((x_76 >= ivec2(0, 0))); + bool x_81 = all(greaterThanEqual(x_76, ivec2(0, 0))); x_88_phi = x_81; if (x_81) { ivec2 x_84 = coord; ivec2 x_85 = shape; - x_87 = all((x_84 < x_85)); + x_87 = all(lessThan(x_84, x_85)); x_88_phi = x_87; } return x_88_phi;