tint/PromoteInitializers: Do not hoist abstracts

Hoisting them would cause premature materialization. This will only
happen in cases where we do not actually need to hoist (e.g. assigning
to a phony), so we can safely skip these.

Fixed: tint:1852
Change-Id: Ifcbe3e13496daa0a6aaceb58540e60cb037885ea
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/122104
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>

src/tint/transform/promote_initializers_to_let.cc

@@ -52,6 +52,11 @@ Transform::ApplyResult PromoteInitializersToLet::Apply(const Program* src,
             // Follow const-chains
             auto* root_expr = expr;
             if (expr->Stage() == sem::EvaluationStage::kConstant) {
+                if (expr->Type()->HoldsAbstract()) {
+                    // Do not hoist expressions that are not materialized, as doing so would cause
+                    // premature materialization.
+                    return false;
+                }
                 while (auto* user = root_expr->UnwrapMaterialize()->As<sem::VariableUser>()) {
                     root_expr = user->Variable()->Initializer();
                 }

src/tint/transform/promote_initializers_to_let_test.cc

@@ -1335,5 +1335,17 @@ fn f() {
     EXPECT_EQ(expect, str(got));
 }
 
+TEST_F(PromoteInitializersToLetTest, AssignAbstractArray_ToPhony) {
+    // Test that we do not try to hoist an abstract array expression that is the RHS of a phony
+    // assignment, as its type will not be materialized.
+    auto* src = R"(
+fn f() {
+  _ = array(1, 2, 3, 4);
+}
+)";
+
+    EXPECT_FALSE(ShouldRun<PromoteInitializersToLet>(src));
+}
+
 }  // namespace
 }  // namespace tint::transform

src/tint/transform/utils/hoist_to_decl_before.cc

@@ -44,10 +44,11 @@ struct HoistToDeclBefore::State {
 
         switch (kind) {
             case VariableKind::kLet: {
-                auto builder = [this, expr, name] {
+                auto* ty = ctx.src->Sem().GetVal(expr)->Type();
-                    return b.Decl(b.Let(
+                TINT_ASSERT(Transform, !ty->HoldsAbstract());
-                        name, Transform::CreateASTTypeFor(ctx, ctx.src->Sem().GetVal(expr)->Type()),
+                auto builder = [this, expr, name, ty] {
-                        ctx.CloneWithoutTransform(expr)));
+                    return b.Decl(b.Let(name, Transform::CreateASTTypeFor(ctx, ty),
+                                        ctx.CloneWithoutTransform(expr)));
                 };
                 if (!InsertBeforeImpl(before_expr->Stmt(), std::move(builder))) {
                     return false;
@@ -56,10 +57,11 @@ struct HoistToDeclBefore::State {
             }
 
             case VariableKind::kVar: {
-                auto builder = [this, expr, name] {
+                auto* ty = ctx.src->Sem().GetVal(expr)->Type();
-                    return b.Decl(b.Var(
+                TINT_ASSERT(Transform, !ty->HoldsAbstract());
-                        name, Transform::CreateASTTypeFor(ctx, ctx.src->Sem().GetVal(expr)->Type()),
+                auto builder = [this, expr, name, ty] {
-                        ctx.CloneWithoutTransform(expr)));
+                    return b.Decl(b.Var(name, Transform::CreateASTTypeFor(ctx, ty),
+                                        ctx.CloneWithoutTransform(expr)));
                 };
                 if (!InsertBeforeImpl(before_expr->Stmt(), std::move(builder))) {
                     return false;
@@ -78,7 +80,7 @@ struct HoistToDeclBefore::State {
             }
         }
 
-        // Replace the initializer expression with a reference to the let
+        // Replace the source expression with a reference to the hoisted declaration.
         ctx.Replace(expr, b.Expr(name));
         return true;
     }

test/tint/bug/tint/1852.wgsl

@@ -0,0 +1,3 @@
+fn foo(){
+  _ = array(4);
+}

test/tint/bug/tint/1852.wgsl.expected.dxc.hlsl

@@ -0,0 +1,7 @@
+[numthreads(1, 1, 1)]
+void unused_entry_point() {
+  return;
+}
+
+void foo() {
+}

test/tint/bug/tint/1852.wgsl.expected.fxc.hlsl

@@ -0,0 +1,7 @@
+[numthreads(1, 1, 1)]
+void unused_entry_point() {
+  return;
+}
+
+void foo() {
+}

test/tint/bug/tint/1852.wgsl.expected.glsl

@@ -0,0 +1,9 @@
+#version 310 es
+
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+void unused_entry_point() {
+  return;
+}
+void foo() {
+}
+

test/tint/bug/tint/1852.wgsl.expected.msl

@@ -0,0 +1,6 @@
+#include <metal_stdlib>
+
+using namespace metal;
+void foo() {
+}
+

test/tint/bug/tint/1852.wgsl.expected.spvasm

@@ -0,0 +1,21 @@
+; SPIR-V
+; Version: 1.3
+; Generator: Google Tint Compiler; 0
+; Bound: 7
+; Schema: 0
+               OpCapability Shader
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint GLCompute %unused_entry_point "unused_entry_point"
+               OpExecutionMode %unused_entry_point LocalSize 1 1 1
+               OpName %unused_entry_point "unused_entry_point"
+               OpName %foo "foo"
+       %void = OpTypeVoid
+          %1 = OpTypeFunction %void
+%unused_entry_point = OpFunction %void None %1
+          %4 = OpLabel
+               OpReturn
+               OpFunctionEnd
+        %foo = OpFunction %void None %1
+          %6 = OpLabel
+               OpReturn
+               OpFunctionEnd

test/tint/bug/tint/1852.wgsl.expected.wgsl

@@ -0,0 +1,3 @@
+fn foo() {
+  _ = array(4);
+}