Lunderberg commented on code in PR #14498:
URL: https://github.com/apache/tvm/pull/14498#discussion_r1157566268
##########
src/ir/module.cc:
##########
@@ -63,46 +63,46 @@ IRModule::IRModule(tvm::Map<GlobalVar, BaseFunc> functions,
}
bool IRModuleNode::SEqualReduce(const IRModuleNode* other, SEqualReducer
equal) const {
- if (!equal(this->attrs, other->attrs)) return false;
+ if (!equal(this->attrs, other->attrs, [](const auto& path) { return
path->Attr("attrs"); })) {
+ return false;
+ }
+
+ if (equal.IsPathTracingEnabled()) {
+ if ((functions.size() != other->functions.size()) ||
+ (type_definitions.size() != other->type_definitions.size())) {
+ return false;
+ }
+ }
- if (functions.size() != other->functions.size()) return false;
- // Update GlobalVar remap
+ // Define remaps for GlobalVar and GlobalTypeVar based on their
+ // string name. Early bail-out is only performed when path-tracing
+ // is disabled, as the later equality checks on the member variables
+ // will provide better error messages.
for (const auto& gv : this->GetGlobalVars()) {
- if (!other->ContainGlobalVar(gv->name_hint)) return false;
- if (!equal.DefEqual(gv, other->GetGlobalVar(gv->name_hint))) return false;
+ if (other->ContainGlobalVar(gv->name_hint)) {
+ if (!equal.DefEqual(gv, other->GetGlobalVar(gv->name_hint))) return
false;
+ } else if (!equal.IsPathTracingEnabled()) {
+ return false;
+ }
}
- // Checking functions
- for (const auto& kv : this->functions) {
- if (equal.IsPathTracingEnabled()) {
- const ObjectPathPair& obj_path_pair = equal.GetCurrentObjectPaths();
- ObjectPathPair func_paths =
{obj_path_pair->lhs_path->Attr("functions")->MapValue(kv.first),
- obj_path_pair->rhs_path->Attr("functions")
-
->MapValue(other->GetGlobalVar(kv.first->name_hint))};
- if (!equal(kv.second, other->Lookup(kv.first->name_hint), func_paths))
return false;
- } else {
- if (!equal(kv.second, other->Lookup(kv.first->name_hint))) return false;
+ for (const auto& gtv : this->GetGlobalTypeVars()) {
+ if (other->ContainGlobalTypeVar(gtv->name_hint)) {
+ if (!equal.DefEqual(gtv, other->GetGlobalTypeVar(gtv->name_hint)))
return false;
+ } else if (!equal.IsPathTracingEnabled()) {
+ return false;
}
}
- if (type_definitions.size() != other->type_definitions.size()) return false;
- // Update GlobalTypeVar remap
- for (const auto& gtv : this->GetGlobalTypeVars()) {
- if (!other->ContainGlobalTypeVar(gtv->name_hint)) return false;
- if (!equal.DefEqual(gtv, other->GetGlobalTypeVar(gtv->name_hint))) return
false;
+ // Checking functions and type definitions
+ if (!equal(this->functions, other->functions,
+ [](const auto& path) { return path->Attr("functions"); })) {
Review Comment:
It is, yes. The `IRModule`'s handler only needs to handle appending
`.functions` to the path, with the function name added by the equality check on
`Map`. This results in a path of
`<root>.functions[I.GlobalVar("func")].body...`.
I've added a unit test that verifies that the error message includes the
function name, as that's definitely a good thing to verify for
user-friendliness.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
