Remove constexpr from Analysis operators

* Had to remove templating from InstructionBuilder as a result
 * now preserved analyses are specified as a constructor argument
* updated tests and uses
* changed static_assert to a runtime assert
 * this should probably get further changes in the future

diff --git a/source/opt/if_conversion.cpp b/source/opt/if_conversion.cpp
index 5fefca6..9b6c24e 100644 (file)
--- a/source/opt/if_conversion.cpp
+++ b/source/opt/if_conversion.cpp
@@ -37,9 +37,10 @@ Pass::Status IfConversion::Process(ir::IRContext* c) {
         ++iter;
       }
 
-      InstructionBuilder<ir::IRContext::kAnalysisDefUse |
-                         ir::IRContext::kAnalysisInstrToBlockMapping>
-          builder(context(), &*iter);
+      InstructionBuilder builder(
+          context(), &*iter,
+          ir::IRContext::kAnalysisDefUse |
+              ir::IRContext::kAnalysisInstrToBlockMapping);
       block.ForEachPhiInst([this, &builder, &modified, &common, &to_kill,
                             dominators, &block](ir::Instruction* phi) {
         // This phi is not compatible, but subsequent phis might be.
@@ -143,10 +144,9 @@ bool IfConversion::CheckPhiUsers(ir::Instruction* phi, ir::BasicBlock* block) {
   });
 }
 
