SingleStore: Support OpVariable Initialization

Treat an OpVariable with initialization as if it was an OpStore.
This fixes issue #1017.

diff --git a/source/opt/local_single_store_elim_pass.cpp b/source/opt/local_single_store_elim_pass.cpp
index 78d1ace..ae9e461 100644 (file)
--- 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 126350a..a73bef3 100644 (file)
--- 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<opt::LocalSingleStoreElimPass>(
+      predefs_before + before, predefs_after + after, true, true);
+}
+
 // TODO(greg-lunarg): Add tests to verify handling of these cases:
 //
 //    Other types