-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[TVMScript] Add object path tracing to StructuralEqual #12101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4c28f15
0187ef8
d2fbcf3
7b1679f
9146b33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ | |
| #define TVM_NODE_STRUCTURAL_EQUAL_H_ | ||
|
|
||
| #include <tvm/node/functor.h> | ||
| #include <tvm/node/object_path.h> | ||
| #include <tvm/runtime/container/array.h> | ||
| #include <tvm/runtime/data_type.h> | ||
|
|
||
|
|
@@ -56,6 +57,27 @@ class BaseValueEqual { | |
| } | ||
| }; | ||
|
|
||
| /*! | ||
| * \brief Pair of `ObjectPath`s, one for each object being tested for structural equality. | ||
| */ | ||
| class ObjectPathPairNode : public Object { | ||
| public: | ||
| ObjectPath lhs_path; | ||
| ObjectPath rhs_path; | ||
|
|
||
| ObjectPathPairNode(ObjectPath lhs_path, ObjectPath rhs_path); | ||
|
|
||
| static constexpr const char* _type_key = "ObjectPathPair"; | ||
| TVM_DECLARE_FINAL_OBJECT_INFO(ObjectPathPairNode, Object); | ||
| }; | ||
|
|
||
| class ObjectPathPair : public ObjectRef { | ||
| public: | ||
| ObjectPathPair(ObjectPath lhs_path, ObjectPath rhs_path); | ||
|
|
||
| TVM_DEFINE_NOTNULLABLE_OBJECT_REF_METHODS(ObjectPathPair, ObjectRef, ObjectPathPairNode); | ||
| }; | ||
|
|
||
| /*! | ||
| * \brief Content-aware structural equality comparator for objects. | ||
| * | ||
|
|
@@ -99,7 +121,10 @@ class StructuralEqual : public BaseValueEqual { | |
| * equality checking. Instead, it can store the necessary equality conditions | ||
| * and check later via an internally managed stack. | ||
| */ | ||
| class SEqualReducer : public BaseValueEqual { | ||
| class SEqualReducer { | ||
| private: | ||
| struct PathTracingData; | ||
|
|
||
| public: | ||
| /*! \brief Internal handler that defines custom behaviors.. */ | ||
| class Handler { | ||
|
|
@@ -110,12 +135,24 @@ class SEqualReducer : public BaseValueEqual { | |
| * \param lhs The left operand. | ||
| * \param rhs The right operand. | ||
| * \param map_free_vars Whether do we allow remap variables if possible. | ||
| * \param current_paths Optional paths to `lhs` and `rhs` objects, for error traceability. | ||
| * | ||
| * \return false if there is an immediate failure, true otherwise. | ||
| * \note This function may save the equality condition of (lhs == rhs) in an internal | ||
| * stack and try to resolve later. | ||
| */ | ||
| virtual bool SEqualReduce(const ObjectRef& lhs, const ObjectRef& rhs, bool map_free_vars) = 0; | ||
| virtual bool SEqualReduce(const ObjectRef& lhs, const ObjectRef& rhs, bool map_free_vars, | ||
| const Optional<ObjectPathPair>& current_paths) = 0; | ||
|
|
||
| /*! | ||
| * \brief Mark the comparison as failed, but don't fail immediately. | ||
| * | ||
| * This is useful for producing better error messages when comparing containers. | ||
| * For example, if two array sizes mismatch, it's better to mark the comparison as failed | ||
| * but compare array elements anyway, so that we could find the true first mismatch. | ||
| */ | ||
| virtual void DeferFail(const ObjectPathPair& mismatch_paths) = 0; | ||
|
|
||
| /*! | ||
| * \brief Lookup the graph node equal map for vars that are already mapped. | ||
| * | ||
|
|
@@ -129,28 +166,72 @@ class SEqualReducer : public BaseValueEqual { | |
| * \brief Mark current comparison as graph node equal comparison. | ||
| */ | ||
| virtual void MarkGraphNode() = 0; | ||
| }; | ||
|
|
||
| using BaseValueEqual::operator(); | ||
| protected: | ||
| using PathTracingData = SEqualReducer::PathTracingData; | ||
| }; | ||
|
|
||
| /*! \brief default constructor */ | ||
| SEqualReducer() = default; | ||
| /*! | ||
| * \brief Constructor with a specific handler. | ||
| * \param handler The equal handler for objects. | ||
| * \param tracing_data Optional pointer to the path tracing data. | ||
| * \param map_free_vars Whether or not to map free variables. | ||
| */ | ||
| explicit SEqualReducer(Handler* handler, bool map_free_vars) | ||
| : handler_(handler), map_free_vars_(map_free_vars) {} | ||
| explicit SEqualReducer(Handler* handler, const PathTracingData* tracing_data, bool map_free_vars) | ||
| : handler_(handler), tracing_data_(tracing_data), map_free_vars_(map_free_vars) {} | ||
|
|
||
| /*! | ||
| * \brief Reduce condition to comparison of two attribute values. | ||
| * \param lhs The left operand. | ||
| * \param rhs The right operand. | ||
| * \return the immediate check result. | ||
| */ | ||
| bool operator()(const double& lhs, const double& rhs) const; | ||
| bool operator()(const int64_t& lhs, const int64_t& rhs) const; | ||
| bool operator()(const uint64_t& lhs, const uint64_t& rhs) const; | ||
| bool operator()(const int& lhs, const int& rhs) const; | ||
| bool operator()(const bool& lhs, const bool& rhs) const; | ||
| bool operator()(const std::string& lhs, const std::string& rhs) const; | ||
| bool operator()(const DataType& lhs, const DataType& rhs) const; | ||
|
|
||
| template <typename ENum, typename = typename std::enable_if<std::is_enum<ENum>::value>::type> | ||
| bool operator()(const ENum& lhs, const ENum& rhs) const { | ||
| using Underlying = typename std::underlying_type<ENum>::type; | ||
| static_assert(std::is_same<Underlying, int>::value, | ||
| "Enum must have `int` as the underlying type"); | ||
| return EnumAttrsEqual(static_cast<int>(lhs), static_cast<int>(rhs), &lhs, &rhs); | ||
| } | ||
|
|
||
| /*! | ||
| * \brief Reduce condition to comparison of two objects. | ||
| * \param lhs The left operand. | ||
| * \param rhs The right operand. | ||
| * \return the immediate check result. | ||
| */ | ||
| bool operator()(const ObjectRef& lhs, const ObjectRef& rhs) const; | ||
|
|
||
| /*! | ||
| * \brief Reduce condition to comparison of two objects. | ||
| * | ||
| * Like `operator()`, but with an additional `paths` parameter that specifies explicit object | ||
| * paths for `lhs` and `rhs`. This is useful for implementing SEqualReduce() methods for container | ||
| * objects like Array and Map, or other custom objects that store nested objects that are not | ||
| * simply attributes. | ||
| * | ||
| * Can only be called when `IsPathTracingEnabled()` is `true`. | ||
| * | ||
| * \param lhs The left operand. | ||
| * \param rhs The right operand. | ||
| * \param paths Object paths for `lhs` and `rhs`. | ||
| * \return the immediate check result. | ||
| */ | ||
| bool operator()(const ObjectRef& lhs, const ObjectRef& rhs) const { | ||
| return handler_->SEqualReduce(lhs, rhs, map_free_vars_); | ||
| bool operator()(const ObjectRef& lhs, const ObjectRef& rhs, const ObjectPathPair& paths) const { | ||
| ICHECK(IsPathTracingEnabled()) << "Path tracing must be enabled when calling this function"; | ||
| return ObjectAttrsEqual(lhs, rhs, map_free_vars_, &paths); | ||
| } | ||
|
|
||
| /*! | ||
| * \brief Reduce condition to comparison of two definitions, | ||
| * where free vars can be mapped. | ||
|
|
@@ -162,9 +243,8 @@ class SEqualReducer : public BaseValueEqual { | |
| * \param rhs The right operand. | ||
| * \return the immediate check result. | ||
| */ | ||
| bool DefEqual(const ObjectRef& lhs, const ObjectRef& rhs) { | ||
| return handler_->SEqualReduce(lhs, rhs, true); | ||
| } | ||
| bool DefEqual(const ObjectRef& lhs, const ObjectRef& rhs); | ||
|
|
||
| /*! | ||
| * \brief Reduce condition to comparison of two arrays. | ||
| * \param lhs The left operand. | ||
|
|
@@ -173,13 +253,20 @@ class SEqualReducer : public BaseValueEqual { | |
| */ | ||
| template <typename T> | ||
| bool operator()(const Array<T>& lhs, const Array<T>& rhs) const { | ||
| // quick specialization for Array to reduce amount of recursion | ||
| // depth as array comparison is pretty common. | ||
| if (lhs.size() != rhs.size()) return false; | ||
| for (size_t i = 0; i < lhs.size(); ++i) { | ||
| if (!(operator()(lhs[i], rhs[i]))) return false; | ||
| if (tracing_data_ == nullptr) { | ||
| // quick specialization for Array to reduce amount of recursion | ||
| // depth as array comparison is pretty common. | ||
| if (lhs.size() != rhs.size()) return false; | ||
| for (size_t i = 0; i < lhs.size(); ++i) { | ||
| if (!(operator()(lhs[i], rhs[i]))) return false; | ||
| } | ||
| return true; | ||
| } | ||
| return true; | ||
|
|
||
| // If tracing is enabled, fall back to the regular path | ||
| const ObjectRef& lhs_obj = lhs; | ||
| const ObjectRef& rhs_obj = rhs; | ||
| return (*this)(lhs_obj, rhs_obj); | ||
| } | ||
| /*! | ||
| * \brief Implementation for equality rule of var type objects(e.g. TypeVar, tir::Var). | ||
|
|
@@ -198,11 +285,43 @@ class SEqualReducer : public BaseValueEqual { | |
| /*! \return Get the internal handler. */ | ||
| Handler* operator->() const { return handler_; } | ||
|
|
||
| /*! \brief Check if this reducer is tracing paths to the first mismatch. */ | ||
| bool IsPathTracingEnabled() const { return tracing_data_ != nullptr; } | ||
|
|
||
| /*! | ||
| * \brief Get the paths of the currently compared objects. | ||
| * | ||
| * Can only be called when `IsPathTracingEnabled()` is true. | ||
| */ | ||
| const ObjectPathPair& GetCurrentObjectPaths() const; | ||
|
|
||
| /*! | ||
| * \brief Specify the object paths of a detected mismatch. | ||
| * | ||
| * Can only be called when `IsPathTracingEnabled()` is true. | ||
| */ | ||
| void RecordMismatchPaths(const ObjectPathPair& paths) const; | ||
|
|
||
| private: | ||
| bool EnumAttrsEqual(int lhs, int rhs, const void* lhs_address, const void* rhs_address) const; | ||
|
|
||
| bool ObjectAttrsEqual(const ObjectRef& lhs, const ObjectRef& rhs, bool map_free_vars, | ||
| const ObjectPathPair* paths) const; | ||
|
|
||
| static void GetPathsFromAttrAddressesAndStoreMismatch(const void* lhs_address, | ||
| const void* rhs_address, | ||
| const PathTracingData* tracing_data); | ||
|
|
||
| template <typename T> | ||
| static bool CompareAttributeValues(const T& lhs, const T& rhs, | ||
| const PathTracingData* tracing_data); | ||
|
Comment on lines
+311
to
+317
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious - do you think it would make more sense to move those two methods to the cc file? The primary concern I'm having is that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the actual concern about having a function template declared in the header? I don't have a strong opinion, can move these to a helper friend class if you prefer it that way.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no concrete problem in this particular case AFAICT - because template instantiation is only defined and used in a single cc file. On the other hand, in more generic usecases, we would prefer template instantiation being defined in header files so that it's discoverable by the compiler when multiple cc files refer to this method. Therefore, it's somehow a personal preference (so it's subjective, not any general requirement) that I either define both instantiation and declaration in header file, or both in cc files
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I mostly understand your point, but I'm missing one thing: why is this specific to function templates? For example, the non-template helper function Even if we go with the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah my personal (again it's just subjective) preference is that we hide anything that's not intended to be publicly used, except for non-static methods when it requires some boilerplate code (adding helper friend classes / methods). If a method is in a header file, I would prefer to document it more or less to make it easier for others to catch up |
||
|
|
||
| /*! \brief Internal class pointer. */ | ||
| Handler* handler_; | ||
| Handler* handler_ = nullptr; | ||
| /*! \brief Pointer to the current path tracing context, or nullptr if path tracing is disabled. */ | ||
| const PathTracingData* tracing_data_ = nullptr; | ||
| /*! \brief Whether or not to map free vars. */ | ||
| bool map_free_vars_; | ||
| bool map_free_vars_ = false; | ||
| }; | ||
|
|
||
| } // namespace tvm | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.