From 4649377d0575a0ae8c314ee6235fa0cd55413e22 Mon Sep 17 00:00:00 2001
From: Ben Clayton <bclayton@google.com>
Date: Sat, 17 Apr 2021 06:02:21 +0000
Subject: [PATCH] transform: Don't create detached AST nodes

Detached AST nodes will become an ICE, so only create nodes if they are going to be used.

Bug: tint:469
Change-Id: I29b3275a355e30c934e6e508185a19981dcc8a42
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48050
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
---
 src/transform/canonicalize_entry_point_io.cc | 28 +++++++++++++-------
 src/transform/spirv.cc                       |  9 +++++--
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/transform/canonicalize_entry_point_io.cc b/src/transform/canonicalize_entry_point_io.cc
index 9f7e8813a5..38f7d6868f 100644
--- a/src/transform/canonicalize_entry_point_io.cc
+++ b/src/transform/canonicalize_entry_point_io.cc
@@ -16,6 +16,7 @@
 
 #include <algorithm>
 #include <utility>
+#include <vector>
 
 #include "src/program_builder.h"
 #include "src/sem/function.h"
@@ -104,11 +105,11 @@ Output CanonicalizeEntryPointIO::Run(const Program* in, const DataMap&) {
         auto* param_ty = ctx.src->Sem().Get(param)->Type();
         auto* param_declared_ty = ctx.src->Sem().Get(param)->DeclaredType();
 
-        ast::Expression* func_const_initializer = nullptr;
+        std::function<ast::Expression*()> func_const_initializer;
 
         if (auto* struct_ty = param_ty->As<type::Struct>()) {
           // Pull out all struct members and build initializer list.
-          ast::ExpressionList init_values;
+          std::vector<Symbol> member_names;
           for (auto* member : struct_ty->impl()->members()) {
             if (member->type()->UnwrapAll()->Is<type::Struct>()) {
               TINT_ICE(ctx.dst->Diagnostics()) << "nested pipeline IO struct";
@@ -122,12 +123,19 @@ Output CanonicalizeEntryPointIO::Run(const Program* in, const DataMap&) {
             auto member_name = ctx.Clone(member->symbol());
             new_struct_members.push_back(ctx.dst->Member(
                 member_name, ctx.Clone(member->type()), new_decorations));
-            init_values.push_back(
-                ctx.dst->MemberAccessor(new_struct_param_symbol, member_name));
+            member_names.emplace_back(member_name);
           }
 
-          func_const_initializer =
-              ctx.dst->Construct(ctx.Clone(param_declared_ty), init_values);
+          func_const_initializer = [&ctx, new_struct_param_symbol,
+                                    param_declared_ty, member_names]() {
+            ast::ExpressionList init_values;
+            for (auto name : member_names) {
+              init_values.push_back(
+                  ctx.dst->MemberAccessor(new_struct_param_symbol, name));
+            }
+            return ctx.dst->Construct(ctx.Clone(param_declared_ty),
+                                      init_values);
+          };
         } else {
           ast::DecorationList new_decorations = RemoveDecorations(
               &ctx, param->decorations(), [](const ast::Decoration* deco) {
@@ -136,8 +144,10 @@ Output CanonicalizeEntryPointIO::Run(const Program* in, const DataMap&) {
               });
           new_struct_members.push_back(ctx.dst->Member(
               param_name, ctx.Clone(param_declared_ty), new_decorations));
-          func_const_initializer =
-              ctx.dst->MemberAccessor(new_struct_param_symbol, param_name);
+          func_const_initializer = [&ctx, new_struct_param_symbol,
+                                    param_name]() {
+            return ctx.dst->MemberAccessor(new_struct_param_symbol, param_name);
+          };
         }
 
         if (func->body()->empty()) {
@@ -148,7 +158,7 @@ Output CanonicalizeEntryPointIO::Run(const Program* in, const DataMap&) {
         // Create a function-scope const to replace the parameter.
         // Initialize it with the value extracted from the new struct parameter.
         auto* func_const = ctx.dst->Const(
-            param_name, ctx.Clone(param_declared_ty), func_const_initializer);
+            param_name, ctx.Clone(param_declared_ty), func_const_initializer());
         ctx.InsertBefore(func->body()->statements(), *func->body()->begin(),
                          ctx.dst->WrapInStatement(func_const));
 
diff --git a/src/transform/spirv.cc b/src/transform/spirv.cc
index 411213d549..0077198763 100644
--- a/src/transform/spirv.cc
+++ b/src/transform/spirv.cc
@@ -271,12 +271,12 @@ Symbol Spirv::HoistToInputVariables(
   }
 
   // Recurse into struct members and build the initializer list.
-  ast::ExpressionList init_values;
+  std::vector<Symbol> init_value_names;
   auto* struct_ty = ty->As<type::Struct>();
   for (auto* member : struct_ty->impl()->members()) {
     auto member_var = HoistToInputVariables(
         ctx, func, member->type(), member->type(), member->decorations());
-    init_values.push_back(ctx.dst->Expr(member_var));
+    init_value_names.emplace_back(member_var);
   }
 
   auto func_var_symbol = ctx.dst->Symbols().New();
@@ -285,6 +285,11 @@ Symbol Spirv::HoistToInputVariables(
     return func_var_symbol;
   }
 
+  ast::ExpressionList init_values;
+  for (auto name : init_value_names) {
+    init_values.push_back(ctx.dst->Expr(name));
+  }
+
   // Create a function-scope variable for the struct.
   auto* initializer = ctx.dst->Construct(ctx.Clone(declared_ty), init_values);
   auto* func_var =