[hlsl][ir] Skip promoting lets to lets

In the `PromoteInitializers` transform, if a let is used as a value, we
may determine that the value needs to be hoisted to a new let. But, this
ends up just creating a let of a let. Skip creating the duplicate let in
this case.

Bug: 369450791
Change-Id: I45178ec0029b13019314ab189df8073781a9dd9b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/211817
Reviewed-by: James Price <[email protected]>
Commit-Queue: dan sinclair <[email protected]>
Reviewed-by: Antonio Maiorano <[email protected]>

Author: dj2

src/tint/lang/hlsl/writer/access_test.cc

@@ -1903,8 +1903,7 @@ void v_1(uint offset, float3 obj[5]) {
 }
 
 void foo() {
-  float3 v_4[5] = (float3[5])0;
-  float3 ary[5] = v_4;
+  float3 ary[5] = (float3[5])0;
   v_1(0u, ary);
 }
 
@@ -2061,8 +2060,7 @@ void v_4(uint offset, SB obj) {
 }
 
 void foo() {
-  SB v_6 = (SB)0;
-  SB s = v_6;
+  SB s = (SB)0;
   v_4(0u, s);
 }
 
@@ -2157,8 +2155,7 @@ void v_9(uint offset, SB obj) {
 }
 
 void foo() {
-  SB v_11 = (SB)0;
-  SB s = v_11;
+  SB s = (SB)0;
   v_9(0u, s);
 }
 

src/tint/lang/hlsl/writer/function_test.cc

@@ -309,16 +309,13 @@ void frag_main_inner(Interface inputs) {
 
 vert_main_outputs vert_main() {
   Interface v_1 = vert_main_inner();
-  Interface v_2 = v_1;
-  Interface v_3 = v_1;
-  Interface v_4 = v_1;
-  vert_main_outputs v_5 = {v_3.col1, v_4.col2, v_2.pos};
-  return v_5;
+  vert_main_outputs v_2 = {v_1.col1, v_1.col2, v_1.pos};
+  return v_2;
 }
 
 void frag_main(frag_main_inputs inputs) {
-  Interface v_6 = {float4(inputs.Interface_pos.xyz, (1.0f / inputs.Interface_pos[3u])), inputs.Interface_col1, inputs.Interface_col2};
-  frag_main_inner(v_6);
+  Interface v_3 = {float4(inputs.Interface_pos.xyz, (1.0f / inputs.Interface_pos[3u])), inputs.Interface_col1, inputs.Interface_col2};
+  frag_main_inner(v_3);
 }
 
 )");

src/tint/lang/hlsl/writer/raise/promote_initializers.cc

@@ -96,7 +96,10 @@ struct State {
         Vector<core::ir::Construct*, 4> const_worklist;
         for (auto& item : worklist) {
             if (auto* res = As<core::ir::InstructionResult>(item.val)) {
-                PutInLet(item.inst, item.index, res);
+                // If the value isn't already a `let`, put it into a `let`.
+                if (!res->Instruction()->Is<core::ir::Let>()) {
+                    PutInLet(item.inst, item.index, res);
+                }
             } else if (auto* val = As<core::ir::Constant>(item.val)) {
                 auto* let = PutInLet(item.inst, item.index, val);
                 auto ret = HoistModuleScopeLetToConstruct(is_root_block, item.inst, let, val);

src/tint/lang/hlsl/writer/raise/promote_initializers_test.cc

@@ -753,5 +753,115 @@ TEST_F(HlslWriterPromoteInitializersTest, DuplicateConstant) {
     EXPECT_EQ(expect, str());
 }
 
+// TODO(dsinclair): Fixup duplicate let creation
+TEST_F(HlslWriterPromoteInitializersTest, DISABLED_DuplicateAccess) {
+    capabilities = core::ir::Capabilities{core::ir::Capability::kAllowModuleScopeLets};
+
+    auto* func = b.Function("foo", ty.void_(), core::ir::Function::PipelineStage::kFragment);
+    b.Append(func->Block(), [&] {
+        auto* ary = b.Construct(ty.array(ty.f32(), 8));
+        b.Access(ty.f32(), ary, 0_u);
+        b.Access(ty.f32(), ary, 1_u);
+        b.Access(ty.f32(), ary, 2_u);
+        b.Return(func);
+    });
+
+    auto* src = R"(
+%foo = @fragment func():void {
+  $B1: {
+    %2:array<f32, 8> = construct
+    %3:f32 = access %2, 0u
+    %4:f32 = access %2, 1u
+    %5:f32 = access %2, 2u
+    ret
+  }
+}
+)";
+    EXPECT_EQ(src, str());
+
+    auto* expect = R"(
+%foo = @fragment func():void {
+  $B1: {
+    %2:array<f32, 8> = construct
+    %3:array<f32, 8> = let %2
+    %4:f32 = access %3, 0u
+    %5:f32 = access %3, 1u
+    %6:f32 = access %3, 2u
+    ret
+  }
+}
+)";
+
+    Run(PromoteInitializers);
+    EXPECT_EQ(expect, str());
+}
+
+TEST_F(HlslWriterPromoteInitializersTest, LetOfLet) {
+    capabilities = core::ir::Capabilities{core::ir::Capability::kAllowModuleScopeLets};
+
+    auto* str_ty = ty.Struct(mod.symbols.New("S"), {
+                                                       {mod.symbols.New("a"), ty.vec4<i32>()},
+                                                   });
+
+    auto* inner = b.Function("inner", str_ty);
+    b.Append(inner->Block(),
+             [&] { b.Return(inner, b.Construct(str_ty, b.Splat(ty.vec4<i32>(), 1_i))); });
+
+    auto* func = b.Function("foo", ty.void_(), core::ir::Function::PipelineStage::kFragment);
+    b.Append(func->Block(), [&] {
+        auto* in = b.Call(inner);
+        auto* l = b.Let("a", in);
+        b.Access(ty.vec4<i32>(), l, 0_u);
+        b.Return(func);
+    });
+
+    auto* src = R"(
+S = struct @align(16) {
+  a:vec4<i32> @offset(0)
+}
+
+%inner = func():S {
+  $B1: {
+    %2:S = construct vec4<i32>(1i)
+    ret %2
+  }
+}
+%foo = @fragment func():void {
+  $B2: {
+    %4:S = call %inner
+    %a:S = let %4
+    %6:vec4<i32> = access %a, 0u
+    ret
+  }
+}
+)";
+    EXPECT_EQ(src, str());
+
+    auto* expect = R"(
+S = struct @align(16) {
+  a:vec4<i32> @offset(0)
+}
+
+%inner = func():S {
+  $B1: {
+    %2:S = construct vec4<i32>(1i)
+    %3:S = let %2
+    ret %3
+  }
+}
+%foo = @fragment func():void {
+  $B2: {
+    %5:S = call %inner
+    %a:S = let %5
+    %7:vec4<i32> = access %a, 0u
+    ret
+  }
+}
+)";
+
+    Run(PromoteInitializers);
+    EXPECT_EQ(expect, str());
+}
+
 }  // namespace
 }  // namespace tint::hlsl::writer::raise

