GLSL: special-case BinaryOp::kModulo for floating point.

The '%' operator in GLSL is integer-only. Use the full OpFRem
expression: (a - b * trunc(a / b)).

Bug: tint:1270
Change-Id: I0a969983bef132e004ce456d4a738488e400a61b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68760
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>

src/tint/writer/glsl/generator_impl.cc

@@ -372,6 +372,66 @@ bool GeneratorImpl::EmitBitwiseBoolOp(std::ostream& out,
   return true;
 }
 
+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 {
+    TextBuffer b;
+    TINT_DEFER(helpers_.Append(b));
+
+    auto fn_name = UniqueIdentifier("tint_float_modulo");
+    std::vector<std::string> parameter_names;
+    {
+      auto decl = line(&b);
+      if (!EmitTypeAndName(decl, ret_ty, ast::StorageClass::kNone,
+                           ast::Access::kUndefined, fn_name)) {
+        return "";
+      }
+      {
+        ScopedParen sp(decl);
+        const auto* ty = TypeOf(expr->lhs)->UnwrapRef();
+        if (!EmitTypeAndName(decl, ty, ast::StorageClass::kNone,
+                             ast::Access::kUndefined, "lhs")) {
+          return "";
+        }
+        decl << ", ";
+        ty = TypeOf(expr->rhs)->UnwrapRef();
+        if (!EmitTypeAndName(decl, ty, ast::StorageClass::kNone,
+                             ast::Access::kUndefined, "rhs")) {
+          return "";
+        }
+      }
+      decl << " {";
+    }
+    {
+      ScopedIndent si(&b);
+      line(&b) << "return (lhs - rhs * trunc(lhs / rhs));";
+    }
+    line(&b) << "}";
+    line(&b);
+    return fn_name;
+  });
+
+  if (fn.empty()) {
+    return false;
+  }
+
+  // Call the helper
+  out << fn;
+  {
+    ScopedParen sp(out);
+    if (!EmitExpression(out, expr->lhs)) {
+      return false;
+    }
+    out << ", ";
+    if (!EmitExpression(out, expr->rhs)) {
+      return false;
+    }
+  }
+  return true;
+}
+
 bool GeneratorImpl::EmitBinary(std::ostream& out,
                                const ast::BinaryExpression* expr) {
   if (IsRelational(expr->op) && !TypeOf(expr->lhs)->UnwrapRef()->is_scalar()) {
@@ -416,6 +476,12 @@ bool GeneratorImpl::EmitBinary(std::ostream& out,
     return EmitBitwiseBoolOp(out, expr);
   }
 
+  if (expr->op == ast::BinaryOp::kModulo &&
+      (TypeOf(expr->lhs)->UnwrapRef()->is_float_scalar_or_vector() ||
+       TypeOf(expr->rhs)->UnwrapRef()->is_float_scalar_or_vector())) {
+    return EmitFloatModulo(out, expr);
+  }
+
   out << "(";
   if (!EmitExpression(out, expr->lhs)) {
     return false;

src/tint/writer/glsl/generator_impl.h

@@ -82,6 +82,11 @@ class GeneratorImpl : public TextGenerator {
   /// @param out the output of the expression stream
   /// @param expr the binary expression
   /// @returns true if the expression was emitted, false otherwise
+  bool EmitFloatModulo(std::ostream& out, const ast::BinaryExpression* expr);
+  /// Handles generating the modulo operator on float vector operands
+  /// @param out the output of the expression stream
+  /// @param expr the binary expression
+  /// @returns true if the expression was emitted, false otherwise
   bool EmitBinary(std::ostream& out, const ast::BinaryExpression* expr);
   /// Handles generating a bitcast expression
   /// @param out the output of the expression stream
@@ -503,6 +508,7 @@ class GeneratorImpl : public TextGenerator {
   std::unordered_map<const sem::Struct*, std::string> structure_builders_;
   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_;
   bool requires_oes_sample_variables_ = false;
   bool requires_default_precision_qualifier_ = false;
   Version version_;

src/tint/writer/glsl/generator_impl_binary_test.cc

@@ -40,7 +40,8 @@ TEST_P(GlslBinaryTest, Emit_f32) {
   if (params.op == ast::BinaryOp::kAnd || params.op == ast::BinaryOp::kOr ||
       params.op == ast::BinaryOp::kXor ||
       params.op == ast::BinaryOp::kShiftLeft ||
-      params.op == ast::BinaryOp::kShiftRight) {
+      params.op == ast::BinaryOp::kShiftRight ||
+      params.op == ast::BinaryOp::kModulo) {
     return;
   }
 
@@ -260,6 +261,36 @@ if (tint_tmp) {
 )");
 }
 
+TEST_F(GlslGeneratorImplTest_Binary, ModF32) {
+  Global("a", ty.f32(), ast::StorageClass::kPrivate);
+  Global("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, ModVec3F32) {
+  Global("a", ty.vec3<f32>(), ast::StorageClass::kPrivate);
+  Global("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, Logical_Multi) {
   // (a && b) || (c || d)
   Global("a", ty.bool_(), ast::StorageClass::kPrivate);