commit db5ad9f357b5be9e6a89e57b10a82dc9e0e60d2b
author James Price <jrprice@google.com> Wed May 17 14:49:26 2023 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed May 17 14:49:26 2023 +0000
tree 3bfada2edabfff7815113d5561b0062ecff42df5
parent 11ee6b6cc6775ff54bbef5bac2d20c2e75b83fe1

[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

diff --git a/src/tint/resolver/materialize_test.cc b/src/tint/resolver/materialize_test.cc
index 696f279..cf67c93 100644
--- a/src/tint/resolver/materialize_test.cc
+++ b/src/tint/resolver/materialize_test.cc
@@ -211,6 +211,10 @@
 
     // 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 @@
             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 @@
                  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 @@
                     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 @@
 /// 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

src/tint/resolver/resolver.cc

diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc
index a65d825..09d8464 100644
--- a/src/tint/resolver/resolver.cc
+++ b/src/tint/resolver/resolver.cc
@@ -4479,7 +4479,7 @@
             return false;
         }
 
-        auto* rhs = Load(ValueExpression(stmt->rhs));
+        const auto* rhs = ValueExpression(stmt->rhs);
         if (!rhs) {
             return false;
         }
@@ -4491,12 +4491,19 @@
         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);
     });
 }