Support scalar replacement of large structs (#6019)

* Removes size limit on ScalarReplacementPass

* Adds test to verify default ScalarReplacementPass size limit

---------

Co-authored-by: Zackery Mason-Blaug <zackery.mason-blaug@ntd.nintendo.com>

Author: daniel-story

include/spirv-tools/optimizer.hpp

@@ -655,7 +655,7 @@ Optimizer::PassToken CreateRedundancyEliminationPass();
 // element if those elements are accessed individually.  The parameter is a
 // limit on the number of members in the composite variable that the pass will
 // consider replacing.
-Optimizer::PassToken CreateScalarReplacementPass(uint32_t size_limit = 100);
+Optimizer::PassToken CreateScalarReplacementPass(uint32_t size_limit = 0);
 
 // Create a private to local pass.
 // This pass looks for variables declared in the private storage class that are

source/opt/optimizer.cpp

@@ -189,7 +189,7 @@ Optimizer& Optimizer::RegisterPerformancePasses(bool preserve_interface) {
       .RegisterPass(CreateLocalSingleBlockLoadStoreElimPass())
       .RegisterPass(CreateLocalSingleStoreElimPass())
       .RegisterPass(CreateAggressiveDCEPass(preserve_interface))
-      .RegisterPass(CreateScalarReplacementPass())
+      .RegisterPass(CreateScalarReplacementPass(0))
       .RegisterPass(CreateLocalAccessChainConvertPass())
       .RegisterPass(CreateLocalSingleBlockLoadStoreElimPass())
       .RegisterPass(CreateLocalSingleStoreElimPass())
@@ -203,7 +203,7 @@ Optimizer& Optimizer::RegisterPerformancePasses(bool preserve_interface) {
       .RegisterPass(CreateRedundancyEliminationPass())
       .RegisterPass(CreateCombineAccessChainsPass())
       .RegisterPass(CreateSimplificationPass())
-      .RegisterPass(CreateScalarReplacementPass())
+      .RegisterPass(CreateScalarReplacementPass(0))
       .RegisterPass(CreateLocalAccessChainConvertPass())
       .RegisterPass(CreateLocalSingleBlockLoadStoreElimPass())
       .RegisterPass(CreateLocalSingleStoreElimPass())
@@ -401,7 +401,7 @@ bool Optimizer::RegisterPassFromFlag(const std::string& flag,
     RegisterPass(CreateLoopUnswitchPass());
   } else if (pass_name == "scalar-replacement") {
     if (pass_args.size() == 0) {
-      RegisterPass(CreateScalarReplacementPass());
+      RegisterPass(CreateScalarReplacementPass(0));
     } else {
       int limit = -1;
       if (pass_args.find_first_not_of("0123456789") == std::string::npos) {

source/opt/scalar_replacement_pass.h

@@ -33,7 +33,7 @@ namespace opt {
 // Documented in optimizer.hpp
 class ScalarReplacementPass : public MemPass {
  private:
-  static constexpr uint32_t kDefaultLimit = 100;
+  static constexpr uint32_t kDefaultLimit = 0;
 
  public:
   ScalarReplacementPass(uint32_t limit = kDefaultLimit)

test/opt/optimizer_test.cpp

@@ -388,6 +388,124 @@ OpFunctionEnd
       << "Was expecting the result id of DebugScope to have been changed.";
 }
 
+TEST(Optimizer, CheckDefaultPerformancePassesLargeStructScalarization) {
+  std::string start = R"(OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Vertex %4 "main" %46 %48
+OpSource GLSL 430
+OpName %4 "main"
+OpDecorate %44 Block
+OpMemberDecorate %44 0 BuiltIn Position
+OpMemberDecorate %44 1 BuiltIn PointSize
+OpMemberDecorate %44 2 BuiltIn ClipDistance
+OpDecorate %48 Location 0
+%2 = OpTypeVoid
+%3 = OpTypeFunction %2
+%6 = OpTypeFloat 32
+%7 = OpTypeVector %6 4
+%8 = OpTypePointer Function %7
+%9 = OpTypeStruct %7)";
+
+  // add 200 float members to the struct
+  for (int i = 0; i < 200; i++) {
+    start += " %6";
+  }
+
+  start += R"(
+%10 = OpTypeFunction %9 %8
+%14 = OpTypeFunction %6 %9
+%18 = OpTypePointer Function %9
+%20 = OpTypeInt 32 1
+%21 = OpConstant %20 0
+%24 = OpConstant %20 1
+%25 = OpTypeInt 32 0
+%26 = OpConstant %25 1
+%27 = OpTypePointer Function %6
+%43 = OpTypeArray %6 %26
+%44 = OpTypeStruct %7 %6 %43
+%45 = OpTypePointer Output %44
+%46 = OpVariable %45 Output
+%47 = OpTypePointer Input %7
+%48 = OpVariable %47 Input
+%54 = OpTypePointer Output %7
+%4 = OpFunction %2 None %3
+%5 = OpLabel
+%49 = OpVariable %8 Function
+%50 = OpLoad %7 %48
+OpStore %49 %50
+%51 = OpFunctionCall %9 %12 %49
+%52 = OpFunctionCall %6 %16 %51
+%53 = OpCompositeConstruct %7 %52 %52 %52 %52
+%55 = OpAccessChain %54 %46 %21
+OpStore %55 %53
+OpReturn
+OpFunctionEnd
+%12 = OpFunction %9 None %10
+%11 = OpFunctionParameter %8
+%13 = OpLabel
+%19 = OpVariable %18 Function
+%22 = OpLoad %7 %11
+%23 = OpAccessChain %8 %19 %21
+OpStore %23 %22
+%28 = OpAccessChain %27 %11 %26
+%29 = OpLoad %6 %28
+%30 = OpConvertFToS %20 %29
+%31 = OpAccessChain %27 %19 %21 %30
+%32 = OpLoad %6 %31
+%33 = OpAccessChain %27 %19 %24
+OpStore %33 %32
+%34 = OpLoad %9 %19
+OpReturnValue %34
+OpFunctionEnd
+%16 = OpFunction %6 None %14
+%15 = OpFunctionParameter %9
+%17 = OpLabel
+%37 = OpCompositeExtract %6 %15 1
+%38 = OpConvertFToS %20 %37
+%39 = OpCompositeExtract %7 %15 0
+%40 = OpVectorExtractDynamic %6 %39 %38
+OpReturnValue %40
+OpFunctionEnd)";
+
+  std::vector<uint32_t> binary;
+  SpirvTools tools(SPV_ENV_VULKAN_1_3);
+  tools.Assemble(start, &binary,
+                 SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+
+  std::string test_disassembly;
+  std::string default_disassembly;
+
+  {
+    Optimizer opt(SPV_ENV_VULKAN_1_3);
+    opt.RegisterPerformancePasses();
+
+    std::vector<uint32_t> optimized;
+    ASSERT_TRUE(opt.Run(binary.data(), binary.size(), &optimized))
+        << start << "\n";
+
+    tools.Disassemble(optimized.data(), optimized.size(), &default_disassembly,
+                      SPV_BINARY_TO_TEXT_OPTION_NO_HEADER);
+  }
+
+  {
+    // default passes should not benefit from additional scalar replacement
+    Optimizer opt(SPV_ENV_VULKAN_1_3);
+    opt.RegisterPerformancePasses()
+        .RegisterPass(CreateScalarReplacementPass(201))
+        .RegisterPass(CreateAggressiveDCEPass());
+
+    std::vector<uint32_t> optimized;
+    ASSERT_TRUE(opt.Run(binary.data(), binary.size(), &optimized))
+        << start << "\n";
+
+    tools.Disassemble(optimized.data(), optimized.size(), &test_disassembly,
+                      SPV_BINARY_TO_TEXT_OPTION_NO_HEADER);
+  }
+
+  EXPECT_EQ(test_disassembly, default_disassembly);
+}
+
 }  // namespace
 }  // namespace opt
 }  // namespace spvtools

test/opt/scalar_replacement_test.cpp

@@ -27,7 +27,7 @@ using ScalarReplacementPassName = ::testing::Test;
 
 TEST_F(ScalarReplacementPassName, Default) {
   auto srp = ScalarReplacementPass();
-  EXPECT_STREQ(srp.name(), "scalar-replacement=100");
+  EXPECT_STREQ(srp.name(), "scalar-replacement=0");
 }
 
 TEST_F(ScalarReplacementPassName, Large) {

test/tools/opt/flags.py

@@ -113,7 +113,7 @@ class TestValidPassFlags(expect.ValidObjectFile1_6,
       'remove-duplicates',
       'replace-invalid-opcode',
       'ssa-rewrite',
-      'scalar-replacement=100',
+      'scalar-replacement=0',
       'scalar-replacement=42',
       'strength-reduction',
       'strip-debug',
@@ -148,7 +148,7 @@ class TestPerformanceOptimizationPasses(expect.ValidObjectFile1_6,
       'eliminate-local-single-block',
       'eliminate-local-single-store',
       'eliminate-dead-code-aggressive',
-      'scalar-replacement=100',
+      'scalar-replacement=0',
       'convert-local-access-chains',
       'eliminate-local-single-block',
       'eliminate-local-single-store',
@@ -162,7 +162,7 @@ class TestPerformanceOptimizationPasses(expect.ValidObjectFile1_6,
       'redundancy-elimination',
       'combine-access-chains',
       'simplify-instructions',
-      'scalar-replacement=100',
+      'scalar-replacement=0',
       'convert-local-access-chains',
       'eliminate-local-single-block',
       'eliminate-local-single-store',