From c6fdf68c2fe626895add8cbae749050806d6ee4e Mon Sep 17 00:00:00 2001 From: GregF Date: Thu, 7 Dec 2017 12:11:29 -0700 Subject: [PATCH] SingleStore: Support OpVariable Initialization Treat an OpVariable with initialization as if it was an OpStore. This fixes issue #1017. --- source/opt/local_single_store_elim_pass.cpp | 90 ++++++++++++--------- test/opt/local_single_store_elim_test.cpp | 78 ++++++++++++++++++ 2 files changed, 131 insertions(+), 37 deletions(-) diff --git a/source/opt/local_single_store_elim_pass.cpp b/source/opt/local_single_store_elim_pass.cpp index 78d1ace0..ae9e461e 100644 --- a/source/opt/local_single_store_elim_pass.cpp +++ b/source/opt/local_single_store_elim_pass.cpp @@ -26,6 +26,7 @@ namespace opt { namespace { const uint32_t kStoreValIdInIdx = 1; +const uint32_t kVariableInitIdInIdx = 1; } // anonymous namespace @@ -58,41 +59,50 @@ void LocalSingleStoreElimPass::SingleStoreAnalyze(ir::Function* func) { for (auto bi = func->begin(); bi != func->end(); ++bi) { uint32_t instIdx = 0; for (auto ii = bi->begin(); ii != bi->end(); ++ii, ++instIdx) { + uint32_t varId = 0; + ir::Instruction* ptrInst; switch (ii->opcode()) { case SpvOpStore: { - // Verify store variable is target type - uint32_t varId; - ir::Instruction* ptrInst = GetPtr(&*ii, &varId); - if (non_ssa_vars_.find(varId) != non_ssa_vars_.end()) continue; - if (ptrInst->opcode() != SpvOpVariable) { - non_ssa_vars_.insert(varId); - ssa_var2store_.erase(varId); - continue; - } - // Verify target type and function storage class - if (!IsTargetVar(varId)) { - non_ssa_vars_.insert(varId); - continue; - } - if (!HasOnlySupportedRefs(varId)) { - non_ssa_vars_.insert(varId); - continue; - } - // Ignore variables with multiple stores - if (ssa_var2store_.find(varId) != ssa_var2store_.end()) { - non_ssa_vars_.insert(varId); - ssa_var2store_.erase(varId); - continue; + ptrInst = GetPtr(&*ii, &varId); + } break; + case SpvOpVariable: { + // If initializer, treat like store + if (ii->NumInOperands() > 1) { + varId = ii->result_id(); + ptrInst = &*ii; } - // Remember pointer to variable's store and it's - // ordinal position in block - ssa_var2store_[varId] = &*ii; - store2idx_[&*ii] = instIdx; - store2blk_[&*ii] = &*bi; } break; default: break; } // switch + if (varId == 0) continue; + // Verify variable is target type + if (non_ssa_vars_.find(varId) != non_ssa_vars_.end()) continue; + if (ptrInst->opcode() != SpvOpVariable) { + non_ssa_vars_.insert(varId); + ssa_var2store_.erase(varId); + continue; + } + // Verify target type and function storage class + if (!IsTargetVar(varId)) { + non_ssa_vars_.insert(varId); + continue; + } + if (!HasOnlySupportedRefs(varId)) { + non_ssa_vars_.insert(varId); + continue; + } + // Ignore variables with multiple stores + if (ssa_var2store_.find(varId) != ssa_var2store_.end()) { + non_ssa_vars_.insert(varId); + ssa_var2store_.erase(varId); + continue; + } + // Remember pointer to variable's store and it's + // ordinal position in block + ssa_var2store_[varId] = &*ii; + store2idx_[&*ii] = instIdx; + store2blk_[&*ii] = &*bi; } } } @@ -188,9 +198,14 @@ bool LocalSingleStoreElimPass::SingleStoreProcess(ir::Function* func) { if (!Dominates(store2blk_[vsi->second], store2idx_[vsi->second], &*bi, instIdx)) continue; - // Use store value as replacement id - uint32_t replId = vsi->second->GetSingleWordInOperand(kStoreValIdInIdx); - // replace all instances of the load's id with the SSA value's id + // Determine replacement id depending on OpStore or OpVariable + uint32_t replId; + if (vsi->second->opcode() == SpvOpStore) + replId = vsi->second->GetSingleWordInOperand(kStoreValIdInIdx); + else + replId = vsi->second->GetSingleWordInOperand(kVariableInitIdInIdx); + // Replace all instances of the load's id with the SSA value's id + // and add load to removal list context()->KillNamesAndDecorates(&*ii); context()->ReplaceAllUsesWith(ii->result_id(), replId); dead_instructions.push_back(&*ii); @@ -208,13 +223,14 @@ bool LocalSingleStoreElimPass::SingleStoreProcess(ir::Function* func) { dead_instructions.erase(i); } - // Update the variable to store map. - if (other_inst->opcode() != SpvOpStore) { + // Update the variable-to-store map if any of its members is DCE'd. + uint32_t id; + if (other_inst->opcode() == SpvOpStore) GetPtr(other_inst, &id); + if (other_inst->opcode() == SpvOpVariable) + id = other_inst->result_id(); + else return; - } - uint32_t id; - GetPtr(other_inst, &id); auto store = ssa_var2store_.find(id); if (store != ssa_var2store_.end()) { ssa_var2store_.erase(store); @@ -237,7 +253,7 @@ bool LocalSingleStoreElimPass::SingleStoreDCE() { // check that it hasn't already been DCE'd if (already_deleted.find(v.second) != already_deleted.end()) continue; if (non_ssa_vars_.find(v.first) != non_ssa_vars_.end()) continue; - if (!IsLiveStore(v.second)) { + if (!IsLiveVar(v.first)) { DCEInst(v.second, [&already_deleted](ir::Instruction* inst) { already_deleted.insert(inst); }); diff --git a/test/opt/local_single_store_elim_test.cpp b/test/opt/local_single_store_elim_test.cpp index 126350a7..a73bef30 100644 --- a/test/opt/local_single_store_elim_test.cpp +++ b/test/opt/local_single_store_elim_test.cpp @@ -644,6 +644,84 @@ OpFunctionEnd true); } +TEST_F(LocalSingleStoreElimTest, OptInitializedVariableLikeStore) { + // Initialized variable f is optimized like it was a store. + // Note: The SPIR-V was edited to turn the store to f to an + // an initialization. + // + // #version 140 + // + // void main() + // { + // float f = 0.0; + // gl_FragColor = vec4(f); + // } + + const std::string predefs_before = + R"(OpCapability Shader +%1 = OpExtInstImport "GLSL.std.450" +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" %gl_FragColor +OpExecutionMode %main OriginUpperLeft +OpSource GLSL 140 +OpName %main "main" +OpName %f "f" +OpName %gl_FragColor "gl_FragColor" +OpDecorate %gl_FragColor Location 0 +%void = OpTypeVoid +%6 = OpTypeFunction %void +%float = OpTypeFloat 32 +%_ptr_Function_float = OpTypePointer Function %float +%float_0 = OpConstant %float 0 +%v4float = OpTypeVector %float 4 +%_ptr_Output_v4float = OpTypePointer Output %v4float +%gl_FragColor = OpVariable %_ptr_Output_v4float Output +)"; + + const std::string predefs_after = + R"(OpCapability Shader +%1 = OpExtInstImport "GLSL.std.450" +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" %gl_FragColor +OpExecutionMode %main OriginUpperLeft +OpSource GLSL 140 +OpName %main "main" +OpName %gl_FragColor "gl_FragColor" +OpDecorate %gl_FragColor Location 0 +%void = OpTypeVoid +%6 = OpTypeFunction %void +%float = OpTypeFloat 32 +%_ptr_Function_float = OpTypePointer Function %float +%float_0 = OpConstant %float 0 +%v4float = OpTypeVector %float 4 +%_ptr_Output_v4float = OpTypePointer Output %v4float +%gl_FragColor = OpVariable %_ptr_Output_v4float Output +)"; + + const std::string before = + R"(%main = OpFunction %void None %6 +%12 = OpLabel +%f = OpVariable %_ptr_Function_float Function %float_0 +%13 = OpLoad %float %f +%14 = OpCompositeConstruct %v4float %13 %13 %13 %13 +OpStore %gl_FragColor %14 +OpReturn +OpFunctionEnd +)"; + + const std::string after = + R"(%main = OpFunction %void None %6 +%12 = OpLabel +%14 = OpCompositeConstruct %v4float %float_0 %float_0 %float_0 %float_0 +OpStore %gl_FragColor %14 +OpReturn +OpFunctionEnd +)"; + + SinglePassRunAndCheck( + predefs_before + before, predefs_after + after, true, true); +} + // TODO(greg-lunarg): Add tests to verify handling of these cases: // // Other types -- 2.34.1