Remove the limit on struct size in SROA.
Removes the limit on scalar replacement for the lagalization passes.
This is done by adding an option to the pass (and command line option)
to set the limit on maximum size of the composite that scalar
replacement is willing to divide.
Fixes #1494.
diff --git a/include/spirv-tools/optimizer.hpp b/include/spirv-tools/optimizer.hpp
index 5630e36..5ec0e6b 100644
--- a/include/spirv-tools/optimizer.hpp
+++ b/include/spirv-tools/optimizer.hpp
@@ -515,8 +515,10 @@
// Create scalar replacement pass.
// This pass replaces composite function scope variables with variables for each
-// element if those elements are accessed individually.
-Optimizer::PassToken CreateScalarReplacementPass();
+// 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);
// Create a private to local pass.
// This pass looks for variables delcared in the private storage class that are
diff --git a/source/opt/optimizer.cpp b/source/opt/optimizer.cpp
index 7c5da10..cf624f3 100644
--- a/source/opt/optimizer.cpp
+++ b/source/opt/optimizer.cpp
@@ -105,7 +105,7 @@
.RegisterPass(CreateLocalSingleStoreElimPass())
.RegisterPass(CreateAggressiveDCEPass())
// Split up aggragates so they are easier to deal with.
- .RegisterPass(CreateScalarReplacementPass())
+ .RegisterPass(CreateScalarReplacementPass(0))
// Remove loads and stores so everything is in intermediate values.
// Takes care of copy propagation of non-members.
.RegisterPass(CreateLocalSingleBlockLoadStoreElimPass())
@@ -416,9 +416,9 @@
MakeUnique<opt::RemoveDuplicatesPass>());
}
-Optimizer::PassToken CreateScalarReplacementPass() {
+Optimizer::PassToken CreateScalarReplacementPass(uint32_t size_limit) {
return MakeUnique<Optimizer::PassToken::Impl>(
- MakeUnique<opt::ScalarReplacementPass>());
+ MakeUnique<opt::ScalarReplacementPass>(size_limit));
}
Optimizer::PassToken CreatePrivateToLocalPass() {
diff --git a/source/opt/scalar_replacement_pass.cpp b/source/opt/scalar_replacement_pass.cpp
index f2d346e..34286da 100644
--- a/source/opt/scalar_replacement_pass.cpp
+++ b/source/opt/scalar_replacement_pass.cpp
@@ -26,9 +26,6 @@
namespace spvtools {
namespace opt {
-// Heuristic aggregate element limit.
-const uint32_t MAX_NUM_ELEMENTS = 100u;
-
Pass::Status ScalarReplacementPass::Process(ir::IRContext* c) {
InitializeProcessing(c);
@@ -540,20 +537,20 @@
case SpvOpTypeStruct:
// Don't bother with empty structs or very large structs.
if (typeInst->NumInOperands() == 0 ||
- typeInst->NumInOperands() > MAX_NUM_ELEMENTS)
+ IsLargerThanSizeLimit(typeInst->NumInOperands()))
return false;
return true;
case SpvOpTypeArray:
- if (GetArrayLength(typeInst) > MAX_NUM_ELEMENTS) return false;
+ if (IsLargerThanSizeLimit(GetArrayLength(typeInst))) return false;
return true;
- // TODO(alanbaker): Develop some heuristics for when this should be
- // re-enabled.
- //// Specifically including matrix and vector in an attempt to reduce the
- //// number of vector registers required.
- // case SpvOpTypeMatrix:
- // case SpvOpTypeVector:
- // if (GetNumElements(typeInst) > MAX_NUM_ELEMENTS) return false;
- // return true;
+ // TODO(alanbaker): Develop some heuristics for when this should be
+ // re-enabled.
+ //// Specifically including matrix and vector in an attempt to reduce the
+ //// number of vector registers required.
+ // case SpvOpTypeMatrix:
+ // case SpvOpTypeVector:
+ // if (IsLargerThanSizeLimit(GetNumElements(typeInst))) return false;
+ // return true;
case SpvOpTypeRuntimeArray:
default:
@@ -716,6 +713,12 @@
return false;
return true;
}
+bool ScalarReplacementPass::IsLargerThanSizeLimit(size_t length) const {
+ if (max_num_elements_ == 0) {
+ return false;
+ }
+ return length > max_num_elements_;
+}
std::unique_ptr<std::unordered_set<uint64_t>>
ScalarReplacementPass::GetUsedComponents(ir::Instruction* inst) {
diff --git a/source/opt/scalar_replacement_pass.h b/source/opt/scalar_replacement_pass.h
index e659b3f..6a02955 100644
--- a/source/opt/scalar_replacement_pass.h
+++ b/source/opt/scalar_replacement_pass.h
@@ -15,6 +15,8 @@
#ifndef LIBSPIRV_OPT_SCALAR_REPLACEMENT_PASS_H_
#define LIBSPIRV_OPT_SCALAR_REPLACEMENT_PASS_H_
+#include <cstdio>
+
#include "function.h"
#include "pass.h"
#include "type_manager.h"
@@ -26,10 +28,18 @@
// Documented in optimizer.hpp
class ScalarReplacementPass : public Pass {
- public:
- ScalarReplacementPass() = default;
+ private:
+ static const uint32_t kDefaultLimit = 100;
- const char* name() const override { return "scalar-replacement"; }
+ public:
+ ScalarReplacementPass(uint32_t limit = kDefaultLimit)
+ : max_num_elements_(limit) {
+ name_[0] = '\0';
+ strcat(name_, "scalar-replacement=");
+ sprintf(&name_[strlen(name_)], "%d", max_num_elements_);
+ }
+
+ const char* name() const override { return name_; }
// Attempts to scalarize all appropriate function scope variables. Returns
// SuccessWithChange if any change is made.
@@ -207,6 +217,12 @@
// Maps type id to OpConstantNull for that type.
std::unordered_map<uint32_t, uint32_t> type_to_null_;
+
+ // Limit on the number of members in an object that will be replaced.
+ // 0 means there is no limit.
+ uint32_t max_num_elements_;
+ bool IsLargerThanSizeLimit(size_t length) const;
+ char name_[55];
};
} // namespace opt
diff --git a/test/opt/scalar_replacement_test.cpp b/test/opt/scalar_replacement_test.cpp
index f1dd0f2..6a44b9c 100644
--- a/test/opt/scalar_replacement_test.cpp
+++ b/test/opt/scalar_replacement_test.cpp
@@ -1358,4 +1358,76 @@
}
#endif // SPIRV_EFFCEE
+// Test that a struct of size 4 is not replaced when there is a limit of 2.
+TEST_F(ScalarReplacementTest, TestLimit) {
+ const std::string text = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpName %6 "simple_struct"
+%1 = OpTypeVoid
+%2 = OpTypeInt 32 0
+%3 = OpTypeStruct %2 %2 %2 %2
+%4 = OpTypePointer Function %3
+%5 = OpTypePointer Function %2
+%6 = OpTypeFunction %2
+%7 = OpConstantNull %3
+%8 = OpConstant %2 0
+%9 = OpConstant %2 1
+%10 = OpConstant %2 2
+%11 = OpConstant %2 3
+%12 = OpFunction %2 None %6
+%13 = OpLabel
+%14 = OpVariable %4 Function %7
+%15 = OpInBoundsAccessChain %5 %14 %8
+%16 = OpLoad %2 %15
+%17 = OpAccessChain %5 %14 %10
+%18 = OpLoad %2 %17
+%19 = OpIAdd %2 %16 %18
+OpReturnValue %19
+OpFunctionEnd
+ )";
+
+ auto result = SinglePassRunAndDisassemble<opt::ScalarReplacementPass>(
+ text, true, false, 2);
+ EXPECT_EQ(opt::Pass::Status::SuccessWithoutChange, std::get<1>(result));
+}
+
+// Test that a struct of size 4 is replaced when there is a limit of 0 (no
+// limit). This is the same spir-v as a test above, so we do not check that it
+// is correctly transformed. We leave that to the test above.
+TEST_F(ScalarReplacementTest, TestUnimited) {
+ const std::string text = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+OpName %6 "simple_struct"
+%1 = OpTypeVoid
+%2 = OpTypeInt 32 0
+%3 = OpTypeStruct %2 %2 %2 %2
+%4 = OpTypePointer Function %3
+%5 = OpTypePointer Function %2
+%6 = OpTypeFunction %2
+%7 = OpConstantNull %3
+%8 = OpConstant %2 0
+%9 = OpConstant %2 1
+%10 = OpConstant %2 2
+%11 = OpConstant %2 3
+%12 = OpFunction %2 None %6
+%13 = OpLabel
+%14 = OpVariable %4 Function %7
+%15 = OpInBoundsAccessChain %5 %14 %8
+%16 = OpLoad %2 %15
+%17 = OpAccessChain %5 %14 %10
+%18 = OpLoad %2 %17
+%19 = OpIAdd %2 %16 %18
+OpReturnValue %19
+OpFunctionEnd
+ )";
+
+ auto result = SinglePassRunAndDisassemble<opt::ScalarReplacementPass>(
+ text, true, false, 0);
+ EXPECT_EQ(opt::Pass::Status::SuccessWithChange, std::get<1>(result));
+}
+
} // namespace
diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp
index 0d96566..38aa36e 100644
--- a/tools/opt/opt.cpp
+++ b/tools/opt/opt.cpp
@@ -292,10 +292,12 @@
--ssa-rewrite
Replace loads and stores to function local variables with
operations on SSA IDs.
- --scalar-replacement
+ --scalar-replacement[=<n>]
Replace aggregate function scope variables that are only accessed
via their elements with new function variables representing each
- element.
+ element. <n> is a limit on the size of the aggragates that will
+ be replaced. 0 means there is no limit. The default value is
+ 100.
--set-spec-const-default-value "<spec id>:<default value> ..."
Set the default values of the specialization constants with
<spec id>:<default value> pairs specified in a double-quoted
@@ -569,6 +571,9 @@
optimizer->RegisterPass(CreateLoopUnswitchPass());
} else if (0 == strcmp(cur_arg, "--scalar-replacement")) {
optimizer->RegisterPass(CreateScalarReplacementPass());
+ } else if (0 == strncmp(cur_arg, "--scalar-replacement=", 21)) {
+ uint32_t limit = atoi(cur_arg + 21);
+ optimizer->RegisterPass(CreateScalarReplacementPass(limit));
} else if (0 == strcmp(cur_arg, "--strength-reduction")) {
optimizer->RegisterPass(CreateStrengthReductionPass());
} else if (0 == strcmp(cur_arg, "--unify-const")) {
