tint/writer/glsl: Fix emitting float modulo with mixing type

This patch fix the GLSL writer issue that emit only one helper function
when using both `v % s`, `s % v` and `v % vs in the shader, where `s` is
of `f32` and `v` is a vector of `f32`. Unittests are added for GLSL.

Bug: tint:1614
Change-Id: Ia89ae010341b9c88b8101cc6febab7d83c96bb17
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/96085
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>

src/tint/writer/glsl/generator_impl.cc

@@ -500,7 +500,9 @@ bool GeneratorImpl::EmitBitwiseBoolOp(std::ostream& out, const ast::BinaryExpres
 bool GeneratorImpl::EmitFloatModulo(std::ostream& out, const ast::BinaryExpression* expr) {
     std::string fn;
     auto* ret_ty = TypeOf(expr)->UnwrapRef();
-    fn = utils::GetOrCreate(float_modulo_funcs_, ret_ty, [&]() -> std::string {
+    auto* lhs_ty = TypeOf(expr->lhs)->UnwrapRef();
+    auto* rhs_ty = TypeOf(expr->rhs)->UnwrapRef();
+    fn = utils::GetOrCreate(float_modulo_funcs_, {lhs_ty, rhs_ty}, [&]() -> std::string {
         TextBuffer b;
         TINT_DEFER(helpers_.Append(b));
 

src/tint/writer/glsl/generator_impl.h

@@ -492,6 +492,23 @@ class GeneratorImpl : public TextGenerator {
         };
     };
 
+    /// The structure holding both type of two operands for a binary operator.
+    struct BinaryOperandType {
+        const sem::Type* lhs_type;
+        const sem::Type* rhs_type;
+        bool operator==(const BinaryOperandType& rhs) const {
+            return lhs_type == rhs.lhs_type && rhs_type == rhs.rhs_type;
+        }
+        /// Hasher is a std::hash function for BinaryOperandType
+        struct Hasher {
+            /// @param i the BinaryOperandType to hash
+            /// @returns the hash of `i`
+            inline std::size_t operator()(const BinaryOperandType& i) const {
+                return utils::Hash(i.lhs_type, i.rhs_type);
+            }
+        };
+    };
+
     /// CallBuiltinHelper will call the builtin helper function, creating it
     /// if it hasn't been built already. If the builtin needs to be built then
     /// CallBuiltinHelper will generate the function signature and will call
@@ -522,7 +539,8 @@ class GeneratorImpl : public TextGenerator {
     std::unordered_map<const sem::Builtin*, std::string> builtins_;
     std::unordered_map<const sem::Vector*, std::string> dynamic_vector_write_;
     std::unordered_map<const sem::Vector*, std::string> int_dot_funcs_;
-    std::unordered_map<const sem::Type*, std::string> float_modulo_funcs_;
+    std::unordered_map<BinaryOperandType, std::string, BinaryOperandType::Hasher>
+        float_modulo_funcs_;
     std::unordered_set<const sem::Struct*> emitted_structs_;
     bool requires_oes_sample_variables_ = false;
     bool requires_default_precision_qualifier_ = false;

src/tint/writer/glsl/generator_impl_binary_test.cc

@@ -273,6 +273,76 @@ TEST_F(GlslGeneratorImplTest_Binary, ModVec3F32) {
     EXPECT_EQ(out.str(), "tint_float_modulo(a, b)");
 }
 
+TEST_F(GlslGeneratorImplTest_Binary, ModVec3F32ScalarF32) {
+    GlobalVar("a", ty.vec3<f32>(), ast::StorageClass::kPrivate);
+    GlobalVar("b", ty.f32(), ast::StorageClass::kPrivate);
+
+    auto* expr = create<ast::BinaryExpression>(ast::BinaryOp::kModulo, Expr("a"), Expr("b"));
+    WrapInFunction(expr);
+
+    GeneratorImpl& gen = Build();
+
+    std::stringstream out;
+    ASSERT_TRUE(gen.EmitExpression(out, expr)) << gen.error();
+    EXPECT_EQ(out.str(), "tint_float_modulo(a, b)");
+}
+
+TEST_F(GlslGeneratorImplTest_Binary, ModScalarF32Vec3F32) {
+    GlobalVar("a", ty.f32(), ast::StorageClass::kPrivate);
+    GlobalVar("b", ty.vec3<f32>(), ast::StorageClass::kPrivate);
+
+    auto* expr = create<ast::BinaryExpression>(ast::BinaryOp::kModulo, Expr("a"), Expr("b"));
+    WrapInFunction(expr);
+
+    GeneratorImpl& gen = Build();
+
+    std::stringstream out;
+    ASSERT_TRUE(gen.EmitExpression(out, expr)) << gen.error();
+    EXPECT_EQ(out.str(), "tint_float_modulo(a, b)");
+}
+
+TEST_F(GlslGeneratorImplTest_Binary, ModMixedVec3ScalarF32) {
+    GlobalVar("a", ty.vec3<f32>(), ast::StorageClass::kPrivate);
+    GlobalVar("b", ty.f32(), ast::StorageClass::kPrivate);
+
+    auto* expr_vec_mod_vec =
+        create<ast::BinaryExpression>(ast::BinaryOp::kModulo, Expr("a"), Expr("a"));
+    auto* expr_vec_mod_scalar =
+        create<ast::BinaryExpression>(ast::BinaryOp::kModulo, Expr("a"), Expr("b"));
+    auto* expr_scalar_mod_vec =
+        create<ast::BinaryExpression>(ast::BinaryOp::kModulo, Expr("b"), Expr("a"));
+    WrapInFunction(expr_vec_mod_vec, expr_vec_mod_scalar, expr_scalar_mod_vec);
+
+    GeneratorImpl& gen = Build();
+
+    ASSERT_TRUE(gen.Generate()) << gen.error();
+    EXPECT_EQ(gen.result(), R"(#version 310 es
+
+vec3 tint_float_modulo(vec3 lhs, vec3 rhs) {
+  return (lhs - rhs * trunc(lhs / rhs));
+}
+
+vec3 tint_float_modulo_1(vec3 lhs, float rhs) {
+  return (lhs - rhs * trunc(lhs / rhs));
+}
+
+vec3 tint_float_modulo_2(float lhs, vec3 rhs) {
+  return (lhs - rhs * trunc(lhs / rhs));
+}
+
+
+vec3 a = vec3(0.0f, 0.0f, 0.0f);
+float b = 0.0f;
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+void test_function() {
+  vec3 tint_symbol = tint_float_modulo(a, a);
+  vec3 tint_symbol_1 = tint_float_modulo_1(a, b);
+  vec3 tint_symbol_2 = tint_float_modulo_2(b, a);
+  return;
+}
+)");
+}
+
 TEST_F(GlslGeneratorImplTest_Binary, Logical_Multi) {
     // (a && b) || (c || d)
     GlobalVar("a", ty.bool_(), ast::StorageClass::kPrivate);