Add sem::Expression::HasSideEffects()

Will be used to implement order of execution.

Bug: tint:1300
Change-Id: I027295e482da7a3f9d7ca930b5303e8f89d7fe09
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/79824
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>

src/CMakeLists.txt

@@ -730,6 +730,7 @@ if(TINT_BUILD_TESTS)
     resolver/resolver_test_helper.cc
     resolver/resolver_test_helper.h
     resolver/resolver_test.cc
+    resolver/side_effects_test.cc
     resolver/storage_class_layout_validation_test.cc
     resolver/storage_class_validation_test.cc
     resolver/struct_layout_test.cc

src/program_builder.h

@@ -1540,6 +1540,15 @@ class ProgramBuilder {
                                           Expr(std::forward<EXPR>(expr)));
   }
 
+  /// @param expr the expression to perform a unary not on
+  /// @return an ast::UnaryOpExpression that is the unary not of the input
+  /// expression
+  template <typename EXPR>
+  const ast::UnaryOpExpression* Not(EXPR&& expr) {
+    return create<ast::UnaryOpExpression>(ast::UnaryOp::kNot,
+                                          Expr(std::forward<EXPR>(expr)));
+  }
+
   /// @param source the source information
   /// @param func the function name
   /// @param args the function call arguments

src/resolver/resolver.cc

@@ -1140,7 +1140,7 @@ sem::Expression* Resolver::Expression(const ast::Expression* root) {
     } else if (expr->Is<ast::PhonyExpression>()) {
       sem_expr = builder_->create<sem::Expression>(
           expr, builder_->create<sem::Void>(), current_statement_,
-          sem::Constant{});
+          sem::Constant{}, /* has_side_effects */ false);
     } else {
       TINT_ICE(Resolver, diagnostics_)
           << "unhandled expression type: " << expr->TypeInfo().name;
@@ -1193,8 +1193,9 @@ sem::Expression* Resolver::IndexAccessor(
   }
 
   auto val = EvaluateConstantValue(expr, ty);
-  auto* sem =
+  bool has_side_effects = idx->HasSideEffects() || obj->HasSideEffects();
-      builder_->create<sem::Expression>(expr, ty, current_statement_, val);
+  auto* sem = builder_->create<sem::Expression>(expr, ty, current_statement_,
+                                                val, has_side_effects);
   sem->Behaviors() = idx->Behaviors() + obj->Behaviors();
   return sem;
 }
@@ -1207,8 +1208,8 @@ sem::Expression* Resolver::Bitcast(const ast::BitcastExpression* expr) {
   }
 
   auto val = EvaluateConstantValue(expr, ty);
-  auto* sem =
+  auto* sem = builder_->create<sem::Expression>(expr, ty, current_statement_,
-      builder_->create<sem::Expression>(expr, ty, current_statement_, val);
+                                                val, inner->HasSideEffects());
 
   sem->Behaviors() = inner->Behaviors();
 
@@ -1382,8 +1383,13 @@ sem::Call* Resolver::BuiltinCall(const ast::CallExpression* expr,
     AddWarning("use of deprecated builtin", expr->source);
   }
 
+  bool has_side_effects = builtin->HasSideEffects() ||
+                          std::any_of(args.begin(), args.end(), [](auto* e) {
+                            return e->HasSideEffects();
+                          });
   auto* call = builder_->create<sem::Call>(expr, builtin, std::move(args),
-                                           current_statement_, sem::Constant{});
+                                           current_statement_, sem::Constant{},
+                                           has_side_effects);
 
   current_function_->AddDirectlyCalledBuiltin(builtin);
 
