BlockMerge: Add BlockMergePass

Also, add BasicBlock::tail()

include/spirv-tools/optimizer.hpp

@@ -185,6 +185,22 @@ Optimizer::PassToken CreateUnifyConstantPass();
 // OpSpecConstantOp.
 Optimizer::PassToken CreateEliminateDeadConstantPass();
 
+// Creates a block merge pass.
+// This pass searches for blocks with a single Branch to a block with no
+// other predecessors and merges the blocks into a single block. Continue
+// blocks and Merge blocks are not candidates for the second block.
+//
+// The pass is most useful after Dead Branch Elimination, which can leave
+// such sequences of blocks. Merging them makes subsequent passes more
+// effective, such as single block local store-load elimination.
+//
+// While this pass reduces the number of occurrences of this sequence, at
+// this time it does not guarantee all such sequences are eliminated.
+//
+// Presence of phi instructions can inhibit this optimization. Handling
+// these is left for future improvements. 
+Optimizer::PassToken CreateBlockMergePass();
+
 // Creates an inline pass.
 // An inline pass exhaustively inlines all function calls in all functions
 // designated as an entry point. The intent is to enable, albeit through

source/opt/CMakeLists.txt

@@ -13,6 +13,7 @@
 # limitations under the License.
 add_library(SPIRV-Tools-opt
   basic_block.h
+  block_merge_pass.h
   build_module.h
   compact_ids_pass.h
   constants.h
@@ -43,6 +44,7 @@ add_library(SPIRV-Tools-opt
   unify_const_pass.h
 
   basic_block.cpp
+  block_merge_pass.cpp
   build_module.cpp
   compact_ids_pass.cpp
   def_use_manager.cpp

source/opt/basic_block.h

@@ -42,10 +42,15 @@ class BasicBlock {
 
   // Sets the enclosing function for this basic block.
   void SetParent(Function* function) { function_ = function; }
+
   // Appends an instruction to this basic block.
   inline void AddInstruction(std::unique_ptr<Instruction> i);
+
+  // Appends all of block's instructions (except label) to this block
+  inline void AddInstructions(BasicBlock* bp);
+
   // The label starting this basic block.
-  Instruction& Label() { return *label_; }
+  Instruction* GetLabelInst() { return label_.get(); }
 
   // Returns the id of the label at the top of this block
   inline uint32_t id() const { return label_->result_id(); }
@@ -59,6 +64,11 @@ class BasicBlock {
     return const_iterator(&insts_, insts_.cend());
   }
 
+  iterator tail() {
+    assert(!insts_.empty());
+    return iterator(&insts_, std::prev(insts_.end()));
+  }
+
   // Runs the given function |f| on each instruction in this basic block, and
   // optionally on the debug line instructions that might precede them.
   inline void ForEachInst(const std::function<void(Instruction*)>& f,
@@ -91,6 +101,11 @@ inline void BasicBlock::AddInstruction(std::unique_ptr<Instruction> i) {
   insts_.emplace_back(std::move(i));
 }
 
+inline void BasicBlock::AddInstructions(BasicBlock* bp) {
+  auto bEnd = end();
+  (void) bEnd.InsertBefore(&bp->insts_);
+}
+
 inline void BasicBlock::ForEachInst(const std::function<void(Instruction*)>& f,
                                     bool run_on_debug_line_insts) {
   if (label_) label_->ForEachInst(f, run_on_debug_line_insts);

source/opt/block_merge_pass.cpp

@@ -0,0 +1,143 @@
+// Copyright (c) 2017 The Khronos Group Inc.
+// Copyright (c) 2017 Valve Corporation
+// Copyright (c) 2017 LunarG Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "block_merge_pass.h"
+
+#include "iterator.h"
+
+namespace spvtools {
+namespace opt {
+
+namespace {
+
+const int kEntryPointFunctionIdInIdx = 1;
+
+} // anonymous namespace
+
+bool BlockMergePass::IsLoopHeader(ir::BasicBlock* block_ptr) {
+  auto iItr = block_ptr->tail();
+  if (iItr == block_ptr->begin())
+    return false;
+  --iItr;
+  return iItr->opcode() == SpvOpLoopMerge;
+}
+
+bool BlockMergePass::HasMultipleRefs(uint32_t labId) {
+  const analysis::UseList* uses = def_use_mgr_->GetUses(labId);
+  int rcnt = 0;
+  for (const auto u : *uses) {
+    // Don't count OpName
+    if (u.inst->opcode() == SpvOpName)
+      continue;
+    if (rcnt == 1)
+      return true;
+    ++rcnt;
+  }
+  return false;
+}
+
+void BlockMergePass::KillInstAndName(ir::Instruction* inst) {
+  const uint32_t id = inst->result_id();
+  if (id != 0) {
+    analysis::UseList* uses = def_use_mgr_->GetUses(id);
+    if (uses != nullptr)
+      for (auto u : *uses)
+        if (u.inst->opcode() == SpvOpName) {
+          def_use_mgr_->KillInst(u.inst);
+          break;
+        }
+  }
+  def_use_mgr_->KillInst(inst);
+}
+
+bool BlockMergePass::MergeBlocks(ir::Function* func) {
+  bool modified = false;
+  for (auto bi = func->begin(); bi != func->end(); ) {
+    // Do not merge loop header blocks, at least for now.
+    if (IsLoopHeader(&*bi)) {
+      ++bi;
+      continue;
+    }
+    // Find block with single successor which has no other predecessors.
+    // Continue and Merge blocks are currently ruled out as second blocks.
+    // Happily any such candidate blocks will have >1 uses due to their
+    // LoopMerge instruction.
+    // TODO(): Deal with phi instructions that reference the
+    // second block. Happily, these references currently inhibit
+    // the merge.
+    auto ii = bi->end();
+    --ii;
+    ir::Instruction* br = &*ii;
+    if (br->opcode() != SpvOpBranch) {
+      ++bi;
+      continue;
+    }
+    const uint32_t labId = br->GetSingleWordInOperand(0);
+    if (HasMultipleRefs(labId)) {
+      ++bi;
+      continue;
+    }
+    // Merge blocks
+    def_use_mgr_->KillInst(br);
+    auto sbi = bi;
+    for (; sbi != func->end(); ++sbi)
+      if (sbi->id() == labId)
+        break;
+    // If bi is sbi's only predecessor, it dominates sbi and thus
+    // sbi must follow bi in func's ordering.
+    assert(sbi != func->end());
+    bi->AddInstructions(&*sbi);
+    KillInstAndName(sbi->GetLabelInst());
+    (void) sbi.Erase();
+    // reprocess block
+    modified = true;
+  }
+  return modified;
+}
+
+void BlockMergePass::Initialize(ir::Module* module) {
+
+  module_ = module;
+
+  // Initialize function and block maps
+  id2function_.clear();
+  for (auto& fn : *module_) 
+    id2function_[fn.result_id()] = &fn;
+
+  def_use_mgr_.reset(new analysis::DefUseManager(consumer(), module_));
+};
+
+Pass::Status BlockMergePass::ProcessImpl() {
+  bool modified = false;
+  for (auto& e : module_->entry_points()) {
+    ir::Function* fn =
+        id2function_[e.GetSingleWordInOperand(kEntryPointFunctionIdInIdx)];
+    modified = MergeBlocks(fn) || modified;
+  }
+  return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange;
+}
+
+BlockMergePass::BlockMergePass()
+    : module_(nullptr), def_use_mgr_(nullptr) {}
+
+Pass::Status BlockMergePass::Process(ir::Module* module) {
+  Initialize(module);
+  return ProcessImpl();
+}
+
+}  // namespace opt
+}  // namespace spvtools
+

source/opt/block_merge_pass.h

@@ -0,0 +1,73 @@
+// Copyright (c) 2017 The Khronos Group Inc.
+// Copyright (c) 2017 Valve Corporation
+// Copyright (c) 2017 LunarG Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef LIBSPIRV_OPT_BLOCK_MERGE_PASS_H_
+#define LIBSPIRV_OPT_BLOCK_MERGE_PASS_H_
+
+#include <algorithm>
+#include <map>
+#include <queue>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+
+#include "basic_block.h"
+#include "def_use_manager.h"
+#include "module.h"
+#include "pass.h"
+
+namespace spvtools {
+namespace opt {
+
+// See optimizer.hpp for documentation.
+class BlockMergePass : public Pass {
+ public:
+  BlockMergePass();
+  const char* name() const override { return "sroa"; }
+  Status Process(ir::Module*) override;
+
+ private:
+  // Return true if |block_ptr| is loop header block
+  bool IsLoopHeader(ir::BasicBlock* block_ptr);
+
+  // Return true if |labId| has multiple refs. Do not count OpName.
+  bool HasMultipleRefs(uint32_t labId);
+
+  // Kill any OpName instruction referencing |inst|, then kill |inst|.
+  void KillInstAndName(ir::Instruction* inst);
+
+  // Search |func| for blocks which have a single Branch to a block
+  // with no other predecessors. Merge these blocks into a single block.
+  bool MergeBlocks(ir::Function* func);
+
+  void Initialize(ir::Module* module);
+  Pass::Status ProcessImpl();
+
+  // Module this pass is processing
+  ir::Module* module_;
+
+  // Def-Uses for the module we are processing
+  std::unique_ptr<analysis::DefUseManager> def_use_mgr_;
+
+  // Map from function's result id to function
+  std::unordered_map<uint32_t, ir::Function*> id2function_;
+};
+
+}  // namespace opt
+}  // namespace spvtools
+
+#endif  // LIBSPIRV_OPT_BLOCK_MERGE_PASS_H_
+

source/opt/optimizer.cpp

@@ -132,6 +132,11 @@ Optimizer::PassToken CreateEliminateDeadConstantPass() {
       MakeUnique<opt::EliminateDeadConstantPass>());
 }
 
+Optimizer::PassToken CreateBlockMergePass() {
+  return MakeUnique<Optimizer::PassToken::Impl>(
+      MakeUnique<opt::BlockMergePass>());
+}
+
 Optimizer::PassToken CreateInlinePass() {
   return MakeUnique<Optimizer::PassToken::Impl>(MakeUnique<opt::InlinePass>());
 }

source/opt/passes.h

@@ -17,6 +17,7 @@
 
 // A single header to include all passes.
 
+#include "block_merge_pass.h"
 #include "compact_ids_pass.h"
 #include "eliminate_dead_constant_pass.h"
 #include "flatten_decoration_pass.h"

test/opt/CMakeLists.txt

@@ -53,6 +53,11 @@ add_spvtools_unittest(TARGET pass_freeze_spec_const
   LIBS SPIRV-Tools-opt
 )
 
+add_spvtools_unittest(TARGET pass_block_merge
+  SRCS block_merge_test.cpp pass_utils.cpp
+  LIBS SPIRV-Tools-opt
+)
+
 add_spvtools_unittest(TARGET pass_inline
   SRCS inline_test.cpp pass_utils.cpp
   LIBS SPIRV-Tools-opt

test/opt/block_merge_test.cpp

@@ -0,0 +1,337 @@
+// Copyright (c) 2017 Valve Corporation
+// Copyright (c) 2017 LunarG Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "pass_fixture.h"
+#include "pass_utils.h"
+
+namespace {
+
+using namespace spvtools;
+
+using BlockMergeTest = PassTest<::testing::Test>;
+
+TEST_F(BlockMergeTest, Simple) {
+  // Note: SPIR-V hand edited to insert block boundary
+  // between two statements in main.
+  //
+  //  #version 140
+  //  
+  //  in vec4 BaseColor;
+  //  
+  //  void main()
+  //  {
+  //      vec4 v = BaseColor;
+  //      gl_FragColor = v;
+  //  }
+
+  const std::string predefs =
+      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 %15
+%15 = OpLabel
+%16 = OpLoad %v4float %v
+OpStore %gl_FragColor %16
+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
+%16 = OpLoad %v4float %v
+OpStore %gl_FragColor %16
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndCheck<opt::BlockMergePass>(
+      predefs + before, predefs + after, true, true);
+}
+
+TEST_F(BlockMergeTest, EmptyBlock) {
+  // Note: SPIR-V hand edited to insert empty block
+  // after two statements in main.
+  //
+  //  #version 140
+  //  
+  //  in vec4 BaseColor;
+  //  
+  //  void main()
+  //  {
+  //      vec4 v = BaseColor;
+  //      gl_FragColor = v;
+  //  }
+
+  const std::string predefs =
+      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 %15
+%15 = OpLabel
+%16 = OpLoad %v4float %v
+OpStore %gl_FragColor %16
+OpBranch %17
+%17 = OpLabel
+OpBranch %18
+%18 = OpLabel
+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
+%16 = OpLoad %v4float %v
+OpStore %gl_FragColor %16
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndCheck<opt::BlockMergePass>(
+      predefs + before, predefs + after, true, true);
+}
+
+TEST_F(BlockMergeTest, NoOptOfMergeOrContinueBlock) {
+  // Note: SPIR-V hand edited remove dead branch and add block
+  // before continue block
+  //
+  // #version 140
+  // in vec4 BaseColor;
+  // 
+  // void main()
+  // {
+  //     while (true) {
+  //         break;
+  //     }
+  //     gl_FragColor = BaseColor;
+  // }
+
+  const std::string assembly =
+      R"(OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %gl_FragColor %BaseColor
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 140
+OpName %main "main"
+OpName %gl_FragColor "gl_FragColor"
+OpName %BaseColor "BaseColor"
+%void = OpTypeVoid
+%6 = OpTypeFunction %void
+%bool = OpTypeBool
+%true = OpConstantTrue %bool
+%float = OpTypeFloat 32
+%v4float = OpTypeVector %float 4
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%gl_FragColor = OpVariable %_ptr_Output_v4float Output
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%BaseColor = OpVariable %_ptr_Input_v4float Input
+%main = OpFunction %void None %6
+%13 = OpLabel
+OpBranch %14
+%14 = OpLabel
+OpLoopMerge %15 %16 None
+OpBranch %17
+%17 = OpLabel
+OpBranch %15
+%18 = OpLabel
+OpBranch %16
+%16 = OpLabel
+OpBranch %14
+%15 = OpLabel
+%19 = OpLoad %v4float %BaseColor
+OpStore %gl_FragColor %19
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndCheck<opt::BlockMergePass>(
+      assembly, assembly, true, true);
+}
+
+TEST_F(BlockMergeTest, NestedInControlFlow) {
+  // Note: SPIR-V hand edited to insert block boundary
+  // between OpFMul and OpStore in then-part.
+  //
+  // #version 140
+  // in vec4 BaseColor;
+  // 
+  // layout(std140) uniform U_t
+  // {
+  //     bool g_B ;
+  // } ;
+  // 
+  // void main()
+  // {
+  //     vec4 v = BaseColor;
+  //     if (g_B) 
+  //       vec4 v = v * 0.25;
+  //     gl_FragColor = v;
+  // }
+
+  const std::string predefs =
+      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 %U_t "U_t"
+OpMemberName %U_t 0 "g_B"
+OpName %_ ""
+OpName %v_0 "v"
+OpName %gl_FragColor "gl_FragColor"
+OpMemberDecorate %U_t 0 Offset 0
+OpDecorate %U_t Block
+OpDecorate %_ DescriptorSet 0
+%void = OpTypeVoid
+%10 = 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
+%uint = OpTypeInt 32 0
+%U_t = OpTypeStruct %uint
+%_ptr_Uniform_U_t = OpTypePointer Uniform %U_t
+%_ = OpVariable %_ptr_Uniform_U_t Uniform
+%int = OpTypeInt 32 1
+%int_0 = OpConstant %int 0
+%_ptr_Uniform_uint = OpTypePointer Uniform %uint
+%bool = OpTypeBool
+%uint_0 = OpConstant %uint 0
+%float_0_25 = OpConstant %float 0.25
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%gl_FragColor = OpVariable %_ptr_Output_v4float Output
+)";
+
+  const std::string before =
+      R"(%main = OpFunction %void None %10
+%24 = OpLabel
+%v = OpVariable %_ptr_Function_v4float Function
+%v_0 = OpVariable %_ptr_Function_v4float Function
+%25 = OpLoad %v4float %BaseColor
+OpStore %v %25
+%26 = OpAccessChain %_ptr_Uniform_uint %_ %int_0
+%27 = OpLoad %uint %26
+%28 = OpINotEqual %bool %27 %uint_0
+OpSelectionMerge %29 None
+OpBranchConditional %28 %30 %29
+%30 = OpLabel
+%31 = OpLoad %v4float %v
+%32 = OpVectorTimesScalar %v4float %31 %float_0_25
+OpBranch %33
+%33 = OpLabel
+OpStore %v_0 %32
+OpBranch %29
+%29 = OpLabel
+%34 = OpLoad %v4float %v
+OpStore %gl_FragColor %34
+OpReturn
+OpFunctionEnd
+)";
+
+  const std::string after =
+      R"(%main = OpFunction %void None %10
+%24 = OpLabel
+%v = OpVariable %_ptr_Function_v4float Function
+%v_0 = OpVariable %_ptr_Function_v4float Function
+%25 = OpLoad %v4float %BaseColor
+OpStore %v %25
+%26 = OpAccessChain %_ptr_Uniform_uint %_ %int_0
+%27 = OpLoad %uint %26
+%28 = OpINotEqual %bool %27 %uint_0
+OpSelectionMerge %29 None
+OpBranchConditional %28 %30 %29
+%30 = OpLabel
+%31 = OpLoad %v4float %v
+%32 = OpVectorTimesScalar %v4float %31 %float_0_25
+OpStore %v_0 %32
+OpBranch %29
+%29 = OpLabel
+%34 = OpLoad %v4float %v
+OpStore %gl_FragColor %34
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndCheck<opt::BlockMergePass>(
+      predefs + before, predefs + after, true, true);
+}
+
+// TODO(greg-lunarg): Add tests to verify handling of these cases:
+//
+//    More complex control flow
+//    Others?
+
+}  // anonymous namespace

tools/opt/opt.cpp

@@ -141,6 +141,8 @@ int main(int argc, char** argv) {
         optimizer.RegisterPass(CreateLocalSingleBlockLoadStoreElimPass());
       } else if (0 == strcmp(cur_arg, "--eliminate-local-single-store")) {
         optimizer.RegisterPass(CreateLocalSingleStoreElimPass());
+      } else if (0 == strcmp(cur_arg, "--merge-blocks")) {
+        optimizer.RegisterPass(CreateBlockMergePass());
       } else if (0 == strcmp(cur_arg, "--eliminate-dead-const")) {
         optimizer.RegisterPass(CreateEliminateDeadConstantPass());
       } else if (0 == strcmp(cur_arg, "--fold-spec-const-op-composite")) {