[hlsl][ir] Only move a value to a let once.

In promote initializers once we've moved an initializer to a let, if that
value is used again, we can re-use the let that was already created.

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

Author: dj2

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

@@ -50,11 +50,16 @@ struct State {
 
     /// Process the module.
     void Process() {
+        /// Map of values to the new let versions
+        Hashmap<core::ir::Value*, core::ir::Let*, 8> values_in_lets{};
+
         for (auto* block : ir.blocks.Objects()) {
+            values_in_lets.Clear();
+
             // In the root block, all structs need to be split out, no nested structs
             bool is_root_block = block == ir.root_block;
 
-            Process(block, is_root_block);
+            Process(block, is_root_block, values_in_lets);
         }
     }
 
@@ -64,7 +69,9 @@ struct State {
         core::ir::Value* val;
     };
 
-    void Process(core::ir::Block* block, bool is_root_block) {
+    void Process(core::ir::Block* block,
+                 bool is_root_block,
+                 Hashmap<core::ir::Value*, core::ir::Let*, 8>& values_in_lets) {
         Vector<ValueInfo, 4> worklist;
 
         for (auto* inst : *block) {
@@ -98,10 +105,10 @@ struct State {
             if (auto* res = As<core::ir::InstructionResult>(item.val)) {
                 // 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);
+                    PutInLet(item.inst, item.index, res, values_in_lets);
                 }
             } else if (auto* val = As<core::ir::Constant>(item.val)) {
-                auto* let = PutInLet(item.inst, item.index, val);
+                auto* let = PutInLet(item.inst, item.index, val, values_in_lets);
                 auto ret = HoistModuleScopeLetToConstruct(is_root_block, item.inst, let, val);
                 if (ret.has_value()) {
                     const_worklist.Insert(0, *ret);
@@ -210,9 +217,18 @@ struct State {
         return let;
     }
 
-    core::ir::Let* PutInLet(core::ir::Instruction* inst, size_t index, core::ir::Value* value) {
-        auto* let = MakeLet(value);
-        let->InsertBefore(inst);
+    core::ir::Let* PutInLet(core::ir::Instruction* inst,
+                            size_t index,
+                            core::ir::Value* value,
+                            Hashmap<core::ir::Value*, core::ir::Let*, 8>& values_in_lets) {
+        core::ir::Let* let = nullptr;
+        if (auto res = values_in_lets.Get(value); res) {
+            let = *res;
+        } else {
+            let = MakeLet(value);
+            let->InsertBefore(inst);
+            values_in_lets.Add(value, let);
+        }
 
         inst->SetOperand(index, let->Result(0));
         return let;

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

@@ -682,8 +682,7 @@ A = struct @align(4) {
   $B2: {
     %4:A = let A(1i)
     %5:void = call %bar, %4
-    %6:A = let A(1i)
-    %7:void = call %bar, %6
+    %6:void = call %bar, %4
     ret
   }
 }
@@ -753,8 +752,7 @@ TEST_F(HlslWriterPromoteInitializersTest, DuplicateConstant) {
     EXPECT_EQ(expect, str());
 }
 
-// TODO(dsinclair): Fixup duplicate let creation
-TEST_F(HlslWriterPromoteInitializersTest, DISABLED_DuplicateAccess) {
+TEST_F(HlslWriterPromoteInitializersTest, DuplicateAccess) {
     capabilities = core::ir::Capabilities{core::ir::Capability::kAllowModuleScopeLets};
 
     auto* func = b.Function("foo", ty.void_(), core::ir::Function::PipelineStage::kFragment);
@@ -796,6 +794,119 @@ TEST_F(HlslWriterPromoteInitializersTest, DISABLED_DuplicateAccess) {
     EXPECT_EQ(expect, str());
 }
 
+TEST_F(HlslWriterPromoteInitializersTest, DuplicateAccessDifferentFunction) {
+    capabilities = core::ir::Capabilities{core::ir::Capability::kAllowModuleScopeLets};
+
+    auto* a = b.Function("a", ty.void_());
+    b.Append(a->Block(), [&] {
+        auto* ary = b.Splat(ty.array(ty.f32(), 8), 8_f);
+        b.Access(ty.f32(), ary, 0_u);
+        b.Return(a);
+    });
+
+    auto* func = b.Function("foo", ty.void_(), core::ir::Function::PipelineStage::kFragment);
+    b.Append(func->Block(), [&] {
+        auto* ary = b.Splat(ty.array(ty.f32(), 8), 8_f);
+        b.Access(ty.f32(), ary, 0_u);
+        b.Return(func);
+    });
+
+    auto* src = R"(
+%a = func():void {
+  $B1: {
+    %2:f32 = access array<f32, 8>(8.0f), 0u
+    ret
+  }
+}
+%foo = @fragment func():void {
+  $B2: {
+    %4:f32 = access array<f32, 8>(8.0f), 0u
+    ret
+  }
+}
+)";
+    EXPECT_EQ(src, str());
+
+    auto* expect = R"(
+%a = func():void {
+  $B1: {
+    %2:array<f32, 8> = let array<f32, 8>(8.0f)
+    %3:f32 = access %2, 0u
+    ret
+  }
+}
+%foo = @fragment func():void {
+  $B2: {
+    %5:array<f32, 8> = let array<f32, 8>(8.0f)
+    %6:f32 = access %5, 0u
+    ret
+  }
+}
+)";
+
+    Run(PromoteInitializers);
+    EXPECT_EQ(expect, str());
+}
+
+TEST_F(HlslWriterPromoteInitializersTest, DuplicateAccessDifferentScope) {
+    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));
+
+        auto* if_ = b.If(true);
+        b.Append(if_->True(), [&] {
+            b.Access(ty.f32(), ary, 0_u);
+            b.ExitIf(if_);
+        });
+
+        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
+    if true [t: $B2] {  # if_1
+      $B2: {  # true
+        %3:f32 = access %2, 0u
+        exit_if  # if_1
+      }
+    }
+    %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
+    if true [t: $B2] {  # if_1
+      $B2: {  # true
+        %3:array<f32, 8> = let %2
+        %4:f32 = access %3, 0u
+        exit_if  # if_1
+      }
+    }
+    %5:array<f32, 8> = let %2
+    %6:f32 = access %5, 1u
+    %7:f32 = access %5, 2u
+    ret
+  }
+}
+)";
+
+    Run(PromoteInitializers);
+    EXPECT_EQ(expect, str());
+}
+
 TEST_F(HlslWriterPromoteInitializersTest, LetOfLet) {
     capabilities = core::ir::Capabilities{core::ir::Capability::kAllowModuleScopeLets};
 

test/tint/bug/tint/366037039.wgsl.expected.ir.dxc.hlsl

@@ -101,8 +101,7 @@ void foo() {
   S w = v_9(0u);
   S v_23 = (S)0;
   v_3(0u, v_23);
-  S v_24 = (S)0;
-  wbuffer = v_24;
+  wbuffer = v_23;
 }
 
 [numthreads(1, 1, 1)]

test/tint/bug/tint/366037039.wgsl.expected.ir.fxc.hlsl

@@ -101,8 +101,7 @@ void foo() {
   S w = v_9(0u);
   S v_23 = (S)0;
   v_3(0u, v_23);
-  S v_24 = (S)0;
-  wbuffer = v_24;
+  wbuffer = v_23;
 }
 
 [numthreads(1, 1, 1)]

test/tint/extensions/clip_distances/first_member/clip_distances_size_2.wgsl.expected.ir.dxc.hlsl

@@ -17,8 +17,7 @@ VertexOutputs main_inner() {
 main_outputs main() {
   VertexOutputs v_1 = main_inner();
   float v_2[2] = v_1.clipDistance;
-  float v_3[2] = v_1.clipDistance;
-  main_outputs v_4 = {v_1.position, float2(v_2[0u], v_3[1u])};
-  return v_4;
+  main_outputs v_3 = {v_1.position, float2(v_2[0u], v_2[1u])};
+  return v_3;
 }
 

test/tint/extensions/clip_distances/first_member/clip_distances_size_2.wgsl.expected.ir.fxc.hlsl

@@ -17,8 +17,7 @@ VertexOutputs main_inner() {
 main_outputs main() {
   VertexOutputs v_1 = main_inner();
   float v_2[2] = v_1.clipDistance;
-  float v_3[2] = v_1.clipDistance;
-  main_outputs v_4 = {v_1.position, float2(v_2[0u], v_3[1u])};
-  return v_4;
+  main_outputs v_3 = {v_1.position, float2(v_2[0u], v_2[1u])};
+  return v_3;
 }
 

test/tint/extensions/clip_distances/first_member/clip_distances_size_3.wgsl.expected.ir.dxc.hlsl

@@ -17,9 +17,7 @@ VertexOutputs main_inner() {
 main_outputs main() {
   VertexOutputs v_1 = main_inner();
   float v_2[3] = v_1.clipDistance;
-  float v_3[3] = v_1.clipDistance;
-  float v_4[3] = v_1.clipDistance;
-  main_outputs v_5 = {v_1.position, float3(v_2[0u], v_3[1u], v_4[2u])};
-  return v_5;
+  main_outputs v_3 = {v_1.position, float3(v_2[0u], v_2[1u], v_2[2u])};
+  return v_3;
 }
 

test/tint/extensions/clip_distances/first_member/clip_distances_size_3.wgsl.expected.ir.fxc.hlsl

@@ -17,9 +17,7 @@ VertexOutputs main_inner() {
 main_outputs main() {
   VertexOutputs v_1 = main_inner();
   float v_2[3] = v_1.clipDistance;
-  float v_3[3] = v_1.clipDistance;
-  float v_4[3] = v_1.clipDistance;
-  main_outputs v_5 = {v_1.position, float3(v_2[0u], v_3[1u], v_4[2u])};
-  return v_5;
+  main_outputs v_3 = {v_1.position, float3(v_2[0u], v_2[1u], v_2[2u])};
+  return v_3;
 }
 

test/tint/extensions/clip_distances/first_member/clip_distances_size_4.wgsl.expected.ir.dxc.hlsl

@@ -17,10 +17,7 @@ VertexOutputs main_inner() {
 main_outputs main() {
   VertexOutputs v_1 = main_inner();
   float v_2[4] = v_1.clipDistance;
-  float v_3[4] = v_1.clipDistance;
-  float v_4[4] = v_1.clipDistance;
-  float v_5[4] = v_1.clipDistance;
-  main_outputs v_6 = {v_1.position, float4(v_2[0u], v_3[1u], v_4[2u], v_5[3u])};
-  return v_6;
+  main_outputs v_3 = {v_1.position, float4(v_2[0u], v_2[1u], v_2[2u], v_2[3u])};
+  return v_3;
 }
 

test/tint/extensions/clip_distances/first_member/clip_distances_size_4.wgsl.expected.ir.fxc.hlsl

@@ -17,10 +17,7 @@ VertexOutputs main_inner() {
 main_outputs main() {
   VertexOutputs v_1 = main_inner();
   float v_2[4] = v_1.clipDistance;
-  float v_3[4] = v_1.clipDistance;
-  float v_4[4] = v_1.clipDistance;
-  float v_5[4] = v_1.clipDistance;
-  main_outputs v_6 = {v_1.position, float4(v_2[0u], v_3[1u], v_4[2u], v_5[3u])};
-  return v_6;
+  main_outputs v_3 = {v_1.position, float4(v_2[0u], v_2[1u], v_2[2u], v_2[3u])};
+  return v_3;
 }