@@ -1427,8 +1433,12 @@ sem::Call* Resolver::FunctionCall(
   auto sym = expr->target.name->symbol;
   auto name = builder_->Symbols().NameFor(sym);
 
+  // TODO(crbug.com/tint/1420): For now, assume all function calls have side
+  // effects.
+  bool has_side_effects = true;
   auto* call = builder_->create<sem::Call>(expr, target, std::move(args),
-                                           current_statement_, sem::Constant{});
+                                           current_statement_, sem::Constant{},
+                                           has_side_effects);
 
   if (current_function_) {
     // Note: Requires called functions to be resolved first.
@@ -1530,9 +1540,10 @@ sem::Call* Resolver::TypeConversion(const ast::CallExpression* expr,
   }
 
   auto val = EvaluateConstantValue(expr, target);
+  bool has_side_effects = arg->HasSideEffects();
   return builder_->create<sem::Call>(expr, call_target,
                                      std::vector<const sem::Expression*>{arg},
-                                     current_statement_, val);
+                                     current_statement_, val, has_side_effects);
 }
 
 sem::Call* Resolver::TypeConstructor(
@@ -1590,8 +1601,10 @@ sem::Call* Resolver::TypeConstructor(
   }
 
   auto val = EvaluateConstantValue(expr, ty);
+  bool has_side_effects = std::any_of(
+      args.begin(), args.end(), [](auto* e) { return e->HasSideEffects(); });
   return builder_->create<sem::Call>(expr, call_target, std::move(args),
-                                     current_statement_, val);
+                                     current_statement_, val, has_side_effects);
 }
 
 sem::Expression* Resolver::Literal(const ast::LiteralExpression* literal) {
@@ -1601,8 +1614,8 @@ sem::Expression* Resolver::Literal(const ast::LiteralExpression* literal) {
   }
 
   auto val = EvaluateConstantValue(literal, ty);
-  return builder_->create<sem::Expression>(literal, ty, current_statement_,
+  return builder_->create<sem::Expression>(literal, ty, current_statement_, val,
-                                           val);
+                                           /* has_side_effects */ false);
 }
 
 sem::Expression* Resolver::Identifier(const ast::IdentifierExpression* expr) {
@@ -1715,8 +1728,12 @@ sem::Expression* Resolver::MemberAccessor(
                                              ref->Access());
     }
 
