From 30747f607dd9b23a0da8fe010fa04e2a255a0b72 Mon Sep 17 00:00:00 2001 From: Antonio Maiorano Date: Tue, 15 Mar 2022 13:36:53 +0000 Subject: [PATCH] HoistToDeclBefore: revert hoisting of references The goal of this utility is to hoist copies of expressions to ensure order of evaluation of expressions. Hoisting references makes no difference. Bug: tint:1300 Change-Id: I3e7c2e53c9618aeb06836604e39383de016b072c Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/81040 Reviewed-by: Ben Clayton Kokoro: Kokoro Commit-Queue: Antonio Maiorano --- .../transform/utils/hoist_to_decl_before.cc | 23 ++++--------------- .../utils/hoist_to_decl_before_test.cc | 8 +++---- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/src/tint/transform/utils/hoist_to_decl_before.cc b/src/tint/transform/utils/hoist_to_decl_before.cc index 3f66150f29..508b9745fa 100644 --- a/src/tint/transform/utils/hoist_to_decl_before.cc +++ b/src/tint/transform/utils/hoist_to_decl_before.cc @@ -266,23 +266,12 @@ class HoistToDeclBefore::State { bool HoistToDeclBefore(const sem::Expression* before_expr, const ast::Expression* expr, bool as_const, - const char* decl_name = "") { + const char* decl_name) { auto name = b.Symbols().New(decl_name); - auto* sem_expr = ctx.src->Sem().Get(expr); - bool is_ref = - sem_expr && - !sem_expr->Is() // Don't need to take a ref to a var - && sem_expr->Type()->Is(); - - auto* expr_clone = ctx.Clone(expr); - if (is_ref) { - expr_clone = b.AddressOf(expr_clone); - } - // Construct the let/var that holds the hoisted expr - auto* v = as_const ? b.Const(name, nullptr, expr_clone) - : b.Var(name, nullptr, expr_clone); + auto* v = as_const ? b.Const(name, nullptr, ctx.Clone(expr)) + : b.Var(name, nullptr, ctx.Clone(expr)); auto* decl = b.Decl(v); if (!InsertBefore(before_expr, decl)) { @@ -290,11 +279,7 @@ class HoistToDeclBefore::State { } // Replace the initializer expression with a reference to the let - const ast::Expression* new_expr = b.Expr(name); - if (is_ref) { - new_expr = b.Deref(new_expr); - } - ctx.Replace(expr, new_expr); + ctx.Replace(expr, b.Expr(name)); return true; } diff --git a/src/tint/transform/utils/hoist_to_decl_before_test.cc b/src/tint/transform/utils/hoist_to_decl_before_test.cc index 2e6f0abc83..70dc8bc6f8 100644 --- a/src/tint/transform/utils/hoist_to_decl_before_test.cc +++ b/src/tint/transform/utils/hoist_to_decl_before_test.cc @@ -243,8 +243,8 @@ TEST_F(HoistToDeclBeforeTest, Array1D) { auto* expect = R"( fn f() { var a : array; - let tint_symbol = &(a[0]); - var b = *(tint_symbol); + let tint_symbol = a[0]; + var b = tint_symbol; } )"; @@ -279,8 +279,8 @@ TEST_F(HoistToDeclBeforeTest, Array2D) { auto* expect = R"( fn f() { var a : array, 10>; - let tint_symbol = &(a[0][0]); - var b = *(tint_symbol); + let tint_symbol = a[0][0]; + var b = tint_symbol; } )";