Remove redundant stores.

The code patterns generated by DXC around function calls can cause many
store to be storing the same value that was just loaded from the same
location:

```
%10 = OpLoad %type %var
OpStore %var %10
```

We want to clean these up very early on because they can cause other
transformations to do a lot of work.  For the cases I see, they can be
removed during local-single-block-elim.

For one set of shaders the compile time goes from 248s to 182s.  A 26%
improvement.

Part of https://github.com/KhronosGroup/SPIRV-Tools/issues/1494.
This commit is contained in:
Steven Perron 2018-05-14 11:51:22 -04:00
parent af430ec822
commit f46f2d3e5d
2 changed files with 139 additions and 6 deletions

View File

@ -73,16 +73,35 @@ bool LocalSingleBlockLoadStoreElimPass::LocalSingleBlockLoadStoreElim(
if (ptrInst->opcode() == SpvOpVariable) {
// If a previous store to same variable, mark the store
// for deletion if not still used.
ir::Instruction* prev_store = var2store_[varId];
if (prev_store != nullptr &&
instructions_to_save.count(prev_store) == 0)
instructions_to_kill.push_back(prev_store);
var2store_[varId] = &*ii;
auto prev_store = var2store_.find(varId);
if (prev_store != var2store_.end() &&
instructions_to_save.count(prev_store->second) == 0) {
instructions_to_kill.push_back(prev_store->second);
}
bool kill_store = false;
auto li = var2load_.find(varId);
if (li != var2load_.end()) {
if (ii->GetSingleWordInOperand(kStoreValIdInIdx) ==
li->second->result_id()) {
// We are storing the same value that already exists in the
// memory location. The store does nothing.
kill_store = true;
}
}
if (!kill_store) {
var2store_[varId] = &*ii;
var2load_.erase(varId);
} else {
instructions_to_kill.push_back(&*ii);
modified = true;
}
} else {
assert(IsNonPtrAccessChain(ptrInst->opcode()));
var2store_.erase(varId);
var2load_.erase(varId);
}
var2load_.erase(varId);
} break;
case SpvOpLoad: {
// Verify store variable is target type

View File

@ -867,6 +867,120 @@ OpFunctionEnd
SinglePassRunAndCheck<opt::LocalSingleBlockLoadStoreElimPass>(before, after,
true, true);
}
TEST_F(LocalSingleBlockLoadStoreElimTest, RedundantStore) {
// Test that checks if a pointer variable is removed.
const std::string predefs_before =
R"(OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main" %BaseColor %gl_FragColor
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 140
OpName %main "main"
OpName %v "v"
OpName %BaseColor "BaseColor"
OpName %gl_FragColor "gl_FragColor"
%void = OpTypeVoid
%7 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%_ptr_Function_v4float = OpTypePointer Function %v4float
%_ptr_Input_v4float = OpTypePointer Input %v4float
%BaseColor = OpVariable %_ptr_Input_v4float Input
%_ptr_Output_v4float = OpTypePointer Output %v4float
%gl_FragColor = OpVariable %_ptr_Output_v4float Output
)";
const std::string before =
R"(%main = OpFunction %void None %7
%13 = OpLabel
%v = OpVariable %_ptr_Function_v4float Function
%14 = OpLoad %v4float %BaseColor
OpStore %v %14
OpBranch %16
%16 = OpLabel
%15 = OpLoad %v4float %v
OpStore %v %15
OpReturn
OpFunctionEnd
)";
const std::string after =
R"(%main = OpFunction %void None %7
%13 = OpLabel
%v = OpVariable %_ptr_Function_v4float Function
%14 = OpLoad %v4float %BaseColor
OpStore %v %14
OpBranch %16
%16 = OpLabel
%15 = OpLoad %v4float %v
OpReturn
OpFunctionEnd
)";
SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
SinglePassRunAndCheck<opt::LocalSingleBlockLoadStoreElimPass>(
predefs_before + before, predefs_before + after, true, true);
}
TEST_F(LocalSingleBlockLoadStoreElimTest, RedundantStore2) {
// Test that checks if a pointer variable is removed.
const std::string predefs_before =
R"(OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main" %BaseColor %gl_FragColor
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 140
OpName %main "main"
OpName %v "v"
OpName %BaseColor "BaseColor"
OpName %gl_FragColor "gl_FragColor"
%void = OpTypeVoid
%7 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%_ptr_Function_v4float = OpTypePointer Function %v4float
%_ptr_Input_v4float = OpTypePointer Input %v4float
%BaseColor = OpVariable %_ptr_Input_v4float Input
%_ptr_Output_v4float = OpTypePointer Output %v4float
%gl_FragColor = OpVariable %_ptr_Output_v4float Output
)";
const std::string before =
R"(%main = OpFunction %void None %7
%13 = OpLabel
%v = OpVariable %_ptr_Function_v4float Function
%14 = OpLoad %v4float %BaseColor
OpStore %v %14
OpBranch %16
%16 = OpLabel
%15 = OpLoad %v4float %v
OpStore %v %15
%17 = OpLoad %v4float %v
OpStore %v %17
OpReturn
OpFunctionEnd
)";
const std::string after =
R"(%main = OpFunction %void None %7
%13 = OpLabel
%v = OpVariable %_ptr_Function_v4float Function
%14 = OpLoad %v4float %BaseColor
OpStore %v %14
OpBranch %16
%16 = OpLabel
%15 = OpLoad %v4float %v
OpReturn
OpFunctionEnd
)";
SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
SinglePassRunAndCheck<opt::LocalSingleBlockLoadStoreElimPass>(
predefs_before + before, predefs_before + after, true, true);
}
// TODO(greg-lunarg): Add tests to verify handling of these cases:
//
// Other target variable types