+    // Structure may be a side-effecting expression (e.g. function call).
+    auto* sem_structure = Sem(expr->structure);
+    bool has_side_effects = sem_structure && sem_structure->HasSideEffects();
+
     return builder_->create<sem::StructMemberAccess>(
-        expr, ret, current_statement_, member);
+        expr, ret, current_statement_, member, has_side_effects);
   }
 
   if (auto* vec = storage_ty->As<sem::Vector>()) {
@@ -1827,8 +1844,9 @@ sem::Expression* Resolver::Binary(const ast::BinaryExpression* expr) {
 
   auto build = [&](const sem::Type* ty) {
     auto val = EvaluateConstantValue(expr, ty);
-    auto* sem =
+    bool has_side_effects = lhs->HasSideEffects() || rhs->HasSideEffects();
-        builder_->create<sem::Expression>(expr, ty, current_statement_, val);
+    auto* sem = builder_->create<sem::Expression>(expr, ty, current_statement_,
+                                                  val, has_side_effects);
     sem->Behaviors() = lhs->Behaviors() + rhs->Behaviors();
     return sem;
   };
@@ -2075,8 +2093,8 @@ sem::Expression* Resolver::UnaryOp(const ast::UnaryOpExpression* unary) {
   }
 
   auto val = EvaluateConstantValue(unary, ty);
-  auto* sem =
+  auto* sem = builder_->create<sem::Expression>(unary, ty, current_statement_,
-      builder_->create<sem::Expression>(unary, ty, current_statement_, val);
+                                                val, expr->HasSideEffects());
   sem->Behaviors() = expr->Behaviors();
   return sem;
 }

src/resolver/side_effects_test.cc

@@ -0,0 +1,371 @@
+// Copyright 2022 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/resolver/resolver.h"
+
+#include "gtest/gtest.h"
+#include "src/resolver/resolver_test_helper.h"
+#include "src/sem/expression.h"
+#include "src/sem/member_accessor_expression.h"
+
+namespace tint::resolver {
+namespace {
+
+struct SideEffectsTest : ResolverTest {
+  template <typename T>
+  void MakeSideEffectFunc(const char* name) {
+    auto global = Sym();
+    Global(global, ty.Of<T>(), ast::StorageClass::kPrivate);
+    auto local = Sym();
+    Func(name, {}, ty.Of<T>(),
+         {
+             Decl(Var(local, ty.Of<T>())),
+             Assign(global, local),
+             Return(global),
+         });
+  }
+
+  template <typename MAKE_TYPE_FUNC>
+  void MakeSideEffectFunc(const char* name, MAKE_TYPE_FUNC make_type) {
+    auto global = Sym();
+    Global(global, make_type(), ast::StorageClass::kPrivate);
+    auto local = Sym();
+    Func(name, {}, make_type(),
+         {
+             Decl(Var(local, make_type())),
+             Assign(global, local),
+             Return(global),
+         });
+  }
+};
+
+TEST_F(SideEffectsTest, Phony) {
+  auto* expr = Phony();
+  auto* body = Assign(expr, 1);
+  WrapInFunction(body);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Literal) {
+  auto* expr = Expr(1);
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, VariableUser) {
+  auto* var = Decl(Var("a", ty.i32()));
+  auto* expr = Expr("a");
+  WrapInFunction(var, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->Is<sem::VariableUser>());
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Call_Builtin_NoSE) {
+  Global("a", ty.f32(), ast::StorageClass::kPrivate);
+  auto* expr = Call("dpdx", "a");
+  Func("f", {}, ty.void_(), {Ignore(expr)},
+       {create<ast::StageAttribute>(ast::PipelineStage::kFragment)});
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->Is<sem::Call>());
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Call_Builtin_NoSE_WithSEArg) {
+  MakeSideEffectFunc<f32>("se");
+  auto* expr = Call("dpdx", Call("se"));
+  Func("f", {}, ty.void_(), {Ignore(expr)},
+       {create<ast::StageAttribute>(ast::PipelineStage::kFragment)});
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->Is<sem::Call>());
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Call_Builtin_SE) {
+  Global("a", ty.atomic(ty.i32()), ast::StorageClass::kWorkgroup);
+  auto* expr = Call("atomicAdd", AddressOf("a"), 1);
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->Is<sem::Call>());
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Call_Function) {
+  Func("f", {}, ty.i32(), {Return(1)});
+  auto* expr = Call("f");
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->Is<sem::Call>());
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Call_TypeConversion_NoSE) {
+  auto* var = Decl(Var("a", ty.i32()));
+  auto* expr = Construct(ty.f32(), "a");
+  WrapInFunction(var, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->Is<sem::Call>());
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Call_TypeConversion_SE) {
+  MakeSideEffectFunc<i32>("se");
+  auto* expr = Construct(ty.f32(), Call("se"));
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->Is<sem::Call>());
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Call_TypeConstructor_NoSE) {
+  auto* var = Decl(Var("a", ty.f32()));
+  auto* expr = Construct(ty.f32(), "a");
+  WrapInFunction(var, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->Is<sem::Call>());
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Call_TypeConstructor_SE) {
+  MakeSideEffectFunc<f32>("se");
+  auto* expr = Construct(ty.f32(), Call("se"));
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->Is<sem::Call>());
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, MemberAccessor_Struct_NoSE) {
+  auto* s = Structure("S", {Member("m", ty.i32())});
+  auto* var = Decl(Var("a", ty.Of(s)));
+  auto* expr = MemberAccessor("a", "m");
+  WrapInFunction(var, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, MemberAccessor_Struct_SE) {
+  auto* s = Structure("S", {Member("m", ty.i32())});
+  MakeSideEffectFunc("se", [&] { return ty.Of(s); });
+  auto* expr = MemberAccessor(Call("se"), "m");
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, MemberAccessor_Vector) {
+  auto* var = Decl(Var("a", ty.vec4<f32>()));
+  auto* expr = MemberAccessor("a", "x");
+  WrapInFunction(var, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  EXPECT_TRUE(sem->Is<sem::MemberAccessorExpression>());
+  ASSERT_NE(sem, nullptr);
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, MemberAccessor_VectorSwizzle) {
+  auto* var = Decl(Var("a", ty.vec4<f32>()));
+  auto* expr = MemberAccessor("a", "xzyw");
+  WrapInFunction(var, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  EXPECT_TRUE(sem->Is<sem::Swizzle>());
+  ASSERT_NE(sem, nullptr);
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Binary_NoSE) {
+  auto* a = Decl(Var("a", ty.i32()));
+  auto* b = Decl(Var("b", ty.i32()));
+  auto* expr = Add("a", "b");
+  WrapInFunction(a, b, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Binary_LeftSE) {
+  MakeSideEffectFunc<i32>("se");
+  auto* b = Decl(Var("b", ty.i32()));
+  auto* expr = Add(Call("se"), "b");
+  WrapInFunction(b, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Binary_RightSE) {
+  MakeSideEffectFunc<i32>("se");
+  auto* a = Decl(Var("a", ty.i32()));
+  auto* expr = Add("a", Call("se"));
+  WrapInFunction(a, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Binary_BothSE) {
+  MakeSideEffectFunc<i32>("se1");
+  MakeSideEffectFunc<i32>("se2");
+  auto* expr = Add(Call("se1"), Call("se2"));
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Unary_NoSE) {
+  auto* var = Decl(Var("a", ty.bool_()));
+  auto* expr = Not("a");
+  WrapInFunction(var, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Unary_SE) {
+  MakeSideEffectFunc<bool>("se");
+  auto* expr = Not(Call("se"));
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, IndexAccessor_NoSE) {
+  auto* var = Decl(Var("a", ty.array<i32, 10>()));
+  auto* expr = IndexAccessor("a", 0);
+  WrapInFunction(var, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, IndexAccessor_ObjSE) {
+  MakeSideEffectFunc("se", [&] { return ty.array<i32, 10>(); });
+  auto* expr = IndexAccessor(Call("se"), 0);
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, IndexAccessor_IndexSE) {
+  MakeSideEffectFunc<i32>("se");
+  auto* var = Decl(Var("a", ty.array<i32, 10>()));
+  auto* expr = IndexAccessor("a", Call("se"));
+  WrapInFunction(var, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, IndexAccessor_BothSE) {
+  MakeSideEffectFunc("se1", [&] { return ty.array<i32, 10>(); });
+  MakeSideEffectFunc<i32>("se2");
+  auto* expr = IndexAccessor(Call("se1"), Call("se2"));
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Bitcast_NoSE) {
+  auto* var = Decl(Var("a", ty.i32()));
+  auto* expr = Bitcast<f32>("a");
+  WrapInFunction(var, expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_FALSE(sem->HasSideEffects());
+}
+
+TEST_F(SideEffectsTest, Bitcast_SE) {
+  MakeSideEffectFunc<i32>("se");
+  auto* expr = Bitcast<f32>(Call("se"));
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+  auto* sem = Sem().Get(expr);
+  ASSERT_NE(sem, nullptr);
+  EXPECT_TRUE(sem->HasSideEffects());
+}
+
+}  // namespace
+}  // namespace tint::resolver

src/sem/builtin.cc

@@ -160,6 +160,16 @@ bool Builtin::IsAtomic() const {
   return IsAtomicBuiltin(type_);
 }
 
+bool Builtin::HasSideEffects() const {
+  if (IsAtomic() && type_ != sem::BuiltinType::kAtomicLoad) {
+    return true;
+  }
+  if (type_ == sem::BuiltinType::kTextureStore) {
+    return true;
+  }
+  return false;
+}
+
 }  // namespace sem
 }  // namespace tint
 

src/sem/builtin.h

@@ -139,6 +139,10 @@ class Builtin : public Castable<Builtin, CallTarget> {
   /// @returns true if builtin is a atomic builtin
   bool IsAtomic() const;
 
+  /// @returns true if intrinsic may have side-effects (i.e. writes to at least
+  /// one of its inputs)
+  bool HasSideEffects() const;
+
  private:
   const BuiltinType type_;
   const PipelineStageSet supported_stages_;

src/sem/call.cc

@@ -26,8 +26,13 @@ Call::Call(const ast::CallExpression* declaration,
            const CallTarget* target,
            std::vector<const sem::Expression*> arguments,
            const Statement* statement,
-           Constant constant)
+           Constant constant,
-    : Base(declaration, target->ReturnType(), statement, std::move(constant)),
+           bool has_side_effects)
+    : Base(declaration,
+           target->ReturnType(),
+           statement,
+           std::move(constant),
+           has_side_effects),
       target_(target),
       arguments_(std::move(arguments)) {}
 

src/sem/call.h

@@ -33,11 +33,13 @@ class Call : public Castable<Call, Expression> {
   /// @param arguments the call arguments
   /// @param statement the statement that owns this expression
   /// @param constant the constant value of this expression
+  /// @param has_side_effects whether this expression may have side effects
   Call(const ast::CallExpression* declaration,
        const CallTarget* target,
        std::vector<const sem::Expression*> arguments,
        const Statement* statement,
-       Constant constant);
+       Constant constant,
+       bool has_side_effects);
 
   /// Destructor
   ~Call() override;

src/sem/expression.cc

@@ -24,11 +24,13 @@ namespace sem {
 Expression::Expression(const ast::Expression* declaration,
                        const sem::Type* type,
                        const Statement* statement,
-                       Constant constant)
+                       Constant constant,
+                       bool has_side_effects)
     : declaration_(declaration),
       type_(type),
       statement_(statement),
-      constant_(std::move(constant)) {
+      constant_(std::move(constant)),
+      has_side_effects_(has_side_effects) {
   TINT_ASSERT(Semantic, type_);
 }
 

src/sem/expression.h

@@ -34,10 +34,12 @@ class Expression : public Castable<Expression, Node> {
   /// @param type the resolved type of the expression
   /// @param statement the statement that owns this expression
   /// @param constant the constant value of the expression. May be invalid
+  /// @param has_side_effects true if this expression may have side-effects
   Expression(const ast::Expression* declaration,
              const sem::Type* type,
              const Statement* statement,
-             Constant constant);
+             Constant constant,
+             bool has_side_effects);
 
   /// Destructor
   ~Expression() override;
@@ -60,6 +62,9 @@ class Expression : public Castable<Expression, Node> {
   /// @return the behaviors of this statement
   sem::Behaviors& Behaviors() { return behaviors_; }
 
+  /// @return true of this expression may have side effects
+  bool HasSideEffects() const { return has_side_effects_; }
+
  protected:
   /// The AST expression node for this semantic expression
   const ast::Expression* const declaration_;
@@ -69,6 +74,7 @@ class Expression : public Castable<Expression, Node> {
   const Statement* const statement_;
   const Constant constant_;
   sem::Behaviors behaviors_{sem::Behavior::kNext};
+  const bool has_side_effects_;
 };
 
 }  // namespace sem

src/sem/member_accessor_expression.cc

@@ -27,8 +27,9 @@ namespace sem {
 MemberAccessorExpression::MemberAccessorExpression(
     const ast::MemberAccessorExpression* declaration,
     const sem::Type* type,
-    const Statement* statement)
+    const Statement* statement,
-    : Base(declaration, type, statement, Constant{}) {}
+    bool has_side_effects)
+    : Base(declaration, type, statement, Constant{}, has_side_effects) {}
 
 MemberAccessorExpression::~MemberAccessorExpression() = default;
 
@@ -36,8 +37,9 @@ StructMemberAccess::StructMemberAccess(
     const ast::MemberAccessorExpression* declaration,
     const sem::Type* type,
     const Statement* statement,
-    const StructMember* member)
+    const StructMember* member,
-    : Base(declaration, type, statement), member_(member) {}
+    bool has_side_effects)
+    : Base(declaration, type, statement, has_side_effects), member_(member) {}
 
 StructMemberAccess::~StructMemberAccess() = default;
 
@@ -45,7 +47,8 @@ Swizzle::Swizzle(const ast::MemberAccessorExpression* declaration,
                  const sem::Type* type,
                  const Statement* statement,
                  std::vector<uint32_t> indices)
-    : Base(declaration, type, statement), indices_(std::move(indices)) {}
+    : Base(declaration, type, statement, /* has_side_effects */ false),
+      indices_(std::move(indices)) {}
 
 Swizzle::~Swizzle() = default;
 

src/sem/member_accessor_expression.h

@@ -41,9 +41,11 @@ class MemberAccessorExpression
   /// @param declaration the AST node
   /// @param type the resolved type of the expression
   /// @param statement the statement that owns this expression
+  /// @param has_side_effects whether this expression may have side effects
   MemberAccessorExpression(const ast::MemberAccessorExpression* declaration,
                            const sem::Type* type,
-                           const Statement* statement);
+                           const Statement* statement,
+                           bool has_side_effects);
 
   /// Destructor
   ~MemberAccessorExpression() override;
@@ -60,10 +62,12 @@ class StructMemberAccess
   /// @param type the resolved type of the expression
   /// @param statement the statement that owns this expression
   /// @param member the structure member
+  /// @param has_side_effects whether this expression may have side effects
   StructMemberAccess(const ast::MemberAccessorExpression* declaration,
                      const sem::Type* type,
                      const Statement* statement,
-                     const StructMember* member);
+                     const StructMember* member,
+                     bool has_side_effects);
 
   /// Destructor
   ~StructMemberAccess() override;

src/sem/variable.cc

@@ -78,7 +78,11 @@ Parameter::~Parameter() = default;
 VariableUser::VariableUser(const ast::IdentifierExpression* declaration,
                            Statement* statement,
                            sem::Variable* variable)
-    : Base(declaration, variable->Type(), statement, variable->ConstantValue()),
+    : Base(declaration,
+           variable->Type(),
+           statement,
+           variable->ConstantValue(),
+           /* has_side_effects */ false),
       variable_(variable) {}
 
 }  // namespace sem

src/writer/append_vector.cc

@@ -61,7 +61,8 @@ const sem::Expression* Zero(ProgramBuilder& b,
         << "unsupported vector element type: " << ty->TypeInfo().name;
     return nullptr;
   }
-  auto* sem = b.create<sem::Expression>(expr, ty, stmt, sem::Constant{});
+  auto* sem = b.create<sem::Expression>(expr, ty, stmt, sem::Constant{},
+                                        /* has_side_effects */ false);
   b.Sem().Add(expr, sem);
   return sem;
 }
@@ -140,10 +141,10 @@ const sem::Call* AppendVector(ProgramBuilder* b,
         b->create<sem::Parameter>(nullptr, 0, scalar_sem->Type()->UnwrapRef(),
                                   ast::StorageClass::kNone,
                                   ast::Access::kUndefined));
-    auto* scalar_cast_sem =
+    auto* scalar_cast_sem = b->create<sem::Call>(
-        b->create<sem::Call>(scalar_cast_ast, scalar_cast_target,
+        scalar_cast_ast, scalar_cast_target,
-                             std::vector<const sem::Expression*>{scalar_sem},
+        std::vector<const sem::Expression*>{scalar_sem}, statement,
-                             statement, sem::Constant{});
+        sem::Constant{}, /* has_side_effects */ false);
     b->Sem().Add(scalar_cast_ast, scalar_cast_sem);
     packed.emplace_back(scalar_cast_sem);
   } else {
@@ -165,7 +166,8 @@ const sem::Call* AppendVector(ProgramBuilder* b,
                                             ast::Access::kUndefined);
                                       }));
   auto* constructor_sem = b->create<sem::Call>(
-      constructor_ast, constructor_target, packed, statement, sem::Constant{});
+      constructor_ast, constructor_target, packed, statement, sem::Constant{},
+      /* has_side_effects */ false);
   b->Sem().Add(constructor_ast, constructor_sem);
   return constructor_sem;
 }

