Add a new class opt::CFG to represent the CFG for the module.

This class moves some of the CFG-related functionality into a new
class opt::CFG.  There is some other code related to the CFG in the
inliner and in opt::LocalSingleStoreElimPass that should also be moved,
but that require more changes than this pure restructuring.

I will move those bits in a follow-up PR.

Currently, the CFG is computed every time a pass is instantiated, but
this should be later moved to the new IRContext class that @s-perron is
working on.

Other re-factoring:

- Add BasicBlock::ContinueBlockIdIfAny. Re-factored out of MergeBlockIdIfAny
- Rewrite IsLoopHeader in terms of GetLoopMergeInst.
- Run clang-format on some files.

Android.mk

@@ -51,6 +51,7 @@ SPVTOOLS_OPT_SRC_FILES := \
 		source/opt/basic_block.cpp \
 		source/opt/block_merge_pass.cpp \
 		source/opt/build_module.cpp \
+		source/opt/cfg.cpp \
 		source/opt/cfg_cleanup_pass.cpp \
 		source/opt/compact_ids_pass.cpp \
 		source/opt/common_uniform_elim_pass.cpp \

source/cfa.h

@@ -19,6 +19,7 @@
 #include <cassert>
 #include <functional>
 #include <map>
+#include <cstdint>
 #include <unordered_map>
 #include <unordered_set>
 #include <utility>

source/opt/CMakeLists.txt

