Set threshold for reduce-load-size pass (#4499)

Allow uses to set the threshold for spirv-opt reduce-load-size pass
2021-09-02 10:45:51 -04:00 · 2021-09-02 10:45:51 -04:00 · 0c09258e07
--- a/include/spirv-tools/optimizer.hpp
+++ b/include/spirv-tools/optimizer.hpp
@ -701,8 +701,11 @@ Optimizer::PassToken CreateVectorDCEPass();
 // Create a pass to reduce the size of loads.
 // This pass looks for loads of structures where only a few of its members are
 // used.  It replaces the loads feeding an OpExtract with an OpAccessChain and
-// a load of the specific elements.
-Optimizer::PassToken CreateReduceLoadSizePass();
+// a load of the specific elements.  The parameter is a threshold to determine
+// whether we have to replace the load or not.  If the ratio of the used
+// components of the load is less than the threshold, we replace the load.
+Optimizer::PassToken CreateReduceLoadSizePass(
+    double load_replacement_threshold = 0.9);

 // Create a pass to combine chained access chains.
 // This pass looks for access chains fed by other access chains and combines
--- a/source/opt/optimizer.cpp
+++ b/source/opt/optimizer.cpp
@ -385,7 +385,23 @@ bool Optimizer::RegisterPassFromFlag(const std::string& flag) {
  } else if (pass_name == "loop-invariant-code-motion") {
    RegisterPass(CreateLoopInvariantCodeMotionPass());
  } else if (pass_name == "reduce-load-size") {
-    RegisterPass(CreateReduceLoadSizePass());
+    if (pass_args.size() == 0) {
+      RegisterPass(CreateReduceLoadSizePass());
+    } else {
+      double load_replacement_threshold = 0.9;
+      if (pass_args.find_first_not_of(".0123456789") == std::string::npos) {
+        load_replacement_threshold = atof(pass_args.c_str());
+      }
+
+      if (load_replacement_threshold >= 0) {
+        RegisterPass(CreateReduceLoadSizePass(load_replacement_threshold));
+      } else {
+        Error(consumer(), nullptr, {},
+              "--reduce-load-size must have no arguments or a non-negative "
+              "double argument");
+        return false;
+      }
+    }
  } else if (pass_name == "redundancy-elimination") {
    RegisterPass(CreateRedundancyEliminationPass());
  } else if (pass_name == "private-to-local") {
@ -879,9 +895,10 @@ Optimizer::PassToken CreateVectorDCEPass() {
  return MakeUnique<Optimizer::PassToken::Impl>(MakeUnique<opt::VectorDCE>());
 }

-Optimizer::PassToken CreateReduceLoadSizePass() {
+Optimizer::PassToken CreateReduceLoadSizePass(
+    double load_replacement_threshold) {
  return MakeUnique<Optimizer::PassToken::Impl>(
-      MakeUnique<opt::ReduceLoadSize>());
+      MakeUnique<opt::ReduceLoadSize>(load_replacement_threshold));
 }

 Optimizer::PassToken CreateCombineAccessChainsPass() {
--- a/source/opt/reduce_load_size.cpp
+++ b/source/opt/reduce_load_size.cpp
@ -27,7 +27,6 @@ namespace {
 const uint32_t kExtractCompositeIdInIdx = 0;
 const uint32_t kVariableStorageClassInIdx = 0;
 const uint32_t kLoadPointerInIdx = 0;
-const double kThreshold = 0.9;

 }  // namespace

@ -151,6 +150,8 @@ bool ReduceLoadSize::ShouldReplaceExtract(Instruction* inst) {
  bool should_replace = false;
  if (all_elements_used) {
    should_replace = false;
+  } else if (1.0 <= replacement_threshold_) {
+    should_replace = true;
  } else {
    analysis::ConstantManager* const_mgr = context()->get_constant_mgr();
    analysis::TypeManager* type_mgr = context()->get_type_mgr();
@ -172,7 +173,7 @@ bool ReduceLoadSize::ShouldReplaceExtract(Instruction* inst) {
    }
    double percent_used = static_cast<double>(elements_used.size()) /
                          static_cast<double>(total_size);
-    should_replace = (percent_used < kThreshold);
+    should_replace = (percent_used < replacement_threshold_);
  }

  should_replace_cache_[op_inst->result_id()] = should_replace;
--- a/source/opt/reduce_load_size.h
+++ b/source/opt/reduce_load_size.h
@ -27,6 +27,9 @@ namespace opt {
 // See optimizer.hpp for documentation.
 class ReduceLoadSize : public Pass {
 public:
+  explicit ReduceLoadSize(double replacement_threshold)
+      : replacement_threshold_(replacement_threshold) {}
+
  const char* name() const override { return "reduce-load-size"; }
  Status Process() override;

@ -54,6 +57,11 @@ class ReduceLoadSize : public Pass {
  // on the load feeding |inst|.
  bool ShouldReplaceExtract(Instruction* inst);

+  // Threshold to determine whether we have to replace the load or not. If the
+  // ratio of the used components of the load is less than the threshold, we
+  // replace the load.
+  double replacement_threshold_;
+
  // Maps the result id of an OpLoad instruction to the result of whether or
  // not the OpCompositeExtract that use the id should be replaced.
  std::unordered_map<uint32_t, bool> should_replace_cache_;
--- a/test/opt/reduce_load_size_test.cpp
+++ b/test/opt/reduce_load_size_test.cpp
@ -17,6 +17,12 @@
 #include "test/opt/pass_fixture.h"
 #include "test/opt/pass_utils.h"

+namespace {
+
+const double kDefaultLoadReductionThreshold = 0.9;
+
+}  // namespace
+
 namespace spvtools {
 namespace opt {
 namespace {
@ -104,7 +110,8 @@ TEST_F(ReduceLoadSizeTest, cbuffer_load_extract) {
  SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
  SetDisassembleOptions(SPV_BINARY_TO_TEXT_OPTION_NO_HEADER |
                        SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES);
-  SinglePassRunAndMatch<ReduceLoadSize>(test, false);
+  SinglePassRunAndMatch<ReduceLoadSize>(test, false,
+                                        kDefaultLoadReductionThreshold);
 }

 TEST_F(ReduceLoadSizeTest, cbuffer_load_extract_not_affected_by_debug_instr) {
@ -202,7 +209,8 @@ TEST_F(ReduceLoadSizeTest, cbuffer_load_extract_not_affected_by_debug_instr) {
  SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
  SetDisassembleOptions(SPV_BINARY_TO_TEXT_OPTION_NO_HEADER |
                        SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES);
-  SinglePassRunAndMatch<ReduceLoadSize>(test, false);
+  SinglePassRunAndMatch<ReduceLoadSize>(test, false,
+                                        kDefaultLoadReductionThreshold);
 }

 TEST_F(ReduceLoadSizeTest, cbuffer_load_extract_vector) {
@ -280,7 +288,8 @@ OpFunctionEnd
  SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
  SetDisassembleOptions(SPV_BINARY_TO_TEXT_OPTION_NO_HEADER |
                        SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES);
-  SinglePassRunAndCheck<ReduceLoadSize>(test, test, true, false);
+  SinglePassRunAndCheck<ReduceLoadSize>(test, test, true, false,
+                                        kDefaultLoadReductionThreshold);
 }

 TEST_F(ReduceLoadSizeTest, cbuffer_load_5_extract) {
@ -351,7 +360,8 @@ OpFunctionEnd
  SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
  SetDisassembleOptions(SPV_BINARY_TO_TEXT_OPTION_NO_HEADER |
                        SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES);
-  SinglePassRunAndCheck<ReduceLoadSize>(test, test, true, false);
+  SinglePassRunAndCheck<ReduceLoadSize>(test, test, true, false,
+                                        kDefaultLoadReductionThreshold);
 }

 TEST_F(ReduceLoadSizeTest, cbuffer_load_fully_used) {
@ -416,7 +426,76 @@ OpFunctionEnd
  SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
  SetDisassembleOptions(SPV_BINARY_TO_TEXT_OPTION_NO_HEADER |
                        SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES);
-  SinglePassRunAndCheck<ReduceLoadSize>(test, test, true, false);
+  SinglePassRunAndCheck<ReduceLoadSize>(test, test, true, false,
+                                        kDefaultLoadReductionThreshold);
+}
+
+TEST_F(ReduceLoadSizeTest, replace_cbuffer_load_fully_used) {
+  const std::string test =
+      R"(
+               OpCapability Shader
+               OpCapability SampledBuffer
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %main "main" %out_var_SV_Target0
+               OpExecutionMode %main OriginUpperLeft
+               OpSource HLSL 600
+               OpName %type_MaterialInstancing_cbuffer "type.MaterialInstancing_cbuffer"
+               OpMemberName %type_MaterialInstancing_cbuffer 0 "MaterialInstancing_constants"
+               OpName %MaterialInstancing_Constants "MaterialInstancing_Constants"
+               OpMemberName %MaterialInstancing_Constants 0 "offset0"
+               OpMemberName %MaterialInstancing_Constants 1 "params"
+               OpName %InstancingParams_Constants "InstancingParams_Constants"
+               OpMemberName %InstancingParams_Constants 0 "offset1"
+               OpName %MaterialInstancing_cbuffer "MaterialInstancing_cbuffer"
+               OpName %out_var_SV_Target0 "out.var.SV_Target0"
+               OpName %main "main"
+               OpDecorate %out_var_SV_Target0 Location 0
+               OpDecorate %MaterialInstancing_cbuffer DescriptorSet 6
+               OpDecorate %MaterialInstancing_cbuffer Binding 0
+               OpMemberDecorate %InstancingParams_Constants 0 Offset 0
+               OpMemberDecorate %MaterialInstancing_Constants 0 Offset 0
+               OpMemberDecorate %MaterialInstancing_Constants 1 Offset 16
+               OpMemberDecorate %type_MaterialInstancing_cbuffer 0 Offset 0
+               OpDecorate %type_MaterialInstancing_cbuffer Block
+        %int = OpTypeInt 32 1
+      %int_0 = OpConstant %int 0
+      %v4int = OpTypeVector %int 4
+%InstancingParams_Constants = OpTypeStruct %v4int
+%MaterialInstancing_Constants = OpTypeStruct %v4int %InstancingParams_Constants
+%type_MaterialInstancing_cbuffer = OpTypeStruct %MaterialInstancing_Constants
+%_ptr_Uniform_type_MaterialInstancing_cbuffer = OpTypePointer Uniform %type_MaterialInstancing_cbuffer
+%_ptr_Output_int = OpTypePointer Output %int
+       %void = OpTypeVoid
+         %60 = OpTypeFunction %void
+%_ptr_Uniform_MaterialInstancing_Constants = OpTypePointer Uniform %MaterialInstancing_Constants
+%MaterialInstancing_cbuffer = OpVariable %_ptr_Uniform_type_MaterialInstancing_cbuffer Uniform
+%out_var_SV_Target0 = OpVariable %_ptr_Output_int Output
+       %main = OpFunction %void None %60
+         %80 = OpLabel
+        %131 = OpAccessChain %_ptr_Uniform_MaterialInstancing_Constants %MaterialInstancing_cbuffer %int_0
+        %132 = OpLoad %MaterialInstancing_Constants %131
+; CHECK: [[ac1:%\w+]] = OpAccessChain {{%\w+}} %MaterialInstancing_cbuffer %int_0
+; CHECK: [[ac2:%\w+]] = OpAccessChain {{%\w+}} [[ac1]] %uint_0
+; CHECK: OpLoad %v4int [[ac2]]
+
+; CHECK: [[ac3:%\w+]] = OpAccessChain {{%\w+}} [[ac1]] %uint_1
+; CHECK: [[ac4:%\w+]] = OpAccessChain {{%\w+}} [[ac3]] %uint_0
+; CHECK: OpLoad %v4int [[ac4]]
+        %134 = OpCompositeExtract %v4int %132 0
+        %135 = OpCompositeExtract %InstancingParams_Constants %132 1
+        %136 = OpCompositeExtract %v4int %135 0
+        %149 = OpCompositeExtract %int %134 0
+        %185 = OpCompositeExtract %int %136 0
+        %156 = OpIAdd %int %149 %185
+               OpStore %out_var_SV_Target0 %156
+               OpReturn
+               OpFunctionEnd
+  )";
+
+  SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+  SetDisassembleOptions(SPV_BINARY_TO_TEXT_OPTION_NO_HEADER |
+                        SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES);
+  SinglePassRunAndMatch<ReduceLoadSize>(test, false, 1.1);
 }

 }  // namespace
--- a/tools/opt/opt.cpp
+++ b/tools/opt/opt.cpp
@ -387,9 +387,12 @@ Options (in lexicographical order):)",
               Change the scope of private variables that are used in a single
               function to that function.)");
  printf(R"(
-  --reduce-load-size
+  --reduce-load-size[=<threshold>]
               Replaces loads of composite objects where not every component is
-               used by loads of just the elements that are used.)");
+               used by loads of just the elements that are used.  If the ratio
+               of the used components of the load is less than the <threshold>,
+               we replace the load.  <threshold> is a double type number.  If
+               it is bigger than 1.0, we always replaces the load.)");
  printf(R"(
  --redundancy-elimination
               Looks for instructions in the same function that compute the