Validator structured flow checks: back-edge, constructs

Skip structured control flow chekcs for non-shader capability.

Fix infinite loop in dominator algorithm when there's an
unreachable block.

source/val/BasicBlock.cpp

@@ -35,25 +35,38 @@ namespace libspirv {
 BasicBlock::BasicBlock(uint32_t id)
     : id_(id),
       immediate_dominator_(nullptr),
+      immediate_post_dominator_(nullptr),
       predecessors_(),
       successors_(),
+      type_(0),
       reachable_(false) {}
 
 void BasicBlock::SetImmediateDominator(BasicBlock* dom_block) {
   immediate_dominator_ = dom_block;
 }
 
+void BasicBlock::SetImmediatePostDominator(BasicBlock* pdom_block) {
+  immediate_post_dominator_ = pdom_block;
+}
+
 const BasicBlock* BasicBlock::GetImmediateDominator() const {
   return immediate_dominator_;
 }
 
+const BasicBlock* BasicBlock::GetImmediatePostDominator() const {
+  return immediate_post_dominator_;
+}
+
 BasicBlock* BasicBlock::GetImmediateDominator() { return immediate_dominator_; }
+BasicBlock* BasicBlock::GetImmediatePostDominator() {
+  return immediate_post_dominator_;
+}
 
-void BasicBlock::RegisterSuccessors(vector<BasicBlock*> next_blocks) {
+void BasicBlock::RegisterSuccessors(const vector<BasicBlock*>& next_blocks) {
   for (auto& block : next_blocks) {
     block->predecessors_.push_back(this);
     successors_.push_back(block);
-    if (block->reachable_ == false) block->set_reachability(reachable_);
+    if (block->reachable_ == false) block->set_reachable(reachable_);
   }
 }
 
@@ -63,24 +76,29 @@ void BasicBlock::RegisterBranchInstruction(SpvOp branch_instruction) {
 }
 
 BasicBlock::DominatorIterator::DominatorIterator() : current_(nullptr) {}
-BasicBlock::DominatorIterator::DominatorIterator(const BasicBlock* block)
-    : current_(block) {}
+
+BasicBlock::DominatorIterator::DominatorIterator(
+    const BasicBlock* block,
+    std::function<const BasicBlock*(const BasicBlock*)> dominator_func)
+    : current_(block), dom_func_(dominator_func) {}
 
 BasicBlock::DominatorIterator& BasicBlock::DominatorIterator::operator++() {
-  if (current_ == current_->GetImmediateDominator()) {
+  if (current_ == dom_func_(current_)) {
     current_ = nullptr;
   } else {
-    current_ = current_->GetImmediateDominator();
+    current_ = dom_func_(current_);
   }
   return *this;
 }
 
 const BasicBlock::DominatorIterator BasicBlock::dom_begin() const {
-  return DominatorIterator(this);
+  return DominatorIterator(
+      this, [](const BasicBlock* b) { return b->GetImmediateDominator(); });
 }
 
 BasicBlock::DominatorIterator BasicBlock::dom_begin() {
-  return DominatorIterator(this);
+  return DominatorIterator(
+      this, [](const BasicBlock* b) { return b->GetImmediateDominator(); });
 }
 
 const BasicBlock::DominatorIterator BasicBlock::dom_end() const {
@@ -91,6 +109,24 @@ BasicBlock::DominatorIterator BasicBlock::dom_end() {
   return DominatorIterator();
 }
 
+const BasicBlock::DominatorIterator BasicBlock::pdom_begin() const {
+  return DominatorIterator(
+      this, [](const BasicBlock* b) { return b->GetImmediatePostDominator(); });
+}
+
+BasicBlock::DominatorIterator BasicBlock::pdom_begin() {
+  return DominatorIterator(
+    this, [](const BasicBlock* b) { return b->GetImmediatePostDominator(); });
+}
+
+const BasicBlock::DominatorIterator BasicBlock::pdom_end() const {
+  return DominatorIterator();
+}
+
+BasicBlock::DominatorIterator BasicBlock::pdom_end() {
+  return DominatorIterator();
+}
+
 bool operator==(const BasicBlock::DominatorIterator& lhs,
                 const BasicBlock::DominatorIterator& rhs) {
   return lhs.current_ == rhs.current_;

source/val/BasicBlock.h

@@ -30,10 +30,24 @@
 #include "spirv/1.1/spirv.h"
 
 #include <cstdint>
+
+#include <bitset>
+#include <functional>
 #include <vector>
 
 namespace libspirv {
 
+enum BlockType : uint32_t {
+  kBlockTypeUndefined,
+  kBlockTypeHeader,
+  kBlockTypeLoop,
+  kBlockTypeMerge,
+  kBlockTypeBreak,
+  kBlockTypeContinue,
+  kBlockTypeReturn,
+  kBlockTypeCOUNT  ///< Total number of block types. (must be the last element)
+};
+
 // This class represents a basic block in a SPIR-V module
 class BasicBlock {
  public:
@@ -61,27 +75,53 @@ class BasicBlock {
   /// Returns the successors of the BasicBlock
   std::vector<BasicBlock*>* get_successors() { return &successors_; }
 
-  /// Returns true if the  block should be reachable in the CFG
+  /// Returns true if the block is reachable in the CFG
   bool is_reachable() const { return reachable_; }
 
-  void set_reachability(bool reachability) { reachable_ = reachability; }
+  /// Returns true if BasicBlock is of the given type
+  bool is_type(BlockType type) const {
+    if (type == kBlockTypeUndefined) return type_.none();
+    return type_.test(type);
+  }
+
+  /// Sets the reachability of the basic block in the CFG
+  void set_reachable(bool reachability) { reachable_ = reachability; }
+
+  /// Sets the type of the BasicBlock
+  void set_type(BlockType type) {
+    if (type == kBlockTypeUndefined)
+      type_.reset();
+    else
+      type_.set(type);
+  }
 
   /// Sets the immedate dominator of this basic block
   ///
   /// @param[in] dom_block The dominator block
   void SetImmediateDominator(BasicBlock* dom_block);
 
+  /// Sets the immedate post dominator of this basic block
+  ///
+  /// @param[in] pdom_block The post dominator block
+  void SetImmediatePostDominator(BasicBlock* pdom_block);
+
   /// Returns the immedate dominator of this basic block
   BasicBlock* GetImmediateDominator();
 
   /// Returns the immedate dominator of this basic block
   const BasicBlock* GetImmediateDominator() const;
 
+  /// Returns the immedate post dominator of this basic block
+  BasicBlock* GetImmediatePostDominator();
+
+  /// Returns the immedate post dominator of this basic block
+  const BasicBlock* GetImmediatePostDominator() const;
+
   /// Ends the block without a successor
   void RegisterBranchInstruction(SpvOp branch_instruction);
 
   /// Adds @p next BasicBlocks as successors of this BasicBlock
-  void RegisterSuccessors(std::vector<BasicBlock*> next = {});
+  void RegisterSuccessors(const std::vector<BasicBlock*>& next = {});
 
   /// Returns true if the id of the BasicBlock matches
   bool operator==(const BasicBlock& other) const { return other.id_ == id_; }
@@ -91,7 +131,7 @@ class BasicBlock {
 
   /// @brief A BasicBlock dominator iterator class
   ///
-  /// This iterator will iterate over the dominators of the block
+  /// This iterator will iterate over the (post)dominators of the block
   class DominatorIterator
       : public std::iterator<std::forward_iterator_tag, BasicBlock*> {
    public:
@@ -104,8 +144,12 @@ class BasicBlock {
     /// @brief Constructs an iterator for the given block which points to
     ///        @p block
     ///
-    /// @param block The block which is referenced by the iterator
-    explicit DominatorIterator(const BasicBlock* block);
+    /// @param block          The block which is referenced by the iterator
+    /// @param dominator_func This function will be called to get the immediate
+    ///                       (post)dominator of the current block
+    DominatorIterator(
+        const BasicBlock* block,
+        std::function<const BasicBlock*(const BasicBlock*)> dominator_func);
 
     /// @brief Advances the iterator
     DominatorIterator& operator++();
@@ -118,29 +162,56 @@ class BasicBlock {
 
    private:
     const BasicBlock* current_;
+    std::function<const BasicBlock*(const BasicBlock*)> dom_func_;
   };
 
-  /// Returns an iterator which points to the current block
+  /// Returns a dominator iterator which points to the current block
   const DominatorIterator dom_begin() const;
+
+  /// Returns a dominator iterator which points to the current block
   DominatorIterator dom_begin();
 
-  /// Returns an iterator which points to one element past the first block
+  /// Returns a dominator iterator which points to one element past the first
+  /// block
   const DominatorIterator dom_end() const;
+
+  /// Returns a dominator iterator which points to one element past the first
+  /// block
   DominatorIterator dom_end();
 
+  /// Returns a post dominator iterator which points to the current block
+  const DominatorIterator pdom_begin() const;
+  /// Returns a post dominator iterator which points to the current block
+  DominatorIterator pdom_begin();
+
+  /// Returns a post dominator iterator which points to one element past the
+  /// last block
+  const DominatorIterator pdom_end() const;
+
+  /// Returns a post dominator iterator which points to one element past the
+  /// last block
+  DominatorIterator pdom_end();
+
  private:
   /// Id of the BasicBlock
   const uint32_t id_;
 
   /// Pointer to the immediate dominator of the BasicBlock
   BasicBlock* immediate_dominator_;
 
+  /// Pointer to the immediate dominator of the BasicBlock
+  BasicBlock* immediate_post_dominator_;
+
   /// The set of predecessors of the BasicBlock
   std::vector<BasicBlock*> predecessors_;
 
   /// The set of successors of the BasicBlock
   std::vector<BasicBlock*> successors_;
 
+  /// The type of the block
+  std::bitset<kBlockTypeCOUNT - 1> type_;
+
+  /// True if the block is reachable in the CFG
   bool reachable_;
 };
 

source/val/Construct.cpp

@@ -26,19 +26,51 @@
 
 #include "val/Construct.h"
 
+#include <cassert>
+#include <cstddef>
+
 namespace libspirv {
 
-Construct::Construct(BasicBlock* header_block, BasicBlock* merge_block,
-                     BasicBlock* continue_block)
-    : header_block_(header_block),
-      merge_block_(merge_block),
-      continue_block_(continue_block) {}
+Construct::Construct(ConstructType type, BasicBlock* entry,
+                     BasicBlock* exit, std::vector<Construct*> constructs)
+    : type_(type),
+      corresponding_constructs_(constructs),
+      entry_block_(entry),
+      exit_block_(exit) {}
+
+ConstructType Construct::get_type() const { return type_; }
+
+const std::vector<Construct*>& Construct::get_corresponding_constructs() const {
+  return corresponding_constructs_;
+}
+std::vector<Construct*>& Construct::get_corresponding_constructs() {
+  return corresponding_constructs_;
+}
+
+bool ValidateConstructSize(ConstructType type, size_t size) {
+  switch (type) {
+    case ConstructType::kSelection: return size == 0;
+    case ConstructType::kContinue:  return size == 1;
+    case ConstructType::kLoop:      return size == 1;
+    case ConstructType::kCase:      return size >= 1;
+    default: assert(1 == 0 && "Type not defined");
+  }
+  return false;
+}
+
+void Construct::set_corresponding_constructs(
+    std::vector<Construct*> constructs) {
+  assert(ValidateConstructSize(type_, constructs.size()));
+  corresponding_constructs_ = constructs;
+}
+
+const BasicBlock* Construct::get_entry() const { return entry_block_; }
+BasicBlock* Construct::get_entry() { return entry_block_; }
 
-const BasicBlock* Construct::get_header() const { return header_block_; }
-const BasicBlock* Construct::get_merge() const { return merge_block_; }
-const BasicBlock* Construct::get_continue() const { return continue_block_; }
+const BasicBlock* Construct::get_exit() const { return exit_block_; }
+BasicBlock* Construct::get_exit() { return exit_block_; }
 
-BasicBlock* Construct::get_header() { return header_block_; }
-BasicBlock* Construct::get_merge() { return merge_block_; }
-BasicBlock* Construct::get_continue() { return continue_block_; }
+void Construct::set_exit(BasicBlock* exit_block) {
+  exit_block_ = exit_block;
 }
+}  /// namespace libspirv