Inline: Split out InlineExhaustivePass from InlinePass

include/spirv-tools/optimizer.hpp

@@ -208,7 +208,7 @@ Optimizer::PassToken CreateBlockMergePass();
 // passes. As the inlining is exhaustive, there is no attempt to optimize for
 // size or runtime performance. Functions that are not designated as entry
 // points are not changed.
-Optimizer::PassToken CreateInlinePass();
+Optimizer::PassToken CreateInlineExhaustivePass();
   
 // Creates a single-block local variable load/store elimination pass.
 // For every entry point function, do single block memory optimization of 

source/opt/CMakeLists.txt

@@ -27,6 +27,7 @@ add_library(SPIRV-Tools-opt
   fold_spec_constant_op_and_composite_pass.h
   freeze_spec_constant_value_pass.h
   inline_pass.h
+  inline_exhaustive_pass.h
   insert_extract_elim.h
   instruction.h
   ir_loader.h
@@ -62,6 +63,7 @@ add_library(SPIRV-Tools-opt
   fold_spec_constant_op_and_composite_pass.cpp
   freeze_spec_constant_value_pass.cpp
   inline_pass.cpp
+  inline_exhaustive_pass.cpp
   insert_extract_elim.cpp
   instruction.cpp
   ir_loader.cpp

source/opt/inline_exhaustive_pass.cpp

@@ -0,0 +1,84 @@
+// 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 "inline_exhaustive_pass.h"
+
+namespace spvtools {
+namespace opt {
+
+namespace {
+
+const int kEntryPointFunctionIdInIdx = 1;
+
+} // anonymous namespace
+
+bool InlineExhaustivePass::InlineExhaustive(ir::Function* func) {
+  bool modified = false;
+  // Using block iterators here because of block erasures and insertions.
+  for (auto bi = func->begin(); bi != func->end(); ++bi) {
+    for (auto ii = bi->begin(); ii != bi->end();) {
+      if (IsInlinableFunctionCall(&*ii)) {
+        // Inline call.
+        std::vector<std::unique_ptr<ir::BasicBlock>> newBlocks;
+        std::vector<std::unique_ptr<ir::Instruction>> newVars;
+        GenInlineCode(&newBlocks, &newVars, ii, bi);
+        // If call block is replaced with more than one block, point
+        // succeeding phis at new last block.
+        if (newBlocks.size() > 1)
+          UpdateSucceedingPhis(newBlocks);
+        // Replace old calling block with new block(s).
+        bi = bi.Erase();
+        bi = bi.InsertBefore(&newBlocks);
+        // Insert new function variables.
+        if (newVars.size() > 0) func->begin()->begin().InsertBefore(&newVars);
+        // Restart inlining at beginning of calling block.
+        ii = bi->begin();
+        modified = true;
+      } else {
+        ++ii;
+      }
+    }
+  }
+  return modified;
+}
+
+void InlineExhaustivePass::Initialize(ir::Module* module) {
+  InitializeInline(module);
+};
+
+Pass::Status InlineExhaustivePass::ProcessImpl() {
+  // Do exhaustive inlining on each entry point function in module
+  bool modified = false;
+  for (auto& e : module_->entry_points()) {
+    ir::Function* fn =
+        id2function_[e.GetSingleWordOperand(kEntryPointFunctionIdInIdx)];
+    modified = InlineExhaustive(fn) || modified;
+  }
+
+  FinalizeNextId(module_);
+
+  return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange;
+}
+
+InlineExhaustivePass::InlineExhaustivePass() {}
+
+Pass::Status InlineExhaustivePass::Process(ir::Module* module) {
+  Initialize(module);
+  return ProcessImpl();
+}
+
+}  // namespace opt
+}  // namespace spvtools

source/opt/inline_exhaustive_pass.h

@@ -0,0 +1,54 @@
+// 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_INLINE_EXHAUSTIVE_PASS_H_
+#define LIBSPIRV_OPT_INLINE_EXHAUSTIVE_PASS_H_
+
+#include <algorithm>
+#include <list>
+#include <memory>
+#include <vector>
+#include <unordered_map>
+
+#include "def_use_manager.h"
+#include "module.h"
+#include "inline_pass.h"
+
+namespace spvtools {
+namespace opt {
+
+// See optimizer.hpp for documentation.
+class InlineExhaustivePass : public InlinePass {
+
+ public:
+  InlineExhaustivePass();
+  Status Process(ir::Module*) override;
+
+  const char* name() const override { return "inline-exhaustive"; }
+
+ private:
+  // Exhaustively inline all function calls in func as well as in
+  // all code that is inlined into func. Return true if func is modified.
+  bool InlineExhaustive(ir::Function* func);
+
+  void Initialize(ir::Module* module);
+  Pass::Status ProcessImpl();
+};
+
+}  // namespace opt
+}  // namespace spvtools
+
+#endif  // LIBSPIRV_OPT_INLINE_EXHAUSTIVE_PASS_H_

source/opt/inline_pass.cpp

@@ -15,11 +15,11 @@
 // limitations under the License.
 
 #include "inline_pass.h"
+
 #include "cfa.h"
 
 // Indices of operands in SPIR-V instructions
 
-static const int kSpvEntryPointFunctionId = 1;
 static const int kSpvFunctionCallFunctionId = 2;
 static const int kSpvFunctionCallArgumentId = 3;
 static const int kSpvReturnValueId = 0;
@@ -429,47 +429,21 @@ bool InlinePass::IsInlinableFunctionCall(const ir::Instruction* inst) {
   return ci != inlinable_.cend();
 }
 
-bool InlinePass::Inline(ir::Function* func) {
-  bool modified = false;
-  // Using block iterators here because of block erasures and insertions.
-  for (auto bi = func->begin(); bi != func->end(); ++bi) {
-    for (auto ii = bi->begin(); ii != bi->end();) {
-      if (IsInlinableFunctionCall(&*ii)) {
-        // Inline call.
-        std::vector<std::unique_ptr<ir::BasicBlock>> newBlocks;
-        std::vector<std::unique_ptr<ir::Instruction>> newVars;
-        GenInlineCode(&newBlocks, &newVars, ii, bi);
-        // Update phi functions in successor blocks if call block
-        // is replaced with more than one block.
-        if (newBlocks.size() > 1) {
-          const auto firstBlk = newBlocks.begin();
-          const auto lastBlk = newBlocks.end() - 1;
-          const uint32_t firstId = (*firstBlk)->id();
-          const uint32_t lastId = (*lastBlk)->id();
-          (*lastBlk)->ForEachSuccessorLabel(
-              [&firstId, &lastId, this](uint32_t succ) {
-                ir::BasicBlock* sbp = this->id2block_[succ];
-                sbp->ForEachPhiInst([&firstId, &lastId](ir::Instruction* phi) {
-                  phi->ForEachInId([&firstId, &lastId](uint32_t* id) {
-                    if (*id == firstId) *id = lastId;
-                  });
-                });
-              });
-        }
-        // Replace old calling block with new block(s).
-        bi = bi.Erase();
-        bi = bi.InsertBefore(&newBlocks);
-        // Insert new function variables.
-        if (newVars.size() > 0) func->begin()->begin().InsertBefore(&newVars);
-        // Restart inlining at beginning of calling block.
-        ii = bi->begin();
-        modified = true;
-      } else {
-        ++ii;
-      }
-    }
-  }
-  return modified;
+void InlinePass::UpdateSucceedingPhis(
+    std::vector<std::unique_ptr<ir::BasicBlock>>& new_blocks) {
+  const auto firstBlk = new_blocks.begin();
+  const auto lastBlk = new_blocks.end() - 1;
+  const uint32_t firstId = (*firstBlk)->id();
+  const uint32_t lastId = (*lastBlk)->id();
+  (*lastBlk)->ForEachSuccessorLabel(
+      [&firstId, &lastId, this](uint32_t succ) {
+        ir::BasicBlock* sbp = this->id2block_[succ];
+        sbp->ForEachPhiInst([&firstId, &lastId](ir::Instruction* phi) {
+          phi->ForEachInId([&firstId, &lastId](uint32_t* id) {
+            if (*id == firstId) *id = lastId;
+          });
+        });
+      });
 }
 
 bool InlinePass::HasMultipleReturns(ir::Function* func) {
@@ -589,11 +563,11 @@ bool InlinePass::IsInlinableFunction(ir::Function* func) {
   // done validly if the return was not in a loop in the original function.
   // Also remember functions with multiple (early) returns.
   AnalyzeReturns(func);
-  const auto ci = no_return_in_loop_.find(func->result_id());
-  return ci != no_return_in_loop_.cend();
+  return no_return_in_loop_.find(func->result_id()) != 
+      no_return_in_loop_.cend();
 }
 
-void InlinePass::Initialize(ir::Module* module) {
+void InlinePass::InitializeInline(ir::Module* module) {
   def_use_mgr_.reset(new analysis::DefUseManager(consumer(), module));
 
   // Initialize next unused Id.
@@ -604,10 +578,14 @@ void InlinePass::Initialize(ir::Module* module) {
 
   false_id_ = 0;
 
+  // clear collections
   id2function_.clear();
   id2block_.clear();
   block2structured_succs_.clear();
   inlinable_.clear();
+  no_return_in_loop_.clear();
+  early_return_.clear();
+
   for (auto& fn : *module_) {
     // Initialize function and block maps.
     id2function_[fn.result_id()] = &fn;
@@ -620,27 +598,9 @@ void InlinePass::Initialize(ir::Module* module) {
   }
 };
 
-Pass::Status InlinePass::ProcessImpl() {
-  // Do exhaustive inlining on each entry point function in module
-  bool modified = false;
-  for (auto& e : module_->entry_points()) {
-    ir::Function* fn =
-        id2function_[e.GetSingleWordOperand(kSpvEntryPointFunctionId)];
-    modified = Inline(fn) || modified;
-  }
-
-  FinalizeNextId(module_);
-
-  return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange;
-}
 
 InlinePass::InlinePass()
     : module_(nullptr), def_use_mgr_(nullptr), next_id_(0) {}
 
-Pass::Status InlinePass::Process(ir::Module* module) {
-  Initialize(module);
-  return ProcessImpl();
-}
-
 }  // namespace opt
 }  // namespace spvtools

source/opt/inline_pass.h

@@ -40,11 +40,9 @@ class InlinePass : public Pass {
      std::function<std::vector<ir::BasicBlock*>*(const ir::BasicBlock*)>;
 
   InlinePass();
-  Status Process(ir::Module*) override;
+  virtual ~InlinePass() = default;
 
-  const char* name() const override { return "inline"; }
-
- private:
+ protected:
   // Return the next available Id and increment it.
   inline uint32_t TakeNextId() { return next_id_++; }
 
@@ -170,14 +168,16 @@ class InlinePass : public Pass {
   // Return true if |func| is a function that can be inlined.
   bool IsInlinableFunction(ir::Function* func);
 
-  // Exhaustively inline all function calls in func as well as in
-  // all code that is inlined into func. Return true if func is modified.
-  bool Inline(ir::Function* func);
+  // Update phis in succeeding blocks to point to new last block
+  void UpdateSucceedingPhis(
+      std::vector<std::unique_ptr<ir::BasicBlock>>& new_blocks);
 
-  void Initialize(ir::Module* module);
-  Pass::Status ProcessImpl();
+  void InitializeInline(ir::Module* module);
 
+  // Module being processed by this pass
   ir::Module* module_;
+
+  // Def/Use database
   std::unique_ptr<analysis::DefUseManager> def_use_mgr_;
 
   // Map from function's result id to function.

source/opt/optimizer.cpp

@@ -137,8 +137,9 @@ Optimizer::PassToken CreateBlockMergePass() {
       MakeUnique<opt::BlockMergePass>());
 }
 
-Optimizer::PassToken CreateInlinePass() {
-  return MakeUnique<Optimizer::PassToken::Impl>(MakeUnique<opt::InlinePass>());
+Optimizer::PassToken CreateInlineExhaustivePass() {
+  return MakeUnique<Optimizer::PassToken::Impl>(
+      MakeUnique<opt::InlineExhaustivePass>());
 }
   
 Optimizer::PassToken CreateLocalAccessChainConvertPass() {

source/opt/passes.h

@@ -24,7 +24,7 @@
 #include "eliminate_dead_constant_pass.h"
 #include "flatten_decoration_pass.h"
 #include "fold_spec_constant_op_and_composite_pass.h"
-#include "inline_pass.h"
+#include "inline_exhaustive_pass.h"
 #include "insert_extract_elim.h"
 #include "local_single_block_elim_pass.h"
 #include "local_single_store_elim_pass.h"

test/opt/inline_test.cpp

@@ -133,7 +133,7 @@ TEST_F(InlineTest, Simple) {
                "OpFunctionEnd",
       // clang-format on
   };
-  SinglePassRunAndCheck<opt::InlinePass>(
+  SinglePassRunAndCheck<opt::InlineExhaustivePass>(
       JoinAllInsts(concat(concat(predefs, before), nonEntryFuncs)),
       JoinAllInsts(concat(concat(predefs, after), nonEntryFuncs)),
       /* skip_nop = */ false, /* do_validate = */ true);
@@ -283,7 +283,7 @@ TEST_F(InlineTest, Nested) {
                "OpFunctionEnd",
       // clang-format on
   };
-  SinglePassRunAndCheck<opt::InlinePass>(
+  SinglePassRunAndCheck<opt::InlineExhaustivePass>(
       JoinAllInsts(concat(concat(predefs, before), nonEntryFuncs)),
       JoinAllInsts(concat(concat(predefs, after), nonEntryFuncs)),
       /* skip_nop = */ false, /* do_validate = */ true);
@@ -412,7 +412,7 @@ TEST_F(InlineTest, InOutParameter) {
                "OpFunctionEnd",
       // clang-format on
   };
-  SinglePassRunAndCheck<opt::InlinePass>(
+  SinglePassRunAndCheck<opt::InlineExhaustivePass>(
       JoinAllInsts(concat(concat(predefs, before), nonEntryFuncs)),
       JoinAllInsts(concat(concat(predefs, after), nonEntryFuncs)),
       /* skip_nop = */ false, /* do_validate = */ true);
@@ -548,7 +548,7 @@ TEST_F(InlineTest, BranchInCallee) {
                "OpFunctionEnd",
       // clang-format on
   };
-  SinglePassRunAndCheck<opt::InlinePass>(
+  SinglePassRunAndCheck<opt::InlineExhaustivePass>(
       JoinAllInsts(concat(concat(predefs, before), nonEntryFuncs)),
       JoinAllInsts(concat(concat(predefs, after), nonEntryFuncs)),
       /* skip_nop = */ false, /* do_validate = */ true);
@@ -743,7 +743,7 @@ TEST_F(InlineTest, PhiAfterCall) {
                "OpFunctionEnd",
       // clang-format on
   };
-  SinglePassRunAndCheck<opt::InlinePass>(
+  SinglePassRunAndCheck<opt::InlineExhaustivePass>(
       JoinAllInsts(concat(concat(predefs, before), nonEntryFuncs)),
       JoinAllInsts(concat(concat(predefs, after), nonEntryFuncs)),
       /* skip_nop = */ false, /* do_validate = */ true);
@@ -940,7 +940,7 @@ TEST_F(InlineTest, OpSampledImageOutOfBlock) {
                "OpFunctionEnd",
       // clang-format on
   };
-  SinglePassRunAndCheck<opt::InlinePass>(
+  SinglePassRunAndCheck<opt::InlineExhaustivePass>(
       JoinAllInsts(concat(concat(predefs, before), nonEntryFuncs)),
       JoinAllInsts(concat(concat(predefs, after), nonEntryFuncs)),
       /* skip_nop = */ false, /* do_validate = */ true);
@@ -1146,7 +1146,7 @@ TEST_F(InlineTest, OpImageOutOfBlock) {
                "OpFunctionEnd",
       // clang-format on
   };
-  SinglePassRunAndCheck<opt::InlinePass>(
+  SinglePassRunAndCheck<opt::InlineExhaustivePass>(
       JoinAllInsts(concat(concat(predefs, before), nonEntryFuncs)),
       JoinAllInsts(concat(concat(predefs, after), nonEntryFuncs)),
       /* skip_nop = */ false, /* do_validate = */ true);
@@ -1352,7 +1352,7 @@ TEST_F(InlineTest, OpImageAndOpSampledImageOutOfBlock) {
                "OpFunctionEnd",
       // clang-format on
   };
-  SinglePassRunAndCheck<opt::InlinePass>(
+  SinglePassRunAndCheck<opt::InlineExhaustivePass>(
       JoinAllInsts(concat(concat(predefs, before), nonEntryFuncs)),
       JoinAllInsts(concat(concat(predefs, after), nonEntryFuncs)),
       /* skip_nop = */ false, /* do_validate = */ true);
@@ -1480,7 +1480,8 @@ OpReturn
 OpFunctionEnd
 )";
 
-  SinglePassRunAndCheck<opt::InlinePass>(predefs + before + nonEntryFuncs, 
+  SinglePassRunAndCheck<opt::InlineExhaustivePass>(
+      predefs + before + nonEntryFuncs, 
       predefs + after + nonEntryFuncs, false, true);
 }
 TEST_F(InlineTest, EarlyReturnInLoopIsNotInlined) {
@@ -1575,7 +1576,8 @@ OpReturnValue %41
 OpFunctionEnd
 )";
 
-  SinglePassRunAndCheck<opt::InlinePass>(assembly, assembly, false, true);
+  SinglePassRunAndCheck<opt::InlineExhaustivePass>(
+      assembly, assembly, false, true);
 }
 
 TEST_F(InlineTest, ExternalFunctionIsNotInlined) {
@@ -1599,7 +1601,8 @@ OpReturn
 OpFunctionEnd
 )";
 
-  SinglePassRunAndCheck<opt::InlinePass>(assembly, assembly, false, true);
+  SinglePassRunAndCheck<opt::InlineExhaustivePass>(
+      assembly, assembly, false, true);
 }
 
 // TODO(greg-lunarg): Add tests to verify handling of these cases:

tools/opt/opt.cpp

@@ -171,7 +171,7 @@ int main(int argc, char** argv) {
       } else if (0 == strcmp(cur_arg, "--freeze-spec-const")) {
         optimizer.RegisterPass(CreateFreezeSpecConstantValuePass());
       } else if (0 == strcmp(cur_arg, "--inline-entry-points-exhaustive")) {
-        optimizer.RegisterPass(CreateInlinePass());
+        optimizer.RegisterPass(CreateInlineExhaustivePass());
       } else if (0 == strcmp(cur_arg, "--convert-local-access-chains")) {
         optimizer.RegisterPass(CreateLocalAccessChainConvertPass());
       } else if (0 == strcmp(cur_arg, "--eliminate-dead-code-aggressive")) {