test/tint/array/assign_to_function_var.wgsl.expected.ir.dxc.hlsl

@@ -76,47 +76,45 @@ void foo(int4 src_param[4]) {
   tint_symbol = src_param;
   int4 v_11[4] = ret_arr();
   tint_symbol = v_11;
-  int4 v_12[4] = (int4[4])0;
-  int4 src_let[4] = v_12;
+  int4 src_let[4] = (int4[4])0;
   tint_symbol = src_let;
-  int4 v_13[4] = src_function;
+  int4 v_12[4] = src_function;
+  tint_symbol = v_12;
+  int4 v_13[4] = src_private;
   tint_symbol = v_13;
-  int4 v_14[4] = src_private;
+  int4 v_14[4] = src_workgroup;
   tint_symbol = v_14;
-  int4 v_15[4] = src_workgroup;
-  tint_symbol = v_15;
-  S v_16 = ret_struct_arr();
-  int4 v_17[4] = v_16.arr;
+  S v_15 = ret_struct_arr();
+  int4 v_16[4] = v_15.arr;
+  tint_symbol = v_16;
+  int4 v_17[4] = v_6(0u);
   tint_symbol = v_17;
-  int4 v_18[4] = v_6(0u);
+  int4 v_18[4] = v_2(0u);
   tint_symbol = v_18;
-  int4 v_19[4] = v_2(0u);
-  tint_symbol = v_19;
   int dst_nested[4][3][2] = (int[4][3][2])0;
   int src_nested[4][3][2] = (int[4][3][2])0;
-  int v_20[4][3][2] = src_nested;
-  dst_nested = v_20;
+  int v_19[4][3][2] = src_nested;
+  dst_nested = v_19;
 }
 
 void main_inner(uint tint_local_index) {
   {
-    uint v_21 = 0u;
-    v_21 = tint_local_index;
+    uint v_20 = 0u;
+    v_20 = tint_local_index;
     while(true) {
-      uint v_22 = v_21;
-      if ((v_22 >= 4u)) {
+      uint v_21 = v_20;
+      if ((v_21 >= 4u)) {
         break;
       }
-      src_workgroup[v_22] = (int(0)).xxxx;
+      src_workgroup[v_21] = (int(0)).xxxx;
       {
-        v_21 = (v_22 + 1u);
+        v_20 = (v_21 + 1u);
       }
       continue;
     }
   }
   GroupMemoryBarrierWithGroupSync();
-  int4 v_23[4] = (int4[4])0;
-  int4 val[4] = v_23;
+  int4 val[4] = (int4[4])0;
   foo(val);
 }
 

test/tint/array/assign_to_function_var.wgsl.expected.ir.fxc.hlsl

@@ -76,47 +76,45 @@ void foo(int4 src_param[4]) {
   tint_symbol = src_param;
   int4 v_11[4] = ret_arr();
   tint_symbol = v_11;
-  int4 v_12[4] = (int4[4])0;
-  int4 src_let[4] = v_12;
+  int4 src_let[4] = (int4[4])0;
   tint_symbol = src_let;
-  int4 v_13[4] = src_function;
+  int4 v_12[4] = src_function;
+  tint_symbol = v_12;
+  int4 v_13[4] = src_private;
   tint_symbol = v_13;
-  int4 v_14[4] = src_private;
+  int4 v_14[4] = src_workgroup;
   tint_symbol = v_14;
-  int4 v_15[4] = src_workgroup;
-  tint_symbol = v_15;
-  S v_16 = ret_struct_arr();
-  int4 v_17[4] = v_16.arr;
+  S v_15 = ret_struct_arr();
+  int4 v_16[4] = v_15.arr;
+  tint_symbol = v_16;
+  int4 v_17[4] = v_6(0u);
   tint_symbol = v_17;
-  int4 v_18[4] = v_6(0u);
+  int4 v_18[4] = v_2(0u);
   tint_symbol = v_18;
-  int4 v_19[4] = v_2(0u);
-  tint_symbol = v_19;
   int dst_nested[4][3][2] = (int[4][3][2])0;
   int src_nested[4][3][2] = (int[4][3][2])0;
-  int v_20[4][3][2] = src_nested;
-  dst_nested = v_20;
+  int v_19[4][3][2] = src_nested;
+  dst_nested = v_19;
 }
 
 void main_inner(uint tint_local_index) {
   {
-    uint v_21 = 0u;
-    v_21 = tint_local_index;
+    uint v_20 = 0u;
+    v_20 = tint_local_index;
     while(true) {
-      uint v_22 = v_21;
-      if ((v_22 >= 4u)) {
+      uint v_21 = v_20;
+      if ((v_21 >= 4u)) {
         break;
       }
-      src_workgroup[v_22] = (int(0)).xxxx;
+      src_workgroup[v_21] = (int(0)).xxxx;
       {
-        v_21 = (v_22 + 1u);
+        v_20 = (v_21 + 1u);
       }
       continue;
     }
   }
   GroupMemoryBarrierWithGroupSync();
-  int4 v_23[4] = (int4[4])0;
-  int4 val[4] = v_23;
+  int4 val[4] = (int4[4])0;
   foo(val);
 }
 

test/tint/array/assign_to_private_var.wgsl.expected.ir.dxc.hlsl

@@ -77,46 +77,44 @@ void foo(int4 src_param[4]) {
   tint_symbol = src_param;
   int4 v_11[4] = ret_arr();
   tint_symbol = v_11;
-  int4 v_12[4] = (int4[4])0;
-  int4 src_let[4] = v_12;
+  int4 src_let[4] = (int4[4])0;
   tint_symbol = src_let;
-  int4 v_13[4] = src_function;
+  int4 v_12[4] = src_function;
+  tint_symbol = v_12;
+  int4 v_13[4] = src_private;
   tint_symbol = v_13;
-  int4 v_14[4] = src_private;
+  int4 v_14[4] = src_workgroup;
   tint_symbol = v_14;
-  int4 v_15[4] = src_workgroup;
-  tint_symbol = v_15;
-  S v_16 = ret_struct_arr();
-  int4 v_17[4] = v_16.arr;
+  S v_15 = ret_struct_arr();
+  int4 v_16[4] = v_15.arr;
+  tint_symbol = v_16;
+  int4 v_17[4] = v_6(0u);
   tint_symbol = v_17;
-  int4 v_18[4] = v_6(0u);
+  int4 v_18[4] = v_2(0u);
   tint_symbol = v_18;
-  int4 v_19[4] = v_2(0u);
-  tint_symbol = v_19;
   int src_nested[4][3][2] = (int[4][3][2])0;
-  int v_20[4][3][2] = src_nested;
-  dst_nested = v_20;
+  int v_19[4][3][2] = src_nested;
+  dst_nested = v_19;
 }
 
 void main_inner(uint tint_local_index) {
   {
-    uint v_21 = 0u;
-    v_21 = tint_local_index;
+    uint v_20 = 0u;
+    v_20 = tint_local_index;
     while(true) {
-      uint v_22 = v_21;
-      if ((v_22 >= 4u)) {
+      uint v_21 = v_20;
+      if ((v_21 >= 4u)) {
         break;
       }
-      src_workgroup[v_22] = (int(0)).xxxx;
+      src_workgroup[v_21] = (int(0)).xxxx;
       {
-        v_21 = (v_22 + 1u);
+        v_20 = (v_21 + 1u);
       }
       continue;
     }
   }
   GroupMemoryBarrierWithGroupSync();
-  int4 v_23[4] = (int4[4])0;
-  int4 a[4] = v_23;
+  int4 a[4] = (int4[4])0;
   foo(a);
 }