From db5ad9f357b5be9e6a89e57b10a82dc9e0e60d2b Mon Sep 17 00:00:00 2001
From: James Price <jrprice@google.com>
Date: Wed, 17 May 2023 14:49:26 +0000
Subject: [PATCH] [tint] Materialize compound assignment RHS

The RHS of a compound assignment statement may need to be
materialized. This was showing up when converting things like `i += 1`
to IR, as abstract types were creeping into the IR.

Change-Id: Idf9b1523d1751e26c28a795af07769ca85a65f14
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/133221
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
---
 src/tint/resolver/materialize_test.cc | 21 ++++++++++++++++++---
 src/tint/resolver/resolver.cc         | 17 ++++++++++++-----
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/src/tint/resolver/materialize_test.cc b/src/tint/resolver/materialize_test.cc
index 696f2799d3..cf67c93585 100644
--- a/src/tint/resolver/materialize_test.cc
+++ b/src/tint/resolver/materialize_test.cc
@@ -211,6 +211,10 @@ enum class Method {
 
     // abstract_expr[runtime-index]
     kRuntimeIndex,
+
+    // var a : target_type;
+    // a += abstract_expr;
+    kCompoundAssign,
 };
 
 static std::ostream& operator<<(std::ostream& o, Method m) {
@@ -247,6 +251,8 @@ static std::ostream& operator<<(std::ostream& o, Method m) {
             return o << "workgroup-size";
         case Method::kRuntimeIndex:
             return o << "runtime-index";
+        case Method::kCompoundAssign:
+            return o << "compound-assign";
     }
     return o << "<unknown>";
 }
@@ -387,10 +393,15 @@ TEST_P(MaterializeAbstractNumericToConcreteType, Test) {
                  utils::Vector{WorkgroupSize(target_expr(), abstract_expr, Expr(123_a)),
                                Stage(ast::PipelineStage::kCompute)});
             break;
-        case Method::kRuntimeIndex:
+        case Method::kRuntimeIndex: {
             auto* runtime_index = Var("runtime_index", Expr(1_i));
             WrapInFunction(runtime_index, IndexAccessor(abstract_expr, runtime_index));
             break;
+        }
+        case Method::kCompoundAssign:
+            WrapInFunction(Decl(Var("a", target_ty())),
+                           CompoundAssign("a", abstract_expr, ast::BinaryOp::kAdd));
+            break;
     }
 
     switch (expectation) {
@@ -421,6 +432,10 @@ TEST_P(MaterializeAbstractNumericToConcreteType, Test) {
                     expect = "error: no matching overload for operator + (" +
                              data.target_type_name + ", " + data.abstract_type_name + ")";
                     break;
+                case Method::kCompoundAssign:
+                    expect = "error: no matching overload for operator += (" +
+                             data.target_type_name + ", " + data.abstract_type_name + ")";
+                    break;
                 default:
                     expect = "error: cannot convert value of type '" + data.abstract_type_name +
                              "' to type '" + data.target_type_name + "'";
@@ -440,13 +455,13 @@ TEST_P(MaterializeAbstractNumericToConcreteType, Test) {
 /// Methods that support scalar materialization
 constexpr Method kScalarMethods[] = {
     Method::kLet,    Method::kVar,   Method::kAssign, Method::kFnArg,    Method::kBuiltinArg,
-    Method::kReturn, Method::kArray, Method::kStruct, Method::kBinaryOp,
+    Method::kReturn, Method::kArray, Method::kStruct, Method::kBinaryOp, Method::kCompoundAssign,
 };
 
 /// Methods that support vector materialization
 constexpr Method kVectorMethods[] = {
     Method::kLet,    Method::kVar,   Method::kAssign, Method::kFnArg,    Method::kBuiltinArg,
-    Method::kReturn, Method::kArray, Method::kStruct, Method::kBinaryOp,
+    Method::kReturn, Method::kArray, Method::kStruct, Method::kBinaryOp, Method::kCompoundAssign,
 };
 
 /// Methods that support matrix materialization
diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc
index a65d8258d4..09d8464ffe 100644
--- a/src/tint/resolver/resolver.cc
+++ b/src/tint/resolver/resolver.cc
@@ -4479,7 +4479,7 @@ sem::Statement* Resolver::CompoundAssignmentStatement(
             return false;
         }
 
-        auto* rhs = Load(ValueExpression(stmt->rhs));
+        const auto* rhs = ValueExpression(stmt->rhs);
         if (!rhs) {
             return false;
         }
@@ -4491,12 +4491,19 @@ sem::Statement* Resolver::CompoundAssignmentStatement(
         auto* lhs_ty = lhs->Type()->UnwrapRef();
         auto* rhs_ty = rhs->Type()->UnwrapRef();
         auto stage = sem::EarliestStage(lhs->Stage(), rhs->Stage());
-        auto* ty =
-            intrinsic_table_->Lookup(stmt->op, lhs_ty, rhs_ty, stage, stmt->source, true).result;
-        if (!ty) {
+
+        auto op = intrinsic_table_->Lookup(stmt->op, lhs_ty, rhs_ty, stage, stmt->source, true);
+        if (!op.result) {
             return false;
         }
-        return validator_.Assignment(stmt, ty);
+
+        // Load or materialize the RHS if necessary.
+        rhs = Load(Materialize(rhs, op.rhs));
+        if (!rhs) {
+            return false;
+        }
+
+        return validator_.Assignment(stmt, op.result);
     });
 }