spirv-fuzz: Pass submanagers to other submanagers when necessary

source/fuzz/fact_manager/data_synonym_and_id_equation_facts.cpp

@@ -52,19 +52,26 @@ bool DataSynonymAndIdEquationFacts::OperationEquals::operator()(
 }
 
 void DataSynonymAndIdEquationFacts::AddFact(
-    const protobufs::FactDataSynonym& fact, opt::IRContext* context) {
-  // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3550)
-  //  Assert that ids are not irrelevant.
+    const protobufs::FactDataSynonym& fact,
+    const IrrelevantValueFacts& irrelevant_value_facts,
+    opt::IRContext* context) {
+  (void)irrelevant_value_facts;  // Keep release compilers happy.
+  assert(!irrelevant_value_facts.IdIsIrrelevant(fact.data1().object()) &&
+         !irrelevant_value_facts.IdIsIrrelevant(fact.data2().object()) &&
+         "Irrelevant ids cannot be synonymous with other ids.");
 
   // Add the fact, including all facts relating sub-components of the data
   // descriptors that are involved.
   AddDataSynonymFactRecursive(fact.data1(), fact.data2(), context);
 }
 
 void DataSynonymAndIdEquationFacts::AddFact(
-    const protobufs::FactIdEquation& fact, opt::IRContext* context) {
-  // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3550)
-  //  Assert that ids are not irrelevant.
+    const protobufs::FactIdEquation& fact,
+    const IrrelevantValueFacts& irrelevant_value_facts,
+    opt::IRContext* context) {
+  (void)irrelevant_value_facts;  // Keep release compilers happy.
+  assert(!irrelevant_value_facts.IdIsIrrelevant(fact.lhs_id()) &&
+         "Irrelevant ids are not allowed.");
 
   protobufs::DataDescriptor lhs_dd = MakeDataDescriptor(fact.lhs_id(), {});
 
@@ -75,8 +82,8 @@ void DataSynonymAndIdEquationFacts::AddFact(
   // equation.
   std::vector<const protobufs::DataDescriptor*> rhs_dds;
   for (auto rhs_id : fact.rhs_id()) {
-    // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3550)
-    //  Assert that ids are not irrelevant.
+    assert(!irrelevant_value_facts.IdIsIrrelevant(rhs_id) &&
+           "Irrelevant ids are not allowed.");
 
     // Register a data descriptor based on this id in the equivalence relation
     // if needed, and then record the equivalence class representative.
@@ -828,6 +835,11 @@ bool DataSynonymAndIdEquationFacts::DataDescriptorsAreWellFormedAndComparable(
           get_bit_count_for_numeric_type(*type_b));
 }
 
+std::vector<const protobufs::DataDescriptor*>
+DataSynonymAndIdEquationFacts::GetSynonymsForId(uint32_t id) const {
+  return GetSynonymsForDataDescriptor(MakeDataDescriptor(id, {}));
+}
+
 std::vector<const protobufs::DataDescriptor*>
 DataSynonymAndIdEquationFacts::GetSynonymsForDataDescriptor(
     const protobufs::DataDescriptor& data_descriptor) const {

source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h

@@ -27,15 +27,28 @@ namespace spvtools {
 namespace fuzz {
 namespace fact_manager {
 
+// Forward reference to the IrrelevantValueFacts class.
+class IrrelevantValueFacts;
+
 // The purpose of this class is to group the fields and data used to represent
 // facts about data synonyms and id equations.
 class DataSynonymAndIdEquationFacts {
  public:
   // See method in FactManager which delegates to this method.
-  void AddFact(const protobufs::FactDataSynonym& fact, opt::IRContext* context);
+  // |irrelevant_value_facts| is passed for consistency checks.
+  void AddFact(const protobufs::FactDataSynonym& fact,
+               const IrrelevantValueFacts& irrelevant_value_facts,
+               opt::IRContext* context);
+
+  // See method in FactManager which delegates to this method.
+  // |irrelevant_value_facts| is passed for consistency checks.
+  void AddFact(const protobufs::FactIdEquation& fact,
+               const IrrelevantValueFacts& irrelevant_value_facts,
+               opt::IRContext* context);
 
   // See method in FactManager which delegates to this method.
-  void AddFact(const protobufs::FactIdEquation& fact, opt::IRContext* context);
+  std::vector<const protobufs::DataDescriptor*> GetSynonymsForId(
+      uint32_t id) const;
 
   // See method in FactManager which delegates to this method.
   std::vector<const protobufs::DataDescriptor*> GetSynonymsForDataDescriptor(

source/fuzz/fact_manager/fact_manager.cpp

@@ -105,8 +105,8 @@ bool FactManager::AddFact(const fuzz::protobufs::Fact& fact,
       return constant_uniform_facts_.AddFact(fact.constant_uniform_fact(),
                                              context);
     case protobufs::Fact::kDataSynonymFact:
-      data_synonym_and_id_equation_facts_.AddFact(fact.data_synonym_fact(),
-                                                  context);
+      data_synonym_and_id_equation_facts_.AddFact(
+          fact.data_synonym_fact(), irrelevant_value_facts_, context);
       return true;
     case protobufs::Fact::kBlockIsDeadFact:
       dead_block_facts_.AddFact(fact.block_is_dead_fact());
@@ -126,7 +126,8 @@ void FactManager::AddFactDataSynonym(const protobufs::DataDescriptor& data1,
   protobufs::FactDataSynonym fact;
   *fact.mutable_data1() = data1;
   *fact.mutable_data2() = data2;
-  data_synonym_and_id_equation_facts_.AddFact(fact, context);
+  data_synonym_and_id_equation_facts_.AddFact(fact, irrelevant_value_facts_,
+                                              context);
 }
 
 std::vector<uint32_t> FactManager::GetConstantsAvailableFromUniformsForType(
@@ -172,7 +173,7 @@ FactManager::GetSynonymsForDataDescriptor(
 
 std::vector<const protobufs::DataDescriptor*> FactManager::GetSynonymsForId(
     uint32_t id) const {
-  return GetSynonymsForDataDescriptor(MakeDataDescriptor(id, {}));
+  return data_synonym_and_id_equation_facts_.GetSynonymsForId(id);
 }
 
 bool FactManager::IsSynonymous(
@@ -214,16 +215,20 @@ const std::unordered_set<uint32_t>& FactManager::GetIrrelevantIds() const {
   return irrelevant_value_facts_.GetIrrelevantIds();
 }
 
-void FactManager::AddFactValueOfPointeeIsIrrelevant(uint32_t pointer_id) {
+void FactManager::AddFactValueOfPointeeIsIrrelevant(uint32_t pointer_id,
+                                                    opt::IRContext* context) {
   protobufs::FactPointeeValueIsIrrelevant fact;
   fact.set_pointer_id(pointer_id);
-  irrelevant_value_facts_.AddFact(fact);
+  irrelevant_value_facts_.AddFact(fact, data_synonym_and_id_equation_facts_,
+                                  context);
 }
 
-void FactManager::AddFactIdIsIrrelevant(uint32_t result_id) {
+void FactManager::AddFactIdIsIrrelevant(uint32_t result_id,
+                                        opt::IRContext* context) {
   protobufs::FactIdIsIrrelevant fact;
   fact.set_result_id(result_id);
-  irrelevant_value_facts_.AddFact(fact);
+  irrelevant_value_facts_.AddFact(fact, data_synonym_and_id_equation_facts_,
+                                  context);
 }
 
 void FactManager::AddFactIdEquation(uint32_t lhs_id, SpvOp opcode,
@@ -235,7 +240,8 @@ void FactManager::AddFactIdEquation(uint32_t lhs_id, SpvOp opcode,
   for (auto an_rhs_id : rhs_id) {
     fact.add_rhs_id(an_rhs_id);
   }
-  data_synonym_and_id_equation_facts_.AddFact(fact, context);
+  data_synonym_and_id_equation_facts_.AddFact(fact, irrelevant_value_facts_,
+                                              context);
 }
 
 void FactManager::ComputeClosureOfFacts(

source/fuzz/fact_manager/fact_manager.h

@@ -66,11 +66,14 @@ class FactManager {
 
   // Records the fact that the value of the pointee associated with |pointer_id|
   // is irrelevant: it does not affect the observable behaviour of the module.
-  void AddFactValueOfPointeeIsIrrelevant(uint32_t pointer_id);
+  // |pointer_id| must exist in the module and actually be a pointer.
+  void AddFactValueOfPointeeIsIrrelevant(uint32_t pointer_id,
+                                         opt::IRContext* context);
 
   // Records a fact that the |result_id| is irrelevant (i.e. it doesn't affect
-  // the semantics of the module)
-  void AddFactIdIsIrrelevant(uint32_t result_id);
+  // the semantics of the module).
+  // |result_id| must exist in the module and actually be a pointer.
+  void AddFactIdIsIrrelevant(uint32_t result_id, opt::IRContext* context);
 
   // Records the fact that |lhs_id| is defined by the equation:
   //

source/fuzz/fact_manager/irrelevant_value_facts.cpp

@@ -15,25 +15,43 @@
 #include "source/fuzz/fact_manager/irrelevant_value_facts.h"
 
 #include "source/fuzz/data_descriptor.h"
-#include "source/fuzz/fuzzer_util.h"
+#include "source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h"
+#include "source/opt/ir_context.h"
 
 namespace spvtools {
 namespace fuzz {
 namespace fact_manager {
 
 void IrrelevantValueFacts::AddFact(
-    const protobufs::FactPointeeValueIsIrrelevant& fact) {
-  // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3550)
-  //  Assert that the id does not participate in DataSynonym facts and is a
-  //  pointer.
+    const protobufs::FactPointeeValueIsIrrelevant& fact,
+    const DataSynonymAndIdEquationFacts& data_synonym_and_id_equation_facts,
+    opt::IRContext* context) {
+  (void)data_synonym_and_id_equation_facts;  // Keep release compilers happy.
+  assert(data_synonym_and_id_equation_facts.GetSynonymsForId(fact.pointer_id())
+             .empty() &&
+         "The id cannot participate in DataSynonym facts.");
+  auto pointer_def = context->get_def_use_mgr()->GetDef(fact.pointer_id());
+  assert(pointer_def && "The id must exist in the module.");
+  auto type = context->get_type_mgr()->GetType(pointer_def->type_id());
+  (void)type;  // Keep release compilers happy.
+  assert(type && type->AsPointer() && "The id must be a pointer.");
 
   pointers_to_irrelevant_pointees_ids_.insert(fact.pointer_id());
 }
 
-void IrrelevantValueFacts::AddFact(const protobufs::FactIdIsIrrelevant& fact) {
-  // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3550)
-  //  Assert that the id does not participate in DataSynonym facts and is not a
-  //  pointer.
+void IrrelevantValueFacts::AddFact(
+    const protobufs::FactIdIsIrrelevant& fact,
+    const DataSynonymAndIdEquationFacts& data_synonym_and_id_equation_facts,
+    opt::IRContext* context) {
+  (void)data_synonym_and_id_equation_facts;  // Keep release compilers happy.
+  assert(data_synonym_and_id_equation_facts.GetSynonymsForId(fact.result_id())
+             .empty() &&
+         "The id cannot participate in DataSynonym facts.");
+  auto pointer_def = context->get_def_use_mgr()->GetDef(fact.result_id());
+  assert(pointer_def && "The id must exist in the module.");
+  auto type = context->get_type_mgr()->GetType(pointer_def->type_id());
+  (void)type;  // Keep release compilers happy.
+  assert(type && !type->AsPointer() && "The id must not be a pointer.");
 
   irrelevant_ids_.insert(fact.result_id());
 }

source/fuzz/fact_manager/irrelevant_value_facts.h

@@ -24,15 +24,28 @@ namespace spvtools {
 namespace fuzz {
 namespace fact_manager {
 
+// Forward reference to the DataSynonymAndIdEquationFacts class.
+class DataSynonymAndIdEquationFacts;
+
 // The purpose of this class is to group the fields and data used to represent
 // facts about various irrelevant values in the module.
 class IrrelevantValueFacts {
  public:
   // See method in FactManager which delegates to this method.
-  void AddFact(const protobufs::FactPointeeValueIsIrrelevant& fact);
+  // |data_synonym_and_id_equation_facts| and |context| are passed for
+  // consistency checks.
+  void AddFact(
+      const protobufs::FactPointeeValueIsIrrelevant& fact,
+      const DataSynonymAndIdEquationFacts& data_synonym_and_id_equation_facts,
+      opt::IRContext* context);
 
   // See method in FactManager which delegates to this method.
-  void AddFact(const protobufs::FactIdIsIrrelevant& fact);
+  // |data_synonym_and_id_equation_facts| and |context| are passed for
+  // consistency checks.
+  void AddFact(
+      const protobufs::FactIdIsIrrelevant& fact,
+      const DataSynonymAndIdEquationFacts& data_synonym_and_id_equation_facts,
+      opt::IRContext* context);
 
   // See method in FactManager which delegates to this method.
   bool PointeeValueIsIrrelevant(uint32_t pointer_id) const;

source/fuzz/transformation_access_chain.cpp

@@ -339,7 +339,7 @@ void TransformationAccessChain::Apply(
   if (transformation_context->GetFactManager()->PointeeValueIsIrrelevant(
           message_.pointer_id())) {
     transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant(
-        message_.fresh_id());
+        message_.fresh_id(), ir_context);
   }
 }
 

source/fuzz/transformation_add_constant_boolean.cpp

@@ -53,7 +53,7 @@ void TransformationAddConstantBoolean::Apply(
 
   if (message_.is_irrelevant()) {
     transformation_context->GetFactManager()->AddFactIdIsIrrelevant(
-        message_.fresh_id());
+        message_.fresh_id(), ir_context);
   }
 }
 

source/fuzz/transformation_add_constant_composite.cpp

@@ -122,7 +122,7 @@ void TransformationAddConstantComposite::Apply(
 
   if (message_.is_irrelevant()) {
     transformation_context->GetFactManager()->AddFactIdIsIrrelevant(
-        message_.fresh_id());
+        message_.fresh_id(), ir_context);
   }
 }
 

source/fuzz/transformation_add_constant_scalar.cpp

@@ -80,7 +80,7 @@ void TransformationAddConstantScalar::Apply(
 
   if (message_.is_irrelevant()) {
     transformation_context->GetFactManager()->AddFactIdIsIrrelevant(
-        message_.fresh_id());
+        message_.fresh_id(), ir_context);
   }
 }
 

source/fuzz/transformation_add_copy_memory.cpp

@@ -132,18 +132,18 @@ void TransformationAddCopyMemory::Apply(
 
   fuzzerutil::UpdateModuleIdBound(ir_context, message_.fresh_id());
 
+  // Make sure our changes are analyzed
+  ir_context->InvalidateAnalysesExceptFor(
+      opt::IRContext::Analysis::kAnalysisNone);
+
   // Even though the copy memory instruction will - at least temporarily - lead
   // to the destination and source pointers referring to identical values, this
   // fact is not guaranteed to hold throughout execution of the SPIR-V code
   // since the source pointer could be over-written. We thus assume nothing
   // about the destination pointer, and record this fact so that the destination
   // pointer can be used freely by other fuzzer passes.
   transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant(
-      message_.fresh_id());
-
-  // Make sure our changes are analyzed
-  ir_context->InvalidateAnalysesExceptFor(
-      opt::IRContext::Analysis::kAnalysisNone);
+      message_.fresh_id(), ir_context);
 }
 
 protobufs::Transformation TransformationAddCopyMemory::ToMessage() const {

source/fuzz/transformation_add_function.cpp

@@ -168,30 +168,6 @@ void TransformationAddFunction::Apply(
   (void)(success);  // Keep release builds happy (otherwise they may complain
                     // that |success| is not used).
 
-  // Record the fact that all pointer parameters and variables declared in the
-  // function should be regarded as having irrelevant values.  This allows other
-  // passes to store arbitrarily to such variables, and to pass them freely as
-  // parameters to other functions knowing that it is OK if they get
-  // over-written.
-  for (auto& instruction : message_.instruction()) {
-    switch (instruction.opcode()) {
-      case SpvOpFunctionParameter:
-        if (ir_context->get_def_use_mgr()
-                ->GetDef(instruction.result_type_id())
-                ->opcode() == SpvOpTypePointer) {
-          transformation_context->GetFactManager()
-              ->AddFactValueOfPointeeIsIrrelevant(instruction.result_id());
-        }
-        break;
-      case SpvOpVariable:
-        transformation_context->GetFactManager()
-            ->AddFactValueOfPointeeIsIrrelevant(instruction.result_id());
-        break;
-      default:
-        break;
-    }
-  }
-
   if (message_.is_livesafe()) {
     // Make the function livesafe, which also should succeed.
     success = TryToMakeFunctionLivesafe(ir_context, *transformation_context);
@@ -214,6 +190,32 @@ void TransformationAddFunction::Apply(
     }
   }
   ir_context->InvalidateAnalysesExceptFor(opt::IRContext::kAnalysisNone);
+
+  // Record the fact that all pointer parameters and variables declared in the
+  // function should be regarded as having irrelevant values.  This allows other
+  // passes to store arbitrarily to such variables, and to pass them freely as
+  // parameters to other functions knowing that it is OK if they get
+  // over-written.
+  for (auto& instruction : message_.instruction()) {
+    switch (instruction.opcode()) {
+      case SpvOpFunctionParameter:
+        if (ir_context->get_def_use_mgr()
+                ->GetDef(instruction.result_type_id())
+                ->opcode() == SpvOpTypePointer) {
+          transformation_context->GetFactManager()
+              ->AddFactValueOfPointeeIsIrrelevant(instruction.result_id(),
+                                                  ir_context);
+        }
+        break;
+      case SpvOpVariable:
+        transformation_context->GetFactManager()
+            ->AddFactValueOfPointeeIsIrrelevant(instruction.result_id(),
+                                                ir_context);
+        break;
+      default:
+        break;
+    }
+  }
 }
 
 protobufs::Transformation TransformationAddFunction::ToMessage() const {

source/fuzz/transformation_add_global_variable.cpp

@@ -98,15 +98,15 @@ void TransformationAddGlobalVariable::Apply(
       static_cast<SpvStorageClass>(message_.storage_class()),
       message_.initializer_id());
 
-  if (message_.value_is_irrelevant()) {
-    transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant(
-        message_.fresh_id());
-  }
-
   // We have added an instruction to the module, so need to be careful about the
   // validity of existing analyses.
   ir_context->InvalidateAnalysesExceptFor(
       opt::IRContext::Analysis::kAnalysisNone);
+
+  if (message_.value_is_irrelevant()) {
+    transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant(
+        message_.fresh_id(), ir_context);
+  }
 }
 
 protobufs::Transformation TransformationAddGlobalVariable::ToMessage() const {