src/writer/glsl/generator_impl.cc

@@ -1336,8 +1336,8 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& out,
       auto* f32 = builder_.create<sem::F32>();
       auto* zero = builder_.Expr(0.0f);
       auto* stmt = builder_.Sem().Get(param_coords)->Stmt();
-      auto* sem_zero =
+      auto* sem_zero = builder_.create<sem::Expression>(
-          builder_.create<sem::Expression>(zero, f32, stmt, sem::Constant{});
+          zero, f32, stmt, sem::Constant{}, /* has_side_effects */ false);
       builder_.Sem().Add(zero, sem_zero);
       param_coords = AppendVector(&builder_, param_coords, zero)->Declaration();
     }

src/writer/hlsl/generator_impl.cc

@@ -2405,8 +2405,9 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& out,
     auto* i32 = builder_.create<sem::I32>();
     auto* zero = builder_.Expr(0);
     auto* stmt = builder_.Sem().Get(vector)->Stmt();
-    builder_.Sem().Add(zero, builder_.create<sem::Expression>(zero, i32, stmt,
+    builder_.Sem().Add(
-                                                              sem::Constant{}));
+        zero, builder_.create<sem::Expression>(zero, i32, stmt, sem::Constant{},
+                                               /* has_side_effects */ false));
     auto* packed = AppendVector(&builder_, vector, zero);
     return EmitExpression(out, packed->Declaration());
   };

src/writer/msl/generator_impl_constructor_test.cc

@@ -181,7 +181,7 @@ TEST_F(MslGeneratorImplTest, EmitConstructor_Type_Struct_Empty) {
 
   ASSERT_TRUE(gen.Generate()) << gen.error();
   EXPECT_THAT(gen.result(), HasSubstr("{}"));
-  EXPECT_THAT(gen.result(), Not(HasSubstr("{{}}")));
+  EXPECT_THAT(gen.result(), testing::Not(HasSubstr("{{}}")));
 }
 
 }  // namespace

test/BUILD.gn

@@ -260,6 +260,7 @@ tint_unittests_source_set("tint_unittests_resolver_src") {
     "../src/resolver/resolver_test.cc",
     "../src/resolver/resolver_test_helper.cc",
     "../src/resolver/resolver_test_helper.h",
+    "../src/resolver/side_effects_test.cc",
     "../src/resolver/storage_class_layout_validation_test.cc",
     "../src/resolver/storage_class_validation_test.cc",
     "../src/resolver/struct_layout_test.cc",