@@ -17,6 +17,7 @@ add_library(SPIRV-Tools-opt
   block_merge_pass.h
   build_module.h
   cfg_cleanup_pass.h
+  cfg.h
   common_uniform_elim_pass.h
   compact_ids_pass.h
   constants.h
@@ -63,6 +64,7 @@ add_library(SPIRV-Tools-opt
   block_merge_pass.cpp
   build_module.cpp
   cfg_cleanup_pass.cpp
+  cfg.cpp
   common_uniform_elim_pass.cpp
   compact_ids_pass.cpp
   decoration_manager.cpp

source/opt/aggressive_dead_code_elim_pass.cpp

@@ -199,7 +199,7 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
   ComputeInst2BlockMap(func);
   // Compute map from block to controlling conditional branch
   std::list<ir::BasicBlock*> structuredOrder;
-  ComputeStructuredOrder(func, &*func->begin(), &structuredOrder);
+  cfg()->ComputeStructuredOrder(func, &*func->begin(), &structuredOrder);
   ComputeBlock2HeaderMaps(structuredOrder);
   bool modified = false;
   // Add instructions with external side effects to worklist. Also add branches

source/opt/basic_block.cpp

@@ -19,6 +19,14 @@
 namespace spvtools {
 namespace ir {
 
+namespace {
+
+const uint32_t kLoopMergeContinueBlockIdInIdx = 1;
+const uint32_t kLoopMergeMergeBlockIdInIdx = 0;
+const uint32_t kSelectionMergeMergeBlockIdInIdx = 0;
+
+}  // namespace
+
 BasicBlock::BasicBlock(const BasicBlock& bb)
     : function_(nullptr),
       label_(MakeUnique<Instruction>(bb.GetLabelInst())),
@@ -35,7 +43,7 @@ const Instruction* BasicBlock::GetMergeInst() const {
   if (iter != cbegin()) {
     --iter;
     const auto opcode = iter->opcode();
-    if (opcode == SpvOpLoopMerge || opcode  == SpvOpSelectionMerge) {
+    if (opcode == SpvOpLoopMerge || opcode == SpvOpSelectionMerge) {
       result = &*iter;
     }
   }
@@ -50,7 +58,7 @@ Instruction* BasicBlock::GetMergeInst() {
   if (iter != begin()) {
     --iter;
     const auto opcode = iter->opcode();
-    if (opcode == SpvOpLoopMerge || opcode  == SpvOpSelectionMerge) {
+    if (opcode == SpvOpLoopMerge || opcode == SpvOpSelectionMerge) {
       result = &*iter;
     }
   }
@@ -101,13 +109,39 @@ void BasicBlock::ForMergeAndContinueLabel(
   --ii;
   if (ii == insts_.begin()) return;
   --ii;
-  if (ii->opcode() == SpvOpSelectionMerge || 
+  if (ii->opcode() == SpvOpSelectionMerge || ii->opcode() == SpvOpLoopMerge) {
-      ii->opcode() == SpvOpLoopMerge)
+    ii->ForEachInId([&f](const uint32_t* idp) { f(*idp); });
-    ii->ForEachInId([&f](const uint32_t* idp) {
+  }
-      f(*idp);
+}
-    });
+
+uint32_t BasicBlock::MergeBlockIdIfAny() const {
+  auto merge_ii = cend();
+  --merge_ii;
+  uint32_t mbid = 0;
+  if (merge_ii != cbegin()) {
+    --merge_ii;
+    if (merge_ii->opcode() == SpvOpLoopMerge) {
+      mbid = merge_ii->GetSingleWordInOperand(kLoopMergeMergeBlockIdInIdx);
+    } else if (merge_ii->opcode() == SpvOpSelectionMerge) {
+      mbid = merge_ii->GetSingleWordInOperand(kSelectionMergeMergeBlockIdInIdx);
+    }
+  }
+
+  return mbid;
+}
+
+uint32_t BasicBlock::ContinueBlockIdIfAny() const {
+  auto merge_ii = cend();
+  --merge_ii;
+  uint32_t cbid = 0;
+  if (merge_ii != cbegin()) {
+    --merge_ii;
+    if (merge_ii->opcode() == SpvOpLoopMerge) {
+      cbid = merge_ii->GetSingleWordInOperand(kLoopMergeContinueBlockIdInIdx);
+    }
+  }
+  return cbid;
 }
 
 }  // namespace ir
 }  // namespace spvtools
-

source/opt/basic_block.h

@@ -64,6 +64,7 @@ class BasicBlock {
   // Otherwise return null.  May be used whenever tail() can be used.
   const Instruction* GetMergeInst() const;
   Instruction* GetMergeInst();
+
   // Returns the OpLoopMerge instruciton in this basic block, if it exists.
   // Otherwise return null.  May be used whenever tail() can be used.
   const Instruction* GetLoopMergeInst() const;
@@ -84,6 +85,7 @@ class BasicBlock {
     assert(!insts_.empty());
     return --end();
   }
+
   // Returns a const iterator, but othewrise similar to tail().
   const_iterator ctail() const {
     assert(!insts_.empty());
@@ -118,6 +120,18 @@ class BasicBlock {
     return count > 0;
   }
 
+  // Return true if this block is a loop header block.
+  bool IsLoopHeader() const { return GetLoopMergeInst() != nullptr; }
+
+  // Returns the ID of the merge block declared by a merge instruction in this
+  // block, if any.  If none, returns zero.  If |cbid| is not nullptr, the ID of
+  // the continue block in the merge instruction is set in |*cbid|.
+  uint32_t MergeBlockIdIfAny() const;
+
+  // Returns the ID of the continue block declared by a merge instruction in
+  // this block, if any.  If none, returns zero.
+  uint32_t ContinueBlockIdIfAny() const;
+
  private:
   // The enclosing function.
   Function* function_;
@@ -136,7 +150,7 @@ inline void BasicBlock::AddInstruction(std::unique_ptr<Instruction> i) {
 
 inline void BasicBlock::AddInstructions(BasicBlock* bp) {
   auto bEnd = end();
-  (void) bEnd.MoveBefore(&bp->insts_);
+  (void)bEnd.MoveBefore(&bp->insts_);
 }
 
 inline void BasicBlock::ForEachInst(const std::function<void(Instruction*)>& f,
@@ -152,8 +166,8 @@ inline void BasicBlock::ForEachInst(
     static_cast<const Instruction*>(label_.get())
         ->ForEachInst(f, run_on_debug_line_insts);
   for (const auto& inst : insts_)
-    static_cast<const Instruction*>(&inst)
+    static_cast<const Instruction*>(&inst)->ForEachInst(
-        ->ForEachInst(f, run_on_debug_line_insts);
+        f, run_on_debug_line_insts);
 }
 
 inline void BasicBlock::ForEachPhiInst(

source/opt/block_merge_pass.cpp

@@ -54,7 +54,7 @@ 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)) {
+    if (bi->IsLoopHeader()) {
       ++bi;
       continue;
     }

source/opt/cfg.cpp

@@ -0,0 +1,94 @@
+// Copyright (c) 2017 Google 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 "cfa.h"
+#include "cfg.h"
+#include "module.h"
+
+namespace spvtools {
+namespace opt {
+
+namespace {
+
+// Universal Limit of ResultID + 1
+const int kInvalidId = 0x400000;
+
+}  // namespace
+
+CFG::CFG(ir::Module* module)
+    : module_(module),
+      pseudo_entry_block_(std::unique_ptr<ir::Instruction>(
+          new ir::Instruction(SpvOpLabel, 0, 0, {}))),
+      pseudo_exit_block_(std::unique_ptr<ir::Instruction>(
+          new ir::Instruction(SpvOpLabel, 0, kInvalidId, {}))) {
+  for (auto& fn : *module) {
+    for (auto& blk : fn) {
+      uint32_t blkId = blk.id();
+      id2block_[blkId] = &blk;
+      blk.ForEachSuccessorLabel([&blkId, this](uint32_t sbid) {
+        label2preds_[sbid].push_back(blkId);
+      });
+    }
+  }
+}
+
+void CFG::ComputeStructuredOrder(ir::Function* func, ir::BasicBlock* root,
+                                  std::list<ir::BasicBlock*>* order) {
+  assert(module_->HasCapability(SpvCapabilityShader) &&
+         "This only works on structured control flow");
+
+  // Compute structured successors and do DFS.
+  ComputeStructuredSuccessors(func);
+  auto ignore_block = [](cbb_ptr) {};
+  auto ignore_edge = [](cbb_ptr, cbb_ptr) {};
+  auto get_structured_successors = [this](const ir::BasicBlock* b) {
+    return &(block2structured_succs_[b]);
+  };
+
+  // TODO(greg-lunarg): Get rid of const_cast by making moving const
+  // out of the cfa.h prototypes and into the invoking code.
+  auto post_order = [&](cbb_ptr b) {
+    order->push_front(const_cast<ir::BasicBlock*>(b));
+  };
+  spvtools::CFA<ir::BasicBlock>::DepthFirstTraversal(
+      root, get_structured_successors, ignore_block, post_order,
+      ignore_edge);
+}
+
+void CFG::ComputeStructuredSuccessors(ir::Function *func) {
+  block2structured_succs_.clear();
+  for (auto& blk : *func) {
+    // If no predecessors in function, make successor to pseudo entry.
+    if (label2preds_[blk.id()].size() == 0)
+      block2structured_succs_[&pseudo_entry_block_].push_back(&blk);
+
+    // If header, make merge block first successor and continue block second
+    // successor if there is one.
+    uint32_t mbid = blk.MergeBlockIdIfAny();
+    if (mbid != 0) {
+      block2structured_succs_[&blk].push_back(id2block_[mbid]);
+      uint32_t cbid = blk.ContinueBlockIdIfAny();
+      if (cbid != 0)
+        block2structured_succs_[&blk].push_back(id2block_[cbid]);
+    }
+
+    // Add true successors.
+    blk.ForEachSuccessorLabel([&blk, this](uint32_t sbid) {
+      block2structured_succs_[&blk].push_back(id2block_[sbid]);
+    });
+  }
+}
+
+}  // namespace opt
+}  // namespace spvtools

source/opt/cfg.h

@@ -0,0 +1,110 @@
+// Copyright (c) 2017 Google 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_CFG_H_
+#define LIBSPIRV_OPT_CFG_H_
+
+#include "basic_block.h"
+
+#include <list>
+#include <unordered_map>
+
+namespace spvtools {
+namespace opt {
+
+class CFG {
+ public:
+  CFG(ir::Module *module);
+
+  // Return the module described by this CFG.
+  ir::Module* get_module() const { return module_; }
+
+  // Return the list of predecesors for basic block with label |blkid|.
+  // TODO(dnovillo): Move this to ir::BasicBlock.
+  const std::vector<uint32_t>& preds(uint32_t blk_id) const {
+    return label2preds_.at(blk_id);
+  }
+
+  // Return a pointer to the basic block instance corresponding to the label
+  // |blk_id|.
+  ir::BasicBlock* block(uint32_t blk_id) const { return id2block_.at(blk_id); }
+
+  // Return the pseudo entry and exit blocks. TODO(dnovillo): Remove when
+  // LocalSingleStoreElimPass::CalculateImmediateDominators() is moved into this
+  // class.
+  const ir::BasicBlock* pseudo_entry_block() const {
+    return &pseudo_entry_block_;
+  }
+  ir::BasicBlock* pseudo_entry_block() { return &pseudo_entry_block_; }
+
+  const ir::BasicBlock* pseudo_exit_block() const {
+    return &pseudo_exit_block_;
+  }
+  ir::BasicBlock* pseudo_exit_block() { return &pseudo_exit_block_; }
+
+  // Return true if |block_ptr| is the pseudo-entry block.
+  bool IsPseudoEntryBlock(ir::BasicBlock* block_ptr) const {
+    return block_ptr == &pseudo_entry_block_;
+  }
+
+  // Return true if |block_ptr| is the pseudo-exit block.
+  bool IsPseudoExitBlock(ir::BasicBlock* block_ptr) const {
+    return block_ptr == &pseudo_exit_block_;
+  }
+
+  // Compute structured block order into |structuredOrder| for |func| starting
+  // at |root|. This order has the property that dominators come before all
+  // blocks they dominate and merge blocks come after all blocks that are in
+  // the control constructs of their header.
+  void ComputeStructuredOrder(ir::Function* func, ir::BasicBlock* root,
+                              std::list<ir::BasicBlock*>* order);
+
+ private:
+  using cbb_ptr = const ir::BasicBlock*;
+
+  // Compute structured successors for function |func|. A block's structured
+  // successors are the blocks it branches to together with its declared merge
+  // block and continue block if it has them. When order matters, the merge
+  // block and continue block always appear first. This assures correct depth
+  // first search in the presence of early returns and kills. If the successor
+  // vector contain duplicates of the merge or continue blocks, they are safely
+  // ignored by DFS.
+  void ComputeStructuredSuccessors(ir::Function* func);
+
+  // Module for this CFG.
+  ir::Module *module_;
+
+  // Map from block to its structured successor blocks. See
+  // ComputeStructuredSuccessors() for definition.
+  std::unordered_map<const ir::BasicBlock*, std::vector<ir::BasicBlock*>>
+      block2structured_succs_;
+
+  // Extra block whose successors are all blocks with no predecessors
+  // in function. TODO(dnovillo): Needed?
+  ir::BasicBlock pseudo_entry_block_;
+
+  // Augmented CFG Exit Block. TODO(dnovillo): Needed?
+  ir::BasicBlock pseudo_exit_block_;
+
+  // Map from block's label id to its predecessor blocks ids
+  std::unordered_map<uint32_t, std::vector<uint32_t>> label2preds_;
+
+  // Map from block's label id to block.
+  std::unordered_map<uint32_t, ir::BasicBlock*> id2block_;
+};
+
+}  // namespace opt
+}  // namespace spvtools
+
+#endif  // LIBSPIRV_OPT_CFG_H_

source/opt/common_uniform_elim_pass.cpp

@@ -34,14 +34,14 @@ const uint32_t kLoadPtrIdInIdx = 0;
 const uint32_t kCopyObjectOperandInIdx = 0;
 const uint32_t kTypeIntWidthInIdx = 0;
 
-} // anonymous namespace
+}  // anonymous namespace
 
 bool CommonUniformElimPass::IsNonPtrAccessChain(const SpvOp opcode) const {
   return opcode == SpvOpAccessChain || opcode == SpvOpInBoundsAccessChain;
 }
 
 bool CommonUniformElimPass::IsSamplerOrImageType(
-  const ir::Instruction* typeInst) const {
+    const ir::Instruction* typeInst) const {
   switch (typeInst->opcode()) {
     case SpvOpTypeSampler:
     case SpvOpTypeImage:
@@ -50,8 +50,7 @@ bool CommonUniformElimPass::IsSamplerOrImageType(
     default:
       break;
   }
-  if (typeInst->opcode() != SpvOpTypeStruct)
+  if (typeInst->opcode() != SpvOpTypeStruct) return false;
-    return false;
   // Return true if any member is a sampler or image
   int samplerOrImageCnt = 0;
   typeInst->ForEachInId([&samplerOrImageCnt, this](const uint32_t* tid) {
@@ -61,24 +60,23 @@ bool CommonUniformElimPass::IsSamplerOrImageType(
   return samplerOrImageCnt > 0;
 }
 
-bool CommonUniformElimPass::IsSamplerOrImageVar(
+bool CommonUniformElimPass::IsSamplerOrImageVar(uint32_t varId) const {
-    uint32_t varId) const {
   const ir::Instruction* varInst = get_def_use_mgr()->GetDef(varId);
   assert(varInst->opcode() == SpvOpVariable);
   const uint32_t varTypeId = varInst->type_id();
   const ir::Instruction* varTypeInst = get_def_use_mgr()->GetDef(varTypeId);
   const uint32_t varPteTypeId =
-    varTypeInst->GetSingleWordInOperand(kTypePointerTypeIdInIdx);
+      varTypeInst->GetSingleWordInOperand(kTypePointerTypeIdInIdx);
   ir::Instruction* varPteTypeInst = get_def_use_mgr()->GetDef(varPteTypeId);
   return IsSamplerOrImageType(varPteTypeInst);
 }
 
-ir::Instruction* CommonUniformElimPass::GetPtr(
+ir::Instruction* CommonUniformElimPass::GetPtr(ir::Instruction* ip,
-      ir::Instruction* ip, uint32_t* objId) {
+                                               uint32_t* objId) {
   const SpvOp op = ip->opcode();
   assert(op == SpvOpStore || op == SpvOpLoad);
-  *objId = ip->GetSingleWordInOperand(
+  *objId = ip->GetSingleWordInOperand(op == SpvOpStore ? kStorePtrIdInIdx
-      op == SpvOpStore ? kStorePtrIdInIdx : kLoadPtrIdInIdx);
+                                                       : kLoadPtrIdInIdx);
   ir::Instruction* ptrInst = get_def_use_mgr()->GetDef(*objId);
   while (ptrInst->opcode() == SpvOpCopyObject) {
     *objId = ptrInst->GetSingleWordInOperand(kCopyObjectOperandInIdx);
@@ -86,11 +84,10 @@ ir::Instruction* CommonUniformElimPass::GetPtr(
   }
   ir::Instruction* objInst = ptrInst;
   while (objInst->opcode() != SpvOpVariable &&
-      objInst->opcode() != SpvOpFunctionParameter) {
+         objInst->opcode() != SpvOpFunctionParameter) {
     if (IsNonPtrAccessChain(objInst->opcode())) {
       *objId = objInst->GetSingleWordInOperand(kAccessChainPtrIdInIdx);
-    }
+    } else {
-    else {
       assert(objInst->opcode() == SpvOpCopyObject);
       *objId = objInst->GetSingleWordInOperand(kCopyObjectOperandInIdx);
     }
@@ -103,11 +100,14 @@ bool CommonUniformElimPass::IsVolatileStruct(uint32_t type_id) {
   assert(get_def_use_mgr()->GetDef(type_id)->opcode() == SpvOpTypeStruct);
   bool has_volatile_deco = false;
   dec_mgr_->ForEachDecoration(type_id, SpvDecorationVolatile,
-                              [&has_volatile_deco](const ir::Instruction&){ has_volatile_deco = true;});
+                              [&has_volatile_deco](const ir::Instruction&) {
+                                has_volatile_deco = true;
+                              });
   return has_volatile_deco;
 }
 
-bool CommonUniformElimPass::IsAccessChainToVolatileStructType(const ir::Instruction &AccessChainInst) {
+bool CommonUniformElimPass::IsAccessChainToVolatileStructType(
+    const ir::Instruction& AccessChainInst) {
   assert(AccessChainInst.opcode() == SpvOpAccessChain);
 
   uint32_t ptr_id = AccessChainInst.GetSingleWordInOperand(0);
@@ -132,8 +132,10 @@ bool CommonUniformElimPass::IsAccessChainToVolatileStructType(const ir::Instruct
 
         if (idx < num_operands - 1) {
           const uint32_t index_id = AccessChainInst.GetSingleWordOperand(idx);
-          const ir::Instruction* index_inst = get_def_use_mgr()->GetDef(index_id);
+          const ir::Instruction* index_inst =
-          uint32_t index_value = index_inst->GetSingleWordOperand(2); // TODO: replace with GetUintValueFromConstant()
+              get_def_use_mgr()->GetDef(index_id);
+          uint32_t index_value = index_inst->GetSingleWordOperand(
+              2);  // TODO: replace with GetUintValueFromConstant()
           pointee_type_id = pointee_type->GetSingleWordInOperand(index_value);
         }
         break;
@@ -149,8 +151,7 @@ bool CommonUniformElimPass::IsVolatileLoad(const ir::Instruction& loadInst) {
   // Check if this Load instruction has Volatile Memory Access flag
   if (loadInst.NumOperands() == 4) {
     uint32_t memory_access_mask = loadInst.GetSingleWordOperand(3);
-    if (memory_access_mask & SpvMemoryAccessVolatileMask)
+    if (memory_access_mask & SpvMemoryAccessVolatileMask) return true;
-      return true;
   }
   // If we load a struct directly (result type is struct),
   // check if the struct is decorated volatile
@@ -163,65 +164,56 @@ bool CommonUniformElimPass::IsVolatileLoad(const ir::Instruction& loadInst) {
 
 bool CommonUniformElimPass::IsUniformVar(uint32_t varId) {
   const ir::Instruction* varInst =
-    get_def_use_mgr()->id_to_defs().find(varId)->second;
+      get_def_use_mgr()->id_to_defs().find(varId)->second;
-  if (varInst->opcode() != SpvOpVariable)
+  if (varInst->opcode() != SpvOpVariable) return false;
-    return false;
   const uint32_t varTypeId = varInst->type_id();
   const ir::Instruction* varTypeInst =
-    get_def_use_mgr()->id_to_defs().find(varTypeId)->second;
+      get_def_use_mgr()->id_to_defs().find(varTypeId)->second;
   return varTypeInst->GetSingleWordInOperand(kTypePointerStorageClassInIdx) ==
-    SpvStorageClassUniform ||
+             SpvStorageClassUniform ||
-    varTypeInst->GetSingleWordInOperand(kTypePointerStorageClassInIdx) ==
+         varTypeInst->GetSingleWordInOperand(kTypePointerStorageClassInIdx) ==
-    SpvStorageClassUniformConstant;
+             SpvStorageClassUniformConstant;
 }
 
 bool CommonUniformElimPass::HasUnsupportedDecorates(uint32_t id) const {
   analysis::UseList* uses = get_def_use_mgr()->GetUses(id);
-  if (uses == nullptr)
+  if (uses == nullptr) return false;
-    return false;
   for (auto u : *uses) {
     const SpvOp op = u.inst->opcode();
-    if (IsNonTypeDecorate(op))
+    if (IsNonTypeDecorate(op)) return true;
-      return true;
   }
   return false;
 }
 
 bool CommonUniformElimPass::HasOnlyNamesAndDecorates(uint32_t id) const {
   analysis::UseList* uses = get_def_use_mgr()->GetUses(id);
-  if (uses == nullptr)
+  if (uses == nullptr) return true;
-    return true;
   for (auto u : *uses) {
     const SpvOp op = u.inst->opcode();
-    if (op != SpvOpName && !IsNonTypeDecorate(op))
+    if (op != SpvOpName && !IsNonTypeDecorate(op)) return false;
-      return false;
   }
   return true;
 }
 
 void CommonUniformElimPass::KillNamesAndDecorates(uint32_t id) {
-  // TODO(greg-lunarg): Remove id from any OpGroupDecorate and 
+  // TODO(greg-lunarg): Remove id from any OpGroupDecorate and
   // kill if no other operands.
   analysis::UseList* uses = get_def_use_mgr()->GetUses(id);
-  if (uses == nullptr)
+  if (uses == nullptr) return;
-    return;
   std::list<ir::Instruction*> killList;
   for (auto u : *uses) {
     const SpvOp op = u.inst->opcode();
-    if (op != SpvOpName && !IsNonTypeDecorate(op))
+    if (op != SpvOpName && !IsNonTypeDecorate(op)) continue;
-      continue;
     killList.push_back(u.inst);
   }
-  for (auto kip : killList)
+  for (auto kip : killList) get_def_use_mgr()->KillInst(kip);
-    get_def_use_mgr()->KillInst(kip);
 }
 
 void CommonUniformElimPass::KillNamesAndDecorates(ir::Instruction* inst) {
-  // TODO(greg-lunarg): Remove inst from any OpGroupDecorate and 
+  // TODO(greg-lunarg): Remove inst from any OpGroupDecorate and
   // kill if not other operands.
   const uint32_t rId = inst->result_id();
-  if (rId == 0)
+  if (rId == 0) return;
-    return;
   KillNamesAndDecorates(rId);
 }
 
@@ -235,35 +227,34 @@ void CommonUniformElimPass::DeleteIfUseless(ir::Instruction* inst) {
 }
 
 void CommonUniformElimPass::ReplaceAndDeleteLoad(ir::Instruction* loadInst,
-                                      uint32_t replId,
+                                                 uint32_t replId,
-                                      ir::Instruction* ptrInst) {
+                                                 ir::Instruction* ptrInst) {
   const uint32_t loadId = loadInst->result_id();
   KillNamesAndDecorates(loadId);
-  (void) get_def_use_mgr()->ReplaceAllUsesWith(loadId, replId);
+  (void)get_def_use_mgr()->ReplaceAllUsesWith(loadId, replId);
   // remove load instruction
   get_def_use_mgr()->KillInst(loadInst);
   // if access chain, see if it can be removed as well
-  if (IsNonPtrAccessChain(ptrInst->opcode()))
+  if (IsNonPtrAccessChain(ptrInst->opcode())) DeleteIfUseless(ptrInst);
-    DeleteIfUseless(ptrInst);
 }
 
-void CommonUniformElimPass::GenACLoadRepl(const ir::Instruction* ptrInst,
+void CommonUniformElimPass::GenACLoadRepl(
-  std::vector<std::unique_ptr<ir::Instruction>>* newInsts,
+    const ir::Instruction* ptrInst,
-  uint32_t* resultId) {
+    std::vector<std::unique_ptr<ir::Instruction>>* newInsts,
-
+    uint32_t* resultId) {
   // Build and append Load
   const uint32_t ldResultId = TakeNextId();
   const uint32_t varId =
-    ptrInst->GetSingleWordInOperand(kAccessChainPtrIdInIdx);
+      ptrInst->GetSingleWordInOperand(kAccessChainPtrIdInIdx);
   const ir::Instruction* varInst = get_def_use_mgr()->GetDef(varId);
   assert(varInst->opcode() == SpvOpVariable);
   const uint32_t varPteTypeId = GetPointeeTypeId(varInst);
   std::vector<ir::Operand> load_in_operands;
   load_in_operands.push_back(
-    ir::Operand(spv_operand_type_t::SPV_OPERAND_TYPE_ID,
+      ir::Operand(spv_operand_type_t::SPV_OPERAND_TYPE_ID,
-      std::initializer_list<uint32_t>{varId}));
+                  std::initializer_list<uint32_t>{varId}));
-  std::unique_ptr<ir::Instruction> newLoad(new ir::Instruction(SpvOpLoad,
+  std::unique_ptr<ir::Instruction> newLoad(new ir::Instruction(
-    varPteTypeId, ldResultId, load_in_operands));
+      SpvOpLoad, varPteTypeId, ldResultId, load_in_operands));
   get_def_use_mgr()->AnalyzeInstDefUse(&*newLoad);
   newInsts->emplace_back(std::move(newLoad));
 
@@ -272,21 +263,21 @@ void CommonUniformElimPass::GenACLoadRepl(const ir::Instruction* ptrInst,
   const uint32_t ptrPteTypeId = GetPointeeTypeId(ptrInst);
   std::vector<ir::Operand> ext_in_opnds;
   ext_in_opnds.push_back(
-    ir::Operand(spv_operand_type_t::SPV_OPERAND_TYPE_ID,
+      ir::Operand(spv_operand_type_t::SPV_OPERAND_TYPE_ID,
-      std::initializer_list<uint32_t>{ldResultId}));
+                  std::initializer_list<uint32_t>{ldResultId}));
   uint32_t iidIdx = 0;
-  ptrInst->ForEachInId([&iidIdx, &ext_in_opnds, this](const uint32_t *iid) {
+  ptrInst->ForEachInId([&iidIdx, &ext_in_opnds, this](const uint32_t* iid) {
     if (iidIdx > 0) {
       const ir::Instruction* cInst = get_def_use_mgr()->GetDef(*iid);
       uint32_t val = cInst->GetSingleWordInOperand(kConstantValueInIdx);
       ext_in_opnds.push_back(
-        ir::Operand(spv_operand_type_t::SPV_OPERAND_TYPE_LITERAL_INTEGER,
+          ir::Operand(spv_operand_type_t::SPV_OPERAND_TYPE_LITERAL_INTEGER,
-          std::initializer_list<uint32_t>{val}));
+                      std::initializer_list<uint32_t>{val}));
     }
     ++iidIdx;
   });
   std::unique_ptr<ir::Instruction> newExt(new ir::Instruction(
-    SpvOpCompositeExtract, ptrPteTypeId, extResultId, ext_in_opnds));
+      SpvOpCompositeExtract, ptrPteTypeId, extResultId, ext_in_opnds));
   get_def_use_mgr()->AnalyzeInstDefUse(&*newExt);
   newInsts->emplace_back(std::move(newExt));
   *resultId = extResultId;
@@ -309,27 +300,19 @@ bool CommonUniformElimPass::UniformAccessChainConvert(ir::Function* func) {
   bool modified = false;
   for (auto bi = func->begin(); bi != func->end(); ++bi) {
     for (auto ii = bi->begin(); ii != bi->end(); ++ii) {
-      if (ii->opcode() != SpvOpLoad)
+      if (ii->opcode() != SpvOpLoad) continue;
-        continue;
       uint32_t varId;
       ir::Instruction* ptrInst = GetPtr(&*ii, &varId);
-      if (!IsNonPtrAccessChain(ptrInst->opcode()))
+      if (!IsNonPtrAccessChain(ptrInst->opcode())) continue;
-        continue;
       // Do not convert nested access chains
       if (ptrInst->GetSingleWordInOperand(kAccessChainPtrIdInIdx) != varId)
         continue;
-      if (!IsUniformVar(varId))
+      if (!IsUniformVar(varId)) continue;
-        continue;
+      if (!IsConstantIndexAccessChain(ptrInst)) continue;
-      if (!IsConstantIndexAccessChain(ptrInst))
+      if (HasUnsupportedDecorates(ii->result_id())) continue;
-        continue;
+      if (HasUnsupportedDecorates(ptrInst->result_id())) continue;
-      if (HasUnsupportedDecorates(ii->result_id()))
+      if (IsVolatileLoad(*ii)) continue;
-        continue;
+      if (IsAccessChainToVolatileStructType(*ptrInst)) continue;
-      if (HasUnsupportedDecorates(ptrInst->result_id()))
-        continue;
-      if (IsVolatileLoad(*ii))
-        continue;
-      if (IsAccessChainToVolatileStructType(*ptrInst))
-        continue;
       std::vector<std::unique_ptr<ir::Instruction>> newInsts;
       uint32_t replId;
       GenACLoadRepl(ptrInst, &newInsts, &replId);
@@ -344,18 +327,20 @@ bool CommonUniformElimPass::UniformAccessChainConvert(ir::Function* func) {
 }
 
 void CommonUniformElimPass::ComputeStructuredSuccessors(ir::Function* func) {
+  block2structured_succs_.clear();
   for (auto& blk : *func) {
     // If header, make merge block first successor.
-    uint32_t cbid;
+    uint32_t mbid = blk.MergeBlockIdIfAny();
-    const uint32_t mbid = MergeBlockIdIfAny(blk, &cbid);
     if (mbid != 0) {
-      block2structured_succs_[&blk].push_back(id2block_[mbid]);
+      block2structured_succs_[&blk].push_back(cfg()->block(mbid));
-      if (cbid != 0)
+      uint32_t cbid = blk.ContinueBlockIdIfAny();
-        block2structured_succs_[&blk].push_back(id2block_[cbid]);
+      if (cbid != 0) {
+        block2structured_succs_[&blk].push_back(cfg()->block(mbid));
+      }
     }
     // add true successors
     blk.ForEachSuccessorLabel([&blk, this](uint32_t sbid) {
-      block2structured_succs_[&blk].push_back(id2block_[sbid]);
+      block2structured_succs_[&blk].push_back(cfg()->block(sbid));
     });
   }
 }
@@ -367,16 +352,18 @@ void CommonUniformElimPass::ComputeStructuredOrder(
   auto ignore_block = [](cbb_ptr) {};
   auto ignore_edge = [](cbb_ptr, cbb_ptr) {};
   auto get_structured_successors = [this](const ir::BasicBlock* block) {
-      return &(block2structured_succs_[block]); };
+    return &(block2structured_succs_[block]);
+  };
   // TODO(greg-lunarg): Get rid of const_cast by making moving const
   // out of the cfa.h prototypes and into the invoking code.
   auto post_order = [&](cbb_ptr b) {
-      order->push_front(const_cast<ir::BasicBlock*>(b)); };
+    order->push_front(const_cast<ir::BasicBlock*>(b));
-  
+  };
+
   order->clear();
   spvtools::CFA<ir::BasicBlock>::DepthFirstTraversal(
-      &*func->begin(), get_structured_successors, ignore_block,
+      &*func->begin(), get_structured_successors, ignore_block, post_order,
-      post_order, ignore_edge);
+      ignore_edge);
 }
 
 bool CommonUniformElimPass::CommonUniformLoadElimination(ir::Function* func) {
@@ -391,7 +378,7 @@ bool CommonUniformElimPass::CommonUniformLoadElimination(ir::Function* func) {
   // Find insertion point in first block to copy non-dominating loads.
   auto insertItr = func->begin()->begin();
   while (insertItr->opcode() == SpvOpVariable ||
-      insertItr->opcode() == SpvOpNop)
+         insertItr->opcode() == SpvOpNop)
     ++insertItr;
   uint32_t mergeBlockId = 0;
   for (auto bi = structuredOrder.begin(); bi != structuredOrder.end(); ++bi) {
@@ -403,36 +390,29 @@ bool CommonUniformElimPass::CommonUniformLoadElimination(ir::Function* func) {
       insertItr = bp->begin();
     }
     for (auto ii = bp->begin(); ii != bp->end(); ++ii) {
-      if (ii->opcode() != SpvOpLoad)
+      if (ii->opcode() != SpvOpLoad) continue;
-        continue;
       uint32_t varId;
       ir::Instruction* ptrInst = GetPtr(&*ii, &varId);
-      if (ptrInst->opcode() != SpvOpVariable)
+      if (ptrInst->opcode() != SpvOpVariable) continue;
-        continue;
+      if (!IsUniformVar(varId)) continue;
-      if (!IsUniformVar(varId))
+      if (IsSamplerOrImageVar(varId)) continue;
-        continue;
+      if (HasUnsupportedDecorates(ii->result_id())) continue;
-      if (IsSamplerOrImageVar(varId))
+      if (IsVolatileLoad(*ii)) continue;
-        continue;
-      if (HasUnsupportedDecorates(ii->result_id()))
-        continue;
-      if (IsVolatileLoad(*ii))
-        continue;
       uint32_t replId;
       const auto uItr = uniform2load_id_.find(varId);
       if (uItr != uniform2load_id_.end()) {
         replId = uItr->second;
-      }
+      } else {
-      else {
         if (mergeBlockId == 0) {
           // Load is in dominating block; just remember it
           uniform2load_id_[varId] = ii->result_id();
           continue;
-        }
+        } else {
-        else {
           // Copy load into most recent dominating block and remember it
           replId = TakeNextId();
-          std::unique_ptr<ir::Instruction> newLoad(new ir::Instruction(SpvOpLoad,
+          std::unique_ptr<ir::Instruction> newLoad(new ir::Instruction(
-            ii->type_id(), replId, {{spv_operand_type_t::SPV_OPERAND_TYPE_ID, {varId}}}));
+              SpvOpLoad, ii->type_id(), replId,
+              {{spv_operand_type_t::SPV_OPERAND_TYPE_ID, {varId}}}));
           get_def_use_mgr()->AnalyzeInstDefUse(&*newLoad);
           insertItr = insertItr.InsertBefore(std::move(newLoad));
           ++insertItr;
@@ -445,8 +425,7 @@ bool CommonUniformElimPass::CommonUniformLoadElimination(ir::Function* func) {
     // If we are outside of any control construct and entering one, remember
     // the id of the merge block
     if (mergeBlockId == 0) {
-      uint32_t dummy;
+      mergeBlockId = bp->MergeBlockIdIfAny();
-      mergeBlockId = MergeBlockIdIfAny(*bp, &dummy);
     }
   }
   return modified;
@@ -457,26 +436,19 @@ bool CommonUniformElimPass::CommonUniformLoadElimBlock(ir::Function* func) {
   for (auto& blk : *func) {
     uniform2load_id_.clear();
     for (auto ii = blk.begin(); ii != blk.end(); ++ii) {
-      if (ii->opcode() != SpvOpLoad)
+      if (ii->opcode() != SpvOpLoad) continue;
-        continue;
       uint32_t varId;
       ir::Instruction* ptrInst = GetPtr(&*ii, &varId);
-      if (ptrInst->opcode() != SpvOpVariable)
+      if (ptrInst->opcode() != SpvOpVariable) continue;
-        continue;
+      if (!IsUniformVar(varId)) continue;
-      if (!IsUniformVar(varId))
+      if (!IsSamplerOrImageVar(varId)) continue;
-        continue;
+      if (HasUnsupportedDecorates(ii->result_id())) continue;
-      if (!IsSamplerOrImageVar(varId))
+      if (IsVolatileLoad(*ii)) continue;
-        continue;
-      if (HasUnsupportedDecorates(ii->result_id()))
-        continue;
-      if (IsVolatileLoad(*ii))
-        continue;
       uint32_t replId;
       const auto uItr = uniform2load_id_.find(varId);
       if (uItr != uniform2load_id_.end()) {
         replId = uItr->second;
-      }
+      } else {
-      else {
         uniform2load_id_[varId] = ii->result_id();
         continue;
       }
@@ -491,13 +463,10 @@ bool CommonUniformElimPass::CommonExtractElimination(ir::Function* func) {
   // Find all composite ids with duplicate extracts.
   for (auto bi = func->begin(); bi != func->end(); ++bi) {
     for (auto ii = bi->begin(); ii != bi->end(); ++ii) {
-      if (ii->opcode() != SpvOpCompositeExtract)
+      if (ii->opcode() != SpvOpCompositeExtract) continue;
-        continue;
       // TODO(greg-lunarg): Support multiple indices
-      if (ii->NumInOperands() > 2)
+      if (ii->NumInOperands() > 2) continue;
-        continue;
+      if (HasUnsupportedDecorates(ii->result_id())) continue;
-      if (HasUnsupportedDecorates(ii->result_id()))
-        continue;
       uint32_t compId = ii->GetSingleWordInOperand(kExtractCompositeIdInIdx);
       uint32_t idx = ii->GetSingleWordInOperand(kExtractIdx0InIdx);
       comp2idx2inst_[compId][idx].push_back(&*ii);
@@ -509,13 +478,12 @@ bool CommonUniformElimPass::CommonExtractElimination(ir::Function* func) {
   for (auto bi = func->begin(); bi != func->end(); ++bi) {
     for (auto ii = bi->begin(); ii != bi->end(); ++ii) {
       const auto cItr = comp2idx2inst_.find(ii->result_id());
-      if (cItr == comp2idx2inst_.end())
+      if (cItr == comp2idx2inst_.end()) continue;
-        continue;
       for (auto idxItr : cItr->second) {
-        if (idxItr.second.size() < 2)
+        if (idxItr.second.size() < 2) continue;
-          continue;
         uint32_t replId = TakeNextId();
-        std::unique_ptr<ir::Instruction> newExtract(new ir::Instruction(*idxItr.second.front()));
+        std::unique_ptr<ir::Instruction> newExtract(
+            new ir::Instruction(*idxItr.second.front()));
         newExtract->SetResultId(replId);
         get_def_use_mgr()->AnalyzeInstDefUse(&*newExtract);
         ++ii;
@@ -534,13 +502,13 @@ bool CommonUniformElimPass::CommonExtractElimination(ir::Function* func) {
 }
 
 bool CommonUniformElimPass::EliminateCommonUniform(ir::Function* func) {
-    bool modified = false;
+  bool modified = false;
-    modified |= UniformAccessChainConvert(func);
+  modified |= UniformAccessChainConvert(func);
-    modified |= CommonUniformLoadElimination(func);
+  modified |= CommonUniformLoadElimination(func);
-    modified |= CommonExtractElimination(func);
+  modified |= CommonExtractElimination(func);
 
-    modified |= CommonUniformLoadElimBlock(func);
+  modified |= CommonUniformLoadElimBlock(func);
-    return modified;
+  return modified;
 }
 
 void CommonUniformElimPass::Initialize(ir::IRContext* c) {
@@ -557,8 +525,8 @@ void CommonUniformElimPass::Initialize(ir::IRContext* c) {
 bool CommonUniformElimPass::AllExtensionsSupported() const {
   // If any extension not in whitelist, return false
   for (auto& ei : get_module()->extensions()) {
-    const char* extName = reinterpret_cast<const char*>(
+    const char* extName =
-        &ei.GetInOperand(0).words[0]);
+        reinterpret_cast<const char*>(&ei.GetInOperand(0).words[0]);
     if (extensions_whitelist_.find(extName) == extensions_whitelist_.end())
       return false;
   }
@@ -575,14 +543,12 @@ Pass::Status CommonUniformElimPass::ProcessImpl() {
   if (get_module()->HasCapability(SpvCapabilityAddresses))
     return Status::SuccessWithoutChange;
   // Do not process if any disallowed extensions are enabled
-  if (!AllExtensionsSupported())
+  if (!AllExtensionsSupported()) return Status::SuccessWithoutChange;
-      return Status::SuccessWithoutChange;
   // Do not process if module contains OpGroupDecorate. Additional
   // support required in KillNamesAndDecorates().
   // TODO(greg-lunarg): Add support for OpGroupDecorate
   for (auto& ai : get_module()->annotations())
-    if (ai.opcode() == SpvOpGroupDecorate)
+    if (ai.opcode() == SpvOpGroupDecorate) return Status::SuccessWithoutChange;
-      return Status::SuccessWithoutChange;
   // If non-32-bit integer type in module, terminate processing
   // TODO(): Handle non-32-bit integer constants in access chains
   for (const ir::Instruction& inst : get_module()->types_values())
@@ -607,29 +573,29 @@ Pass::Status CommonUniformElimPass::Process(ir::IRContext* c) {
 void CommonUniformElimPass::InitExtensions() {
   extensions_whitelist_.clear();
   extensions_whitelist_.insert({
-    "SPV_AMD_shader_explicit_vertex_parameter",
+      "SPV_AMD_shader_explicit_vertex_parameter",
-    "SPV_AMD_shader_trinary_minmax",
+      "SPV_AMD_shader_trinary_minmax",
-    "SPV_AMD_gcn_shader",
+      "SPV_AMD_gcn_shader",
-    "SPV_KHR_shader_ballot",
+      "SPV_KHR_shader_ballot",
-    "SPV_AMD_shader_ballot",
+      "SPV_AMD_shader_ballot",
-    "SPV_AMD_gpu_shader_half_float",
+      "SPV_AMD_gpu_shader_half_float",
-    "SPV_KHR_shader_draw_parameters",
+      "SPV_KHR_shader_draw_parameters",
-    "SPV_KHR_subgroup_vote",
+      "SPV_KHR_subgroup_vote",
-    "SPV_KHR_16bit_storage",
+      "SPV_KHR_16bit_storage",
-    "SPV_KHR_device_group",
+      "SPV_KHR_device_group",
-    "SPV_KHR_multiview",
+      "SPV_KHR_multiview",
-    "SPV_NVX_multiview_per_view_attributes",
+      "SPV_NVX_multiview_per_view_attributes",
-    "SPV_NV_viewport_array2",
+      "SPV_NV_viewport_array2",
-    "SPV_NV_stereo_view_rendering",
+      "SPV_NV_stereo_view_rendering",
-    "SPV_NV_sample_mask_override_coverage",
+      "SPV_NV_sample_mask_override_coverage",
-    "SPV_NV_geometry_shader_passthrough",
+      "SPV_NV_geometry_shader_passthrough",
-    "SPV_AMD_texture_gather_bias_lod",
+      "SPV_AMD_texture_gather_bias_lod",
-    "SPV_KHR_storage_buffer_storage_class",
+      "SPV_KHR_storage_buffer_storage_class",
-    // SPV_KHR_variable_pointers
+      // SPV_KHR_variable_pointers
-    //   Currently do not support extended pointer expressions
+      //   Currently do not support extended pointer expressions
-    "SPV_AMD_gpu_shader_int16",
+      "SPV_AMD_gpu_shader_int16",
-    "SPV_KHR_post_depth_coverage",
+      "SPV_KHR_post_depth_coverage",
-    "SPV_KHR_shader_atomic_counter_ops",
+      "SPV_KHR_shader_atomic_counter_ops",
   });
 }
 

source/opt/common_uniform_elim_pass.h

@@ -190,6 +190,11 @@ class CommonUniformElimPass : public Pass {
 
   // Extensions supported by this pass.
   std::unordered_set<std::string> extensions_whitelist_;
+
+  // Map from block to its structured successor blocks. See
+  // ComputeStructuredSuccessors() for definition.
+  std::unordered_map<const ir::BasicBlock*, std::vector<ir::BasicBlock*>>
+      block2structured_succs_;
 };
 
 }  // namespace opt

source/opt/dead_branch_elim_pass.cpp

@@ -175,7 +175,7 @@ void DeadBranchElimPass::ComputeBackEdges(
 bool DeadBranchElimPass::EliminateDeadBranches(ir::Function* func) {
   // Traverse blocks in structured order
   std::list<ir::BasicBlock*> structuredOrder;
-  ComputeStructuredOrder(func, &*func->begin(), &structuredOrder);
+  cfg()->ComputeStructuredOrder(func, &*func->begin(), &structuredOrder);
   ComputeBackEdges(structuredOrder);
   std::unordered_set<ir::BasicBlock*> elimBlocks;
   bool modified = false;
@@ -289,7 +289,7 @@ bool DeadBranchElimPass::EliminateDeadBranches(ir::Function* func) {
       pii->ForEachInId(
           [&deadPreds,&icnt,&lcnt,&lidx,this](uint32_t* idp) {
         if (icnt % 2 == 1) {
-          if (deadPreds.find(id2block_[*idp]) == deadPreds.end()) {
+          if (deadPreds.find(cfg()->block(*idp)) == deadPreds.end()) {
             ++lcnt;
             lidx = icnt - 1;
           }
@@ -311,7 +311,7 @@ bool DeadBranchElimPass::EliminateDeadBranches(ir::Function* func) {
         pii->ForEachInId(
             [&deadPreds,&icnt,&phi_in_opnds,&lastId,this](uint32_t* idp) {
           if (icnt % 2 == 1) {
-            if (deadPreds.find(id2block_[*idp]) == deadPreds.end()) {
+            if (deadPreds.find(cfg()->block(*idp)) == deadPreds.end()) {
               phi_in_opnds.push_back(
                   {spv_operand_type_t::SPV_OPERAND_TYPE_ID, {lastId}});
               phi_in_opnds.push_back(
@@ -348,15 +348,6 @@ bool DeadBranchElimPass::EliminateDeadBranches(ir::Function* func) {
 void DeadBranchElimPass::Initialize(ir::IRContext* c) {
   InitializeProcessing(c);
 
-  // Initialize function and block maps
-  id2block_.clear();
-  block2structured_succs_.clear();
-
-  // Initialize block map
-  for (auto& fn : *get_module())
-    for (auto& blk : fn)
-      id2block_[blk.id()] = &blk;
-
   // Initialize extension whitelist
   InitExtensions();
 };

source/opt/inline_pass.cpp

@@ -563,15 +563,18 @@ bool InlinePass::HasMultipleReturns(ir::Function* func) {
 void InlinePass::ComputeStructuredSuccessors(ir::Function* func) {
   // If header, make merge block first successor.
   for (auto& blk : *func) {
-    uint32_t mbid = MergeBlockIdIfAny(blk, nullptr);
+    uint32_t mbid = blk.MergeBlockIdIfAny();
-    if (mbid != 0)
+    if (mbid != 0) {
       block2structured_succs_[&blk].push_back(id2block_[mbid]);
-    // add true successors
+    }
+
+    // Add true successors.
     blk.ForEachSuccessorLabel([&blk, this](uint32_t sbid) {
       block2structured_succs_[&blk].push_back(id2block_[sbid]);
     });
   }
 }
+
 InlinePass::GetBlocksFunction InlinePass::StructuredSuccessorsFunction() {
   return [this](const ir::BasicBlock* block) {
     return &(block2structured_succs_[block]);

source/opt/inline_pass.h

@@ -166,7 +166,8 @@ class InlinePass : public Pass {
   // Map from function's result id to function.
   std::unordered_map<uint32_t, ir::Function*> id2function_;
 
-  // Map from block's label id to block.
+  // Map from block's label id to block. TODO(dnovillo): This is superfluous wrt
+  // opt::CFG. It has functionality not present in opt::CFG. Consolidate.
   std::unordered_map<uint32_t, ir::BasicBlock*> id2block_;
 
   // Set of ids of functions with multiple returns.
@@ -180,6 +181,13 @@ class InlinePass : public Pass {
 
   // result id for OpConstantFalse
   uint32_t false_id_;
+
+  // Map from block to its structured successor blocks. See
+  // ComputeStructuredSuccessors() for definition. TODO(dnovillo): This is
+  // superfluous wrt opt::CFG, but it seems to be computed in a slightly
+  // different way in the inliner. Can these be consolidated?
+  std::unordered_map<const ir::BasicBlock*, std::vector<ir::BasicBlock*>>
+      block2structured_succs_;
 };
 
 }  // namespace opt

source/opt/local_single_store_elim_pass.cpp

@@ -132,16 +132,16 @@ void LocalSingleStoreElimPass::CalculateImmediateDominators(
   // Compute Augmented CFG
   augmented_successors_map_.clear();
   augmented_predecessors_map_.clear();
-  successors_map_[&pseudo_exit_block_] = {};
+  successors_map_[cfg()->pseudo_exit_block()] = {};
-  predecessors_map_[&pseudo_entry_block_] = {};
+  predecessors_map_[cfg()->pseudo_entry_block()] = {};
   auto succ_func = [this](const ir::BasicBlock* b)
     { return &successors_map_[b]; };
   auto pred_func = [this](const ir::BasicBlock* b)
     { return &predecessors_map_[b]; };
   CFA<ir::BasicBlock>::ComputeAugmentedCFG(
     ordered_blocks,
-    &pseudo_entry_block_,
+    cfg()->pseudo_entry_block(),
-    &pseudo_exit_block_,
+    cfg()->pseudo_exit_block(),
     &augmented_successors_map_,
     &augmented_predecessors_map_,
     succ_func,

source/opt/local_single_store_elim_pass.h

@@ -68,6 +68,7 @@ class LocalSingleStoreElimPass : public MemPass {
 
   // Calculate immediate dominators for |func|'s CFG. Leaves result
   // in idom_. Entries for augmented CFG (pseudo blocks) are not created.
+  // TODO(dnovillo): Move to new CFG class.
   void CalculateImmediateDominators(ir::Function* func);
   
   // Return true if instruction in |blk0| at ordinal position |idx0|

source/opt/mem_pass.cpp

@@ -16,6 +16,7 @@
 
 #include "mem_pass.h"
 
+#include "basic_block.h"
 #include "cfa.h"
 #include "iterator.h"
 
@@ -272,19 +273,9 @@ void MemPass::InitSSARewrite(ir::Function* func) {
   visitedBlocks_.clear();
   type2undefs_.clear();
   supported_ref_vars_.clear();
-  block2structured_succs_.clear();
-  label2preds_.clear();
   label2ssa_map_.clear();
   phis_to_patch_.clear();
 
-  // Init predecessors
-  label2preds_.clear();
-  for (auto& blk : *func) {
-    uint32_t blkId = blk.id();
-    blk.ForEachSuccessorLabel(
-        [&blkId, this](uint32_t sbid) { label2preds_[sbid].push_back(blkId); });
-  }
-
   // Collect target (and non-) variable sets. Remove variables with
   // non-load/store refs from target variable set
   for (auto& blk : *func) {
@@ -319,7 +310,7 @@ bool MemPass::IsLiveAfter(uint32_t var_id, uint32_t label) const {
 void MemPass::SSABlockInitSinglePred(ir::BasicBlock* block_ptr) {
   // Copy map entry from single predecessor
   const uint32_t label = block_ptr->id();
-  const uint32_t predLabel = label2preds_[label].front();
+  const uint32_t predLabel = cfg()->preds(label).front();
   assert(visitedBlocks_.find(predLabel) != visitedBlocks_.end());
   label2ssa_map_[label] = label2ssa_map_[predLabel];
 }
@@ -342,7 +333,7 @@ void MemPass::SSABlockInitLoopHeader(
 
   // Determine the back-edge label.
   uint32_t backLabel = 0;
-  for (uint32_t predLabel : label2preds_[label])
+  for (uint32_t predLabel : cfg()->preds(label))
     if (visitedBlocks_.find(predLabel) == visitedBlocks_.end()) {
       assert(backLabel == 0);
       backLabel = predLabel;
@@ -362,7 +353,7 @@ void MemPass::SSABlockInitLoopHeader(
   // generated based on order and test results will otherwise vary across
   // platforms.
   std::map<uint32_t, uint32_t> liveVars;
-  for (uint32_t predLabel : label2preds_[label]) {
+  for (uint32_t predLabel : cfg()->preds(label)) {
     for (auto var_val : label2ssa_map_[predLabel]) {
       uint32_t varId = var_val.first;
       liveVars[varId] = var_val.second;
@@ -395,7 +386,7 @@ void MemPass::SSABlockInitLoopHeader(
     const uint32_t val0Id = var_val.second;
     bool needsPhi = false;
     if (val0Id != 0) {
-      for (uint32_t predLabel : label2preds_[label]) {
+      for (uint32_t predLabel : cfg()->preds(label)) {
         // Skip back edge predecessor.
         if (predLabel == backLabel) continue;
         const auto var_val_itr = label2ssa_map_[predLabel].find(varId);
@@ -426,7 +417,7 @@ void MemPass::SSABlockInitLoopHeader(
     // use undef.
     std::vector<ir::Operand> phi_in_operands;
     uint32_t typeId = GetPointeeTypeId(get_def_use_mgr()->GetDef(varId));
-    for (uint32_t predLabel : label2preds_[label]) {
+    for (uint32_t predLabel : cfg()->preds(label)) {
       uint32_t valId;
       if (predLabel == backLabel) {
         valId = varId;
@@ -462,7 +453,7 @@ void MemPass::SSABlockInitMultiPred(ir::BasicBlock* block_ptr) {
   // predecesors. Must be ordered map because phis are generated based on
   // order and test results will otherwise vary across platforms.
   std::map<uint32_t, uint32_t> liveVars;
-  for (uint32_t predLabel : label2preds_[label]) {
+  for (uint32_t predLabel : cfg()->preds(label)) {
     assert(visitedBlocks_.find(predLabel) != visitedBlocks_.end());
     for (auto var_val : label2ssa_map_[predLabel]) {
       const uint32_t varId = var_val.first;
@@ -477,7 +468,7 @@ void MemPass::SSABlockInitMultiPred(ir::BasicBlock* block_ptr) {
     if (!IsLiveAfter(varId, label)) continue;
     const uint32_t val0Id = var_val.second;
     bool differs = false;
-    for (uint32_t predLabel : label2preds_[label]) {
+    for (uint32_t predLabel : cfg()->preds(label)) {
       const auto var_val_itr = label2ssa_map_[predLabel].find(varId);
       // Missing values cause a difference because we'll need to create an
       // undef for that predecessor.
@@ -499,7 +490,7 @@ void MemPass::SSABlockInitMultiPred(ir::BasicBlock* block_ptr) {
     // id to the map.
     std::vector<ir::Operand> phi_in_operands;
     const uint32_t typeId = GetPointeeTypeId(get_def_use_mgr()->GetDef(varId));
-    for (uint32_t predLabel : label2preds_[label]) {
+    for (uint32_t predLabel : cfg()->preds(label)) {
       const auto var_val_itr = label2ssa_map_[predLabel].find(varId);
       // If variable not defined on this path, use undef
       const uint32_t valId = (var_val_itr != label2ssa_map_[predLabel].end())
@@ -521,11 +512,11 @@ void MemPass::SSABlockInitMultiPred(ir::BasicBlock* block_ptr) {
 }
 
 void MemPass::SSABlockInit(std::list<ir::BasicBlock*>::iterator block_itr) {
-  const size_t numPreds = label2preds_[(*block_itr)->id()].size();
+  const size_t numPreds = cfg()->preds((*block_itr)->id()).size();
   if (numPreds == 0) return;
   if (numPreds == 1)
     SSABlockInitSinglePred(*block_itr);
-  else if (IsLoopHeader(*block_itr))
+  else if ((*block_itr)->IsLoopHeader())
     SSABlockInitLoopHeader(block_itr);
   else
     SSABlockInitMultiPred(*block_itr);
@@ -557,7 +548,7 @@ bool MemPass::IsTargetVar(uint32_t varId) {
 }
 
 void MemPass::PatchPhis(uint32_t header_id, uint32_t back_id) {
-  ir::BasicBlock* header = id2block_[header_id];
+  ir::BasicBlock* header = cfg()->block(header_id);
   auto phiItr = header->begin();
   for (; phiItr->opcode() == SpvOpPhi; ++phiItr) {
     // Only patch phis that we created in a loop header.
@@ -590,8 +581,8 @@ Pass::Status MemPass::InsertPhiInstructions(ir::Function* func) {
   // TODO(dnovillo) the current Phi placement mechanism assumes structured
   // control-flow. This should be generalized
   // (https://github.com/KhronosGroup/SPIRV-Tools/issues/893).
-  if (!get_module()->HasCapability(SpvCapabilityShader))
+  assert(get_module()->HasCapability(SpvCapabilityShader) &&
-    return Status::SuccessWithoutChange;
+         "This only works on structured control flow");
 
   // Initialize the data structures used to insert Phi instructions.
   InitSSARewrite(func);
@@ -600,10 +591,14 @@ Pass::Status MemPass::InsertPhiInstructions(ir::Function* func) {
   // simplest?) to make sure all predecessors blocks are processed before
   // a block itself.
   std::list<ir::BasicBlock*> structuredOrder;
-  ComputeStructuredOrder(func, &pseudo_entry_block_, &structuredOrder);
+  cfg()->ComputeStructuredOrder(func, cfg()->pseudo_entry_block(),
+                                &structuredOrder);
   for (auto bi = structuredOrder.begin(); bi != structuredOrder.end(); ++bi) {
     // Skip pseudo entry block
-    if (*bi == &pseudo_entry_block_) continue;
+    if (cfg()->IsPseudoEntryBlock(*bi)) {
+      continue;
+    }
+
     // Initialize this block's label2ssa_map_ entry using predecessor maps.
     // Then process all stores and loads of targeted variables.
     SSABlockInit(bi);
@@ -725,7 +720,7 @@ void MemPass::RemovePhiOperands(
     assert(i % 2 == 0 && i < phi->NumOperands() - 1 &&
            "malformed Phi arguments");
 
-    ir::BasicBlock *in_block = label2block_[phi->GetSingleWordOperand(i + 1)];
+    ir::BasicBlock *in_block = cfg()->block(phi->GetSingleWordOperand(i + 1));
     if (reachable_blocks.find(in_block) == reachable_blocks.end()) {
       // If the incoming block is unreachable, remove both operands as this
       // means that the |phi| has lost an incoming edge.
@@ -798,7 +793,7 @@ bool MemPass::RemoveUnreachableBlocks(ir::Function* func) {
 
   auto mark_reachable = [&reachable_blocks, &visited_blocks, &worklist,
                          this](uint32_t label_id) {
-    auto successor = label2block_[label_id];
+    auto successor = cfg()->block(label_id);
     if (visited_blocks.count(successor) == 0) {
       reachable_blocks.insert(successor);
       worklist.push(successor);
@@ -855,16 +850,13 @@ bool MemPass::CFGCleanup(ir::Function* func) {
 }
 
 void MemPass::InitializeCFGCleanup(ir::IRContext* c) {
-  // Initialize block lookup map.
+  // Build a map between SSA names to the block they are defined in.
-  label2block_.clear();
+  //
+  // TODO(dnovillo): This is expensive and unnecessary if ir::Instruction
+  // instances could figure out what basic block they belong to. Remove this
+  // once this is possible.
   for (auto& fn : *c->module()) {
     for (auto& block : fn) {
-      label2block_[block.id()] = &block;
-
-      // Build a map between SSA names to the block they are defined in.
-      // TODO(dnovillo): This is expensive and unnecessary if ir::Instruction
-      // instances could figure out what basic block they belong to. Remove this
-      // once this is possible.
       block.ForEachInst([this, &block](ir::Instruction* inst) {
         uint32_t result_id = inst->result_id();
         if (result_id > 0) {
@@ -875,7 +867,6 @@ void MemPass::InitializeCFGCleanup(ir::IRContext* c) {
   }
 }
 
-
 }  // namespace opt
 }  // namespace spvtools
 

source/opt/mem_pass.h

@@ -132,10 +132,6 @@ class MemPass : public Pass {
   // non-target variables.
   bool IsTargetVar(uint32_t varId);
 
-  // Initialize data structures used by EliminateLocalMultiStore for
-  // function |func|, specifically block predecessors and target variables.
-  void InitSSARewrite(ir::Function* func);
-
   // Return undef in function for type. Create and insert an undef after the
   // first non-variable in the function if it doesn't already exist. Add
   // undef to function undef map.
@@ -168,6 +164,10 @@ class MemPass : public Pass {
   // predecessor map.
   void PatchPhis(uint32_t header_id, uint32_t back_id);
 
+  // Initialize data structures used by EliminateLocalMultiStore for
+  // function |func|, specifically block predecessors and target variables.
+  void InitSSARewrite(ir::Function* func);
+
   // Initialize label2ssa_map_ entry for block |block_ptr| with single
   // predecessor.
   void SSABlockInitSinglePred(ir::BasicBlock* block_ptr);
@@ -232,10 +232,6 @@ class MemPass : public Pass {
   // patching of the value for the loop back-edge.
   std::unordered_set<uint32_t> phis_to_patch_;
 
-  // Map from block's label id to block. TODO(dnovillo): Basic blocks ought to
-  // have basic blocks in their pred/succ list.
-  std::unordered_map<uint32_t, ir::BasicBlock*> label2block_;
-
   // Map from an instruction result ID to the block that holds it.
   // TODO(dnovillo): This would be unnecessary if ir::Instruction instances
   // knew what basic block they belong to.

source/opt/pass.cpp

@@ -16,7 +16,6 @@
 
 #include "pass.h"
 
-#include "cfa.h"
 #include "iterator.h"
 
 namespace spvtools {
@@ -25,22 +24,12 @@ namespace opt {
 namespace {
 
 const uint32_t kEntryPointFunctionIdInIdx = 1;
-const uint32_t kLoopMergeContinueBlockIdInIdx = 1;
-const uint32_t kLoopMergeMergeBlockIdInIdx = 0;
-const uint32_t kSelectionMergeMergeBlockIdInIdx = 0;
 const uint32_t kTypePointerTypeIdInIdx = 1;
 
-// Universal Limit of ResultID + 1
-const int kInvalidId = 0x400000;
-
 }  // namespace
 
 Pass::Pass()
-    : pseudo_entry_block_(std::unique_ptr<ir::Instruction>(
+    : consumer_(nullptr),
-          new ir::Instruction(SpvOpLabel, 0, 0, {}))),
-      pseudo_exit_block_(std::unique_ptr<ir::Instruction>(
-          new ir::Instruction(SpvOpLabel, 0, kInvalidId, {}))),
-      consumer_(nullptr),
       def_use_mgr_(nullptr),
       next_id_(0),
       context_(nullptr) {}
@@ -117,85 +106,12 @@ bool Pass::ProcessCallTreeFromRoots(
   return modified;
 }
 
-bool Pass::IsLoopHeader(ir::BasicBlock* block_ptr) const {
-  auto iItr = block_ptr->end();
-  --iItr;
-  if (iItr == block_ptr->begin())
-    return false;
-  --iItr;
-  return iItr->opcode() == SpvOpLoopMerge;
-}
-
 uint32_t Pass::GetPointeeTypeId(const ir::Instruction* ptrInst) const {
   const uint32_t ptrTypeId = ptrInst->type_id();
   const ir::Instruction* ptrTypeInst = get_def_use_mgr()->GetDef(ptrTypeId);
   return ptrTypeInst->GetSingleWordInOperand(kTypePointerTypeIdInIdx);
 }
 
-void Pass::ComputeStructuredOrder(ir::Function* func, ir::BasicBlock* root,
-                                  std::list<ir::BasicBlock*>* order) {
-  // Compute structured successors and do DFS
-  ComputeStructuredSuccessors(func);
-  auto ignore_block = [](cbb_ptr) {};
-  auto ignore_edge = [](cbb_ptr, cbb_ptr) {};
-  auto get_structured_successors = [this](const ir::BasicBlock* block) {
-    return &(block2structured_succs_[block]);
-  };
-
-  // TODO(greg-lunarg): Get rid of const_cast by making moving const
-  // out of the cfa.h prototypes and into the invoking code.
-  auto post_order = [&](cbb_ptr b) {
-    order->push_front(const_cast<ir::BasicBlock*>(b));
-  };
-  spvtools::CFA<ir::BasicBlock>::DepthFirstTraversal(
-      root, get_structured_successors, ignore_block, post_order,
-      ignore_edge);
-}
-
-void Pass::ComputeStructuredSuccessors(ir::Function *func) {
-  block2structured_succs_.clear();
-  for (auto& blk : *func) {
-    // If no predecessors in function, make successor to pseudo entry
-    if (label2preds_[blk.id()].size() == 0)
-      block2structured_succs_[&pseudo_entry_block_].push_back(&blk);
-    // If header, make merge block first successor and continue block second
-    // successor if there is one.
-    uint32_t cbid;
-    const uint32_t mbid = MergeBlockIdIfAny(blk, &cbid);
-    if (mbid != 0) {
-      block2structured_succs_[&blk].push_back(id2block_[mbid]);
-      if (cbid != 0)
-        block2structured_succs_[&blk].push_back(id2block_[cbid]);
-    }
-    // add true successors
-    blk.ForEachSuccessorLabel([&blk, this](uint32_t sbid) {
-      block2structured_succs_[&blk].push_back(id2block_[sbid]);
-    });
-  }
-}
-
-
-uint32_t Pass::MergeBlockIdIfAny(const ir::BasicBlock& blk, uint32_t* cbid) {
-  auto merge_ii = blk.cend();
-  --merge_ii;
-  if (cbid != nullptr) {
-    *cbid = 0;
-  }
-  uint32_t mbid = 0;
-  if (merge_ii != blk.cbegin()) {
-    --merge_ii;
-    if (merge_ii->opcode() == SpvOpLoopMerge) {
-      mbid = merge_ii->GetSingleWordInOperand(kLoopMergeMergeBlockIdInIdx);
-      if (cbid != nullptr) {
-        *cbid =
-            merge_ii->GetSingleWordInOperand(kLoopMergeContinueBlockIdInIdx);
-      }
-    } else if (merge_ii->opcode() == SpvOpSelectionMerge) {
-      mbid = merge_ii->GetSingleWordInOperand(kSelectionMergeMergeBlockIdInIdx);
-    }
-  }
-  return mbid;
-}
 }  // namespace opt
 }  // namespace spvtools
 

source/opt/pass.h

@@ -16,18 +16,18 @@
 #define LIBSPIRV_OPT_PASS_H_
 
 #include <algorithm>
-#include <list>
 #include <map>
 #include <queue>
 #include <unordered_map>
 #include <unordered_set>
 #include <utility>
 
+#include "basic_block.h"
+#include "cfg.h"
 #include "def_use_manager.h"
+#include "ir_context.h"
 #include "module.h"
 #include "spirv-tools/libspirv.hpp"
-#include "basic_block.h"
-#include "ir_context.h"
 
 namespace spvtools {
 namespace opt {
@@ -85,6 +85,10 @@ class Pass {
   // Returns a pointer to the current context for this pass.
   ir::IRContext* context() const { return context_; }
 
+  // Returns a pointer to the CFG for current module. TODO(dnovillo): This
+  // should belong in IRContext.
+  CFG *cfg() const { return cfg_.get(); }
+
   // Add to |todo| all ids of functions called in |func|.
   void AddCalls(ir::Function* func, std::queue<uint32_t>* todo);
 
@@ -121,37 +125,9 @@ class Pass {
     context_ = c;
     next_id_ = context_->IdBound();
     def_use_mgr_.reset(new analysis::DefUseManager(consumer(), get_module()));
-    block2structured_succs_.clear();
+    cfg_.reset(new CFG(get_module()));
-    label2preds_.clear();
-    id2block_.clear();
-    for (auto& fn : *context_->module()) {
-      for (auto& blk : fn) {
-        id2block_[blk.id()] = &blk;
-      }
-    }
   }
 
-  // Return true if |block_ptr| points to a loop header block. TODO(dnovillo)
-  // This belongs in a CFG class.
-  bool IsLoopHeader(ir::BasicBlock* block_ptr) const;
-
-  // Compute structured successors for function |func|. A block's structured
-  // successors are the blocks it branches to together with its declared merge
-  // block and continue block if it has them. When order matters, the merge
-  // block and continue block always appear first. This assures correct depth
-  // first search in the presence of early returns and kills. If the successor
-  // vector contain duplicates of the merge or continue blocks, they are safely
-  // ignored by DFS. TODO(dnovillo): This belongs in a CFG class.
-  void ComputeStructuredSuccessors(ir::Function* func);
-
-  // Compute structured block order into |structuredOrder| for |func| starting
-  // at |root|. This order has the property that dominators come before all
-  // blocks they dominate and merge blocks come after all blocks that are in
-  // the control constructs of their header. TODO(dnovillo): This belongs in
-  // a CFG class.
-  void ComputeStructuredOrder(ir::Function* func, ir::BasicBlock* root,
-                              std::list<ir::BasicBlock*>* order);
-
   // Return type id for |ptrInst|'s pointee
   uint32_t GetPointeeTypeId(const ir::Instruction* ptrInst) const;
 
@@ -163,31 +139,7 @@ class Pass {
     return retval;
   }
 
-  // Returns the id of the merge block declared by a merge instruction in this
-  // block, if any.  If none, returns zero.
-  uint32_t MergeBlockIdIfAny(const ir::BasicBlock& blk, uint32_t* cbid);
-
-  // Map from block to its structured successor blocks. See
-  // ComputeStructuredSuccessors() for definition.
-  std::unordered_map<const ir::BasicBlock*, std::vector<ir::BasicBlock*>>
-      block2structured_succs_;
-
-  // Extra block whose successors are all blocks with no predecessors
-  // in function.
-  ir::BasicBlock pseudo_entry_block_;
-
-  // Augmented CFG Exit Block.
-  ir::BasicBlock pseudo_exit_block_;
-
-  // Map from block's label id to its predecessor blocks ids
-  std::unordered_map<uint32_t, std::vector<uint32_t>> label2preds_;
-
-  // Map from block's label id to block.
-  std::unordered_map<uint32_t, ir::BasicBlock*> id2block_;
-
  private:
-  using cbb_ptr = const ir::BasicBlock*;
-
   MessageConsumer consumer_;  // Message consumer.
 
   // Def-Uses for the module we are processing
@@ -196,8 +148,11 @@ class Pass {
   // Next unused ID
   uint32_t next_id_;
 
-  // The module that the pass is being applied to.
+  // The context that this pass belongs to.
   ir::IRContext* context_;
+
+  // The CFG for all the functions in this module.
+  std::unique_ptr<CFG> cfg_;
 };
 
 }  // namespace opt