From aaf2501ff33dbae999f770d71163f774ee3fd006 Mon Sep 17 00:00:00 2001 From: Stefano Milizia Date: Fri, 11 Sep 2020 17:11:10 +0200 Subject: [PATCH] Pass the necessary submanagers to some methods, to allow them to do some checks --- .../data_synonym_and_id_equation_facts.cpp | 27 ++++++++++++------- .../data_synonym_and_id_equation_facts.h | 13 +++++++-- source/fuzz/fact_manager/fact_manager.cpp | 16 ++++++----- .../fact_manager/irrelevant_value_facts.cpp | 23 +++++++++------- .../fact_manager/irrelevant_value_facts.h | 11 ++++++-- 5 files changed, 61 insertions(+), 29 deletions(-) diff --git a/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.cpp b/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.cpp index 877a38c67f..fbd350160c 100644 --- a/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.cpp +++ b/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.cpp @@ -13,7 +13,6 @@ // limitations under the License. #include "source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h" - #include "source/fuzz/fuzzer_util.h" namespace spvtools { @@ -52,9 +51,12 @@ 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) { + 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. @@ -62,9 +64,11 @@ void DataSynonymAndIdEquationFacts::AddFact( } 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) { + assert(!irrelevant_value_facts.IdIsIrrelevant(fact.lhs_id()) && + "Irrelevant ids are not allowed."); protobufs::DataDescriptor lhs_dd = MakeDataDescriptor(fact.lhs_id(), {}); @@ -75,8 +79,8 @@ void DataSynonymAndIdEquationFacts::AddFact( // equation. std::vector 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 +832,11 @@ bool DataSynonymAndIdEquationFacts::DataDescriptorsAreWellFormedAndComparable( get_bit_count_for_numeric_type(*type_b)); } +std::vector +DataSynonymAndIdEquationFacts::GetSynonymsForId(uint32_t id) const { + return GetSynonymsForDataDescriptor(MakeDataDescriptor(id, {})); +} + std::vector DataSynonymAndIdEquationFacts::GetSynonymsForDataDescriptor( const protobufs::DataDescriptor& data_descriptor) const { diff --git a/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h b/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h index fb15e2ce84..0eae0fb529 100644 --- a/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h +++ b/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h @@ -20,6 +20,7 @@ #include "source/fuzz/data_descriptor.h" #include "source/fuzz/equivalence_relation.h" +#include "source/fuzz/fact_manager/irrelevant_value_facts.h" #include "source/fuzz/protobufs/spirvfuzz_protobufs.h" #include "source/opt/ir_context.h" @@ -32,10 +33,18 @@ namespace fact_manager { class DataSynonymAndIdEquationFacts { public: // See method in FactManager which delegates to this method. - void AddFact(const protobufs::FactDataSynonym& fact, opt::IRContext* context); + void AddFact(const protobufs::FactDataSynonym& 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); + void AddFact(const protobufs::FactIdEquation& fact, + const IrrelevantValueFacts& irrelevant_value_facts, + opt::IRContext* context); + + // See method in FactManager which delegates to this method. + std::vector GetSynonymsForId( + uint32_t id) const; // See method in FactManager which delegates to this method. std::vector GetSynonymsForDataDescriptor( diff --git a/source/fuzz/fact_manager/fact_manager.cpp b/source/fuzz/fact_manager/fact_manager.cpp index b2886bcef5..f1436ead12 100644 --- a/source/fuzz/fact_manager/fact_manager.cpp +++ b/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 FactManager::GetConstantsAvailableFromUniformsForType( @@ -172,7 +173,7 @@ FactManager::GetSynonymsForDataDescriptor( std::vector FactManager::GetSynonymsForId( uint32_t id) const { - return GetSynonymsForDataDescriptor(MakeDataDescriptor(id, {})); + return data_synonym_and_id_equation_facts_.GetSynonymsForId(id); } bool FactManager::IsSynonymous( @@ -217,13 +218,13 @@ const std::unordered_set& FactManager::GetIrrelevantIds() const { void FactManager::AddFactValueOfPointeeIsIrrelevant(uint32_t pointer_id) { 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_); } void FactManager::AddFactIdIsIrrelevant(uint32_t result_id) { 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_); } void FactManager::AddFactIdEquation(uint32_t lhs_id, SpvOp opcode, @@ -235,7 +236,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( diff --git a/source/fuzz/fact_manager/irrelevant_value_facts.cpp b/source/fuzz/fact_manager/irrelevant_value_facts.cpp index 16146f7803..b972e973e0 100644 --- a/source/fuzz/fact_manager/irrelevant_value_facts.cpp +++ b/source/fuzz/fact_manager/irrelevant_value_facts.cpp @@ -15,25 +15,30 @@ #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" 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) { + assert(data_synonym_and_id_equation_facts.GetSynonymsForId(fact.pointer_id()) + .empty() && + "The id cannot participate in DataSynonym facts."); + // TODO: Assert that the id is 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) { + assert(data_synonym_and_id_equation_facts.GetSynonymsForId(fact.result_id()) + .empty() && + "The id cannot participate in DataSynonym facts."); + // TODO: Assert that the id is not a pointer. irrelevant_ids_.insert(fact.result_id()); } diff --git a/source/fuzz/fact_manager/irrelevant_value_facts.h b/source/fuzz/fact_manager/irrelevant_value_facts.h index 4e04faf89a..b8bd4a563f 100644 --- a/source/fuzz/fact_manager/irrelevant_value_facts.h +++ b/source/fuzz/fact_manager/irrelevant_value_facts.h @@ -24,15 +24,22 @@ namespace spvtools { namespace fuzz { namespace fact_manager { +// Forward reference to 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); + void AddFact( + const protobufs::FactPointeeValueIsIrrelevant& fact, + const DataSynonymAndIdEquationFacts& data_synonym_and_id_equation_facts); // See method in FactManager which delegates to this method. - void AddFact(const protobufs::FactIdIsIrrelevant& fact); + void AddFact( + const protobufs::FactIdIsIrrelevant& fact, + const DataSynonymAndIdEquationFacts& data_synonym_and_id_equation_facts); // See method in FactManager which delegates to this method. bool PointeeValueIsIrrelevant(uint32_t pointer_id) const;