commit 11ee6b6cc6775ff54bbef5bac2d20c2e75b83fe1
author dan sinclair <dsinclair@chromium.org> Wed May 17 13:52:41 2023 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed May 17 13:52:41 2023 +0000
tree da541d0af08dfeecbe47e151952cf16a468517c0
parent bbaa456b18e052ddcc6da7b061cbbad046673196

[ir] Handle phony assignment.

This CL adds phony assignment to the IR. The assignment part is ignored
and the RHS of the expression is generated. This creates a result value
which is then never used.

Bug: tint:1918
Change-Id: Ic87fdcb387cb4d9783c4dbbe26ebc76f67a3cdd9
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/133260
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/tint/ir/from_program.cc

diff --git a/src/tint/ir/from_program.cc b/src/tint/ir/from_program.cc
index 76c1519..d42ea0b 100644
--- a/src/tint/ir/from_program.cc
+++ b/src/tint/ir/from_program.cc
@@ -48,6 +48,7 @@
 #include "src/tint/ast/literal_expression.h"
 #include "src/tint/ast/loop_statement.h"
 #include "src/tint/ast/override.h"
+#include "src/tint/ast/phony_expression.h"
 #include "src/tint/ast/return_statement.h"
 #include "src/tint/ast/statement.h"
 #include "src/tint/ast/struct.h"
@@ -338,7 +339,6 @@
             current_flow_block_ = ir_func->start_target;
             EmitBlock(ast_func->body);
 
-            // TODO(dsinclair): Store return type and attributes
             // TODO(dsinclair): Store parameters
 
             // If the branch target has already been set then a `return` was called. Only set in the
@@ -392,6 +392,16 @@
     }
 
     void EmitAssignment(const ast::AssignmentStatement* stmt) {
+        // If assigning to a phony, just generate the RHS and we're done. Note that, because this
+        // isn't used, a subsequent transform could remove it due to it being dead code. This could
+        // then change the interface for the program (i.e. a global var no longer used). If that
+        // happens we have to either fix this to store to a phony value, or make sure we pull the
+        // interface before doing the dead code elimination.
+        if (stmt->lhs->Is<ast::PhonyExpression>()) {
+            (void)EmitExpression(stmt->rhs);
+            return;
+        }
+
         auto lhs = EmitExpression(stmt->lhs);
         if (!lhs) {
             return;
@@ -827,10 +837,9 @@
             // [&](const ast::MemberAccessorExpression* m) {
             // TODO(dsinclair): Implement
             // },
-            // [&](const ast::PhonyExpression*) {
-            // TODO(dsinclair): Implement. The call may have side effects so has to be made.
-            // },
             [&](const ast::UnaryOpExpression* u) { return EmitUnary(u); },
+            // Note, ast::PhonyExpression is explicitly not handled here as it should never get into
+            // this method. The assignment statement should have filtered it out already.
             [&](Default) {
                 add_error(expr->source,
                           "unknown expression type: " + std::string(expr->TypeInfo().name));

src/tint/ir/from_program_test.cc

diff --git a/src/tint/ir/from_program_test.cc b/src/tint/ir/from_program_test.cc
index 234b6cc..47dc911 100644
--- a/src/tint/ir/from_program_test.cc
+++ b/src/tint/ir/from_program_test.cc
@@ -1427,5 +1427,27 @@
 )");
 }
 
+TEST_F(IR_BuilderImplTest, Emit_Phony) {
+    Func("b", utils::Empty, ty.i32(), Return(1_i));
+    WrapInFunction(Ignore(Call("b")));
+
+    auto m = Build();
+    ASSERT_TRUE(m) << (!m ? m.Failure() : "");
+
+    EXPECT_EQ(Disassemble(m.Get()),
+              R"(%fn1 = func b():i32 {
+  %fn2 = block {
+  } -> %func_end 1i # return
+} %func_end
+
+%fn3 = func test_function():void [@compute @workgroup_size(1, 1, 1)] {
+  %fn4 = block {
+    %1:i32 = call b
+  } -> %func_end # return
+} %func_end
+
+)");
+}
+
 }  // namespace
 }  // namespace tint::ir