-uint32_t IfConversion::SplatCondition(
-    analysis::Vector* vec_data_ty, uint32_t cond,
-    InstructionBuilder<ir::IRContext::kAnalysisDefUse |
-                       ir::IRContext::kAnalysisInstrToBlockMapping>* builder) {
+uint32_t IfConversion::SplatCondition(analysis::Vector* vec_data_ty,
+                                      uint32_t cond,
+                                      InstructionBuilder* builder) {
   // If the data inputs to OpSelect are vectors, the condition for
   // OpSelect must be a boolean vector with the same number of
   // components. So splat the condition for the branch into a vector

diff --git a/source/opt/if_conversion.h b/source/opt/if_conversion.h
index 5d3a1ee..e70011b 100644 (file)
--- a/source/opt/if_conversion.h
+++ b/source/opt/if_conversion.h
@@ -56,10 +56,8 @@ class IfConversion : public Pass {
   // |where| indicates the location in |block| to insert the composite
   // construct. If necessary, this function will also construct the necessary
   // type instructions for the boolean vector.
-  uint32_t SplatCondition(
-      analysis::Vector* vec_data_ty, uint32_t cond,
-      InstructionBuilder<ir::IRContext::kAnalysisDefUse |
-                         ir::IRContext::kAnalysisInstrToBlockMapping>* builder);
+  uint32_t SplatCondition(analysis::Vector* vec_data_ty, uint32_t cond,
+                          InstructionBuilder* builder);
 
   // Returns true if none of |phi|'s users are in |block|.
   bool CheckPhiUsers(ir::Instruction* phi, ir::BasicBlock* block);

diff --git a/source/opt/ir_builder.h b/source/opt/ir_builder.h
index 2778473..1762308 100644 (file)
--- a/source/opt/ir_builder.h
+++ b/source/opt/ir_builder.h
@@ -24,35 +24,33 @@ namespace opt {
 
 // In SPIR-V, ids are encoded as uint16_t, this id is guaranteed to be always
 // invalid.
-constexpr uint32_t kInvalidId = std::numeric_limits<uint32_t>::max();
+const uint32_t kInvalidId = std::numeric_limits<uint32_t>::max();
 
 // Helper class to abstract instruction construction and insertion.
-// |AnalysesToPreserve| asks the InstructionBuilder to preserve requested
-// analyses.
-// Supported analyses:
+// The instruction builder can preserve the following analyses (specified via
+// the constructors):
 //   - Def-use analysis
 //   - Instruction to block analysis
-template <ir::IRContext::Analysis AnalysesToPreserve =
-              ir::IRContext::kAnalysisNone>
 class InstructionBuilder {
-  static_assert(!(AnalysesToPreserve &
-                  ~(ir::IRContext::kAnalysisDefUse |
-                    ir::IRContext::kAnalysisInstrToBlockMapping)),
-                "There some unsupported analyses");
-
  public:
   using InsertionPointTy = spvtools::ir::BasicBlock::iterator;
 
   // Creates an InstructionBuilder, all new instructions will be inserted before
   // the instruction |insert_before|.
-  InstructionBuilder(ir::IRContext* context, ir::Instruction* insert_before)
+  InstructionBuilder(
+      ir::IRContext* context, ir::Instruction* insert_before,
+      ir::IRContext::Analysis preserved_analyses = ir::IRContext::kAnalysisNone)
       : InstructionBuilder(context, context->get_instr_block(insert_before),
-                           InsertionPointTy(insert_before)) {}
+                           InsertionPointTy(insert_before),
+                           preserved_analyses) {}
 
   // Creates an InstructionBuilder, all new instructions will be inserted at the
   // end of the basic block |parent_block|.
-  InstructionBuilder(ir::IRContext* context, ir::BasicBlock* parent_block)
-      : InstructionBuilder(context, parent_block, parent_block->end()) {}
+  InstructionBuilder(
+      ir::IRContext* context, ir::BasicBlock* parent_block,
+      ir::IRContext::Analysis preserved_analyses = ir::IRContext::kAnalysisNone)
+      : InstructionBuilder(context, parent_block, parent_block->end(),
+                           preserved_analyses) {}
 
   // Creates a new selection merge instruction.
   // The id |merge_id| is the merge basic block id.
@@ -169,19 +167,27 @@ class InstructionBuilder {
   ir::IRContext* GetContext() const { return context_; }
 
   // Returns the set of preserved analyses.
-  inline static constexpr ir::IRContext::Analysis GetPreservedAnalysis() {
-    return AnalysesToPreserve;
+  inline ir::IRContext::Analysis GetPreservedAnalysis() const {
+    return preserved_analyses_;
   }
 
  private:
   InstructionBuilder(ir::IRContext* context, ir::BasicBlock* parent,
-                     InsertionPointTy insert_before)
-      : context_(context), parent_(parent), insert_before_(insert_before) {}
+                     InsertionPointTy insert_before,
+                     ir::IRContext::Analysis preserved_analyses)
+      : context_(context),
+        parent_(parent),
+        insert_before_(insert_before),
+        preserved_analyses_(preserved_analyses) {
+    assert(!(preserved_analyses_ &
+             ~(ir::IRContext::kAnalysisDefUse |
+               ir::IRContext::kAnalysisInstrToBlockMapping)));
+  }
 
   // Returns true if the users requested to update |analysis|.
-  inline static constexpr bool IsAnalysisUpdateRequested(
-      ir::IRContext::Analysis analysis) {
-    return AnalysesToPreserve & analysis;
+  inline bool IsAnalysisUpdateRequested(
+      ir::IRContext::Analysis analysis) const {
+    return preserved_analyses_ & analysis;
   }
 
   // Updates the def/use manager if the user requested it. If he did not request
@@ -203,6 +209,7 @@ class InstructionBuilder {
   ir::IRContext* context_;
   ir::BasicBlock* parent_;
   InsertionPointTy insert_before_;
+  const ir::IRContext::Analysis preserved_analyses_;
 };
 
 }  // namespace opt

diff --git a/source/opt/ir_context.h b/source/opt/ir_context.h
index bc5f643..03846c2 100644 (file)
--- a/source/opt/ir_context.h
+++ b/source/opt/ir_context.h
@@ -60,9 +60,9 @@ class IRContext {
     kAnalysisEnd = 1 << 7
   };
 
-  friend inline constexpr Analysis operator|(Analysis lhs, Analysis rhs);
+  friend inline Analysis operator|(Analysis lhs, Analysis rhs);
   friend inline Analysis& operator|=(Analysis& lhs, Analysis rhs);
-  friend inline constexpr Analysis operator<<(Analysis a, int shift);
+  friend inline Analysis operator<<(Analysis a, int shift);
   friend inline Analysis& operator<<=(Analysis& a, int shift);
 
   // Creates an |IRContext| that contains an owned |Module|
@@ -521,8 +521,8 @@ class IRContext {
   std::unique_ptr<opt::analysis::TypeManager> type_mgr_;
 };
 
-inline constexpr ir::IRContext::Analysis operator|(
-    ir::IRContext::Analysis lhs, ir::IRContext::Analysis rhs) {
+inline ir::IRContext::Analysis operator|(ir::IRContext::Analysis lhs,
+                                         ir::IRContext::Analysis rhs) {
   return static_cast<ir::IRContext::Analysis>(static_cast<int>(lhs) |
                                               static_cast<int>(rhs));
 }
@@ -534,8 +534,8 @@ inline ir::IRContext::Analysis& operator|=(ir::IRContext::Analysis& lhs,
   return lhs;
 }
 
-inline constexpr ir::IRContext::Analysis operator<<(ir::IRContext::Analysis a,
-                                                    int shift) {
+inline ir::IRContext::Analysis operator<<(ir::IRContext::Analysis a,
+                                          int shift) {
   return static_cast<ir::IRContext::Analysis>(static_cast<int>(a) << shift);
 }
 

diff --git a/test/opt/ir_builder.cpp b/test/opt/ir_builder.cpp
index e8b15bb..322bb27 100644 (file)
--- a/test/opt/ir_builder.cpp
+++ b/test/opt/ir_builder.cpp
@@ -121,7 +121,7 @@ TEST_F(IRBuilderTest, TestInsnAddition) {
     context->get_def_use_mgr();
     context->get_instr_block(nullptr);
 
-    opt::InstructionBuilder<> builder(context.get(), &*bb->begin());
+    opt::InstructionBuilder builder(context.get(), &*bb->begin());
     ir::Instruction* phi1 = builder.AddPhi(7, {9, 14});
     ir::Instruction* phi2 = builder.AddPhi(10, {16, 14});
 
@@ -143,9 +143,10 @@ TEST_F(IRBuilderTest, TestInsnAddition) {
     context->get_instr_block(nullptr);
 
     ir::BasicBlock* bb = context->cfg()->block(18);
-    opt::InstructionBuilder<ir::IRContext::kAnalysisDefUse |
-                            ir::IRContext::kAnalysisInstrToBlockMapping>
-        builder(context.get(), &*bb->begin());
+    opt::InstructionBuilder builder(
+        context.get(), &*bb->begin(),
+        ir::IRContext::kAnalysisDefUse |
+            ir::IRContext::kAnalysisInstrToBlockMapping);
     ir::Instruction* phi1 = builder.AddPhi(7, {9, 14});
     ir::Instruction* phi2 = builder.AddPhi(10, {16, 14});
 
@@ -208,7 +209,7 @@ TEST_F(IRBuilderTest, TestCondBranchAddition) {
             context.get(), SpvOpLabel, 0, context->TakeNextId(), {})))));
     ir::BasicBlock& bb_true = *fn.begin();
     {
-      opt::InstructionBuilder<> builder(context.get(), &*bb_true.begin());
+      opt::InstructionBuilder builder(context.get(), &*bb_true.begin());
       builder.AddBranch(bb_merge.id());
     }
 
@@ -217,7 +218,7 @@ TEST_F(IRBuilderTest, TestCondBranchAddition) {
             context.get(), SpvOpLabel, 0, context->TakeNextId(), {})))));
     ir::BasicBlock& bb_cond = *fn.begin();
 
-    opt::InstructionBuilder<> builder(context.get(), &bb_cond);
+    opt::InstructionBuilder builder(context.get(), &bb_cond);
     // This also test consecutive instruction insertion: merge selection +
     // branch.
     builder.AddConditionalBranch(9, bb_true.id(), bb_merge.id(), bb_merge.id());
@@ -254,7 +255,7 @@ OpFunctionEnd
       BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
   EXPECT_NE(nullptr, context);
 
-  opt::InstructionBuilder<> builder(
+  opt::InstructionBuilder builder(
       context.get(), &*context->module()->begin()->begin()->begin());
   EXPECT_NE(nullptr, builder.AddSelect(3u, 4u, 5u, 6u));
 
@@ -287,7 +288,7 @@ OpFunctionEnd
       BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text);
   EXPECT_NE(nullptr, context);
 
-  opt::InstructionBuilder<> builder(
+  opt::InstructionBuilder builder(
       context.get(), &*context->module()->begin()->begin()->begin());
   std::vector<uint32_t> ids = {3u, 4u, 4u, 3u};
   EXPECT_NE(nullptr, builder.AddCompositeConstruct(5u, ids));