Maintain instruction to block mapping in phi insertion

* Changed MemPass::InsertPhiInstructions to set basic blocks for new
phis
* Local SSA elim now maintains instr to block mapping
 * Added a test and confirmed it fails without the updated phis
* IRContext::set_instr_block no longer builds the map if the analysis is
invalid
* Added instruction to block mapping verification to
IRContext::IsConsistent()

diff --git a/source/opt/ir_context.cpp b/source/opt/ir_context.cpp
index 9ab969a..1e2da41 100644 (file)
--- a/source/opt/ir_context.cpp
+++ b/source/opt/ir_context.cpp
@@ -181,6 +181,19 @@ bool IRContext::IsConsistent() {
       return false;
     }
   }
+  if (AreAnalysesValid(kAnalysisInstrToBlockMapping)) {
+    for (auto& func : *module()) {
+      for (auto& block : func) {
+        bool ok = true;
+        block.ForEachInst([this, &block, &ok](ir::Instruction* inst) {
+          if (get_instr_block(inst) != &block) {
+            ok = false;
+          }
+        });
+        if (!ok) return false;
+      }
+    }
+  }
   return true;
 }
 

diff --git a/source/opt/ir_context.h b/source/opt/ir_context.h
index 9a56053..98e1433 100644 (file)
--- a/source/opt/ir_context.h
+++ b/source/opt/ir_context.h
@@ -224,8 +224,6 @@ class IRContext {
   void set_instr_block(ir::Instruction* inst, ir::BasicBlock* block) {
     if (AreAnalysesValid(kAnalysisInstrToBlockMapping)) {
       instr_to_block_[inst] = block;
-    } else {
-      BuildInstrToBlockMapping();
     }
   }
 

diff --git a/source/opt/local_ssa_elim_pass.h b/source/opt/local_ssa_elim_pass.h
index 12b107d..326f938 100644 (file)
--- a/source/opt/local_ssa_elim_pass.h
+++ b/source/opt/local_ssa_elim_pass.h
@@ -45,7 +45,8 @@ class LocalMultiStoreElimPass : public MemPass {
   Status Process(ir::IRContext* c) override;
 
   ir::IRContext::Analysis GetPreservedAnalyses() override {
-    return ir::IRContext::kAnalysisDefUse;
+    return ir::IRContext::kAnalysisDefUse |
+           ir::IRContext::kAnalysisInstrToBlockMapping;
   }
 
  private:

diff --git a/source/opt/mem_pass.cpp b/source/opt/mem_pass.cpp
index b389052..7177939 100644 (file)
--- a/source/opt/mem_pass.cpp
+++ b/source/opt/mem_pass.cpp
@@ -425,6 +425,7 @@ void MemPass::SSABlockInitLoopHeader(
     // Only analyze the phi define now; analyze the phi uses after the
     // phi backedge predecessor value is patched.
     get_def_use_mgr()->AnalyzeInstDef(&*newPhi);
+    context()->set_instr_block(&*newPhi, *block_itr);
     insertItr = insertItr.InsertBefore(std::move(newPhi));
     ++insertItr;
     label2ssa_map_[label].insert({varId, phiId});
@@ -489,6 +490,7 @@ void MemPass::SSABlockInitMultiPred(ir::BasicBlock* block_ptr) {
     std::unique_ptr<ir::Instruction> newPhi(new ir::Instruction(
         context(), SpvOpPhi, typeId, phiId, phi_in_operands));
     get_def_use_mgr()->AnalyzeInstDefUse(&*newPhi);
+    context()->set_instr_block(&*newPhi, block_ptr);
     insertItr = insertItr.InsertBefore(std::move(newPhi));
     ++insertItr;
     label2ssa_map_[label].insert({varId, phiId});

diff --git a/test/opt/local_ssa_elim_test.cpp b/test/opt/local_ssa_elim_test.cpp
index 942a698..b6f282a 100644 (file)
--- a/test/opt/local_ssa_elim_test.cpp
+++ b/test/opt/local_ssa_elim_test.cpp
@@ -1612,6 +1612,98 @@ OpFunctionEnd
                                                       true);
 }
 
+TEST_F(LocalSSAElimTest, VerifyInstToBlockMap) {
+  // #version 140
+  //
+  // in vec4 BC;
+  // out float fo;
+  //
+  // void main()
+  // {
+  //     float f = 0.0;
+  //     for (int i=0; i<4; i++) {
+  //       f = f + BC[i];
+  //     }
+  //     fo = f;
+  // }
+
+  const std::string text = R"(
+OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %BC %fo
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 140
+OpName %main "main"
+OpName %f "f"
+OpName %i "i"
+OpName %BC "BC"
+OpName %fo "fo"
+%void = OpTypeVoid
+%8 = OpTypeFunction %void
+%float = OpTypeFloat 32
+%_ptr_Function_float = OpTypePointer Function %float
+%float_0 = OpConstant %float 0
+%int = OpTypeInt 32 1
+%_ptr_Function_int = OpTypePointer Function %int
+%int_0 = OpConstant %int 0
+%int_4 = OpConstant %int 4
+%bool = OpTypeBool
+%v4float = OpTypeVector %float 4
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%BC = OpVariable %_ptr_Input_v4float Input
+%_ptr_Input_float = OpTypePointer Input %float
+%int_1 = OpConstant %int 1
+%_ptr_Output_float = OpTypePointer Output %float
+%fo = OpVariable %_ptr_Output_float Output
+%main = OpFunction %void None %8
+%22 = OpLabel
+%f = OpVariable %_ptr_Function_float Function
+%i = OpVariable %_ptr_Function_int Function
+OpStore %f %float_0
+OpStore %i %int_0
+OpBranch %23
+%23 = OpLabel
+OpLoopMerge %24 %25 None
+OpBranch %26
+%26 = OpLabel
+%27 = OpLoad %int %i
+%28 = OpSLessThan %bool %27 %int_4
+OpBranchConditional %28 %29 %24
+%29 = OpLabel
+%30 = OpLoad %float %f
+%31 = OpLoad %int %i
+%32 = OpAccessChain %_ptr_Input_float %BC %31
+%33 = OpLoad %float %32
+%34 = OpFAdd %float %30 %33
+OpStore %f %34
+OpBranch %25
+%25 = OpLabel
+%35 = OpLoad %int %i
+%36 = OpIAdd %int %35 %int_1
+OpStore %i %36
+OpBranch %23
+%24 = OpLabel
+%37 = OpLoad %float %f
+OpStore %fo %37
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<ir::IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+  EXPECT_NE(nullptr, context);
+
+  // Force the instruction to block mapping to get built.
+  context->get_instr_block(27u);
+
+  auto pass = MakeUnique<opt::LocalMultiStoreElimPass>();
+  pass->SetMessageConsumer(nullptr);
+  const auto status = pass->Run(context.get());
+  EXPECT_TRUE(status == opt::Pass::Status::SuccessWithChange);
+}
+
 // TODO(greg-lunarg): Add tests to verify handling of these cases:
 //
 //    No optimization in the presence of