tqchen commented on code in PR #19944:
URL: https://github.com/apache/tvm/pull/19944#discussion_r3524026111
##########
src/relax/ir/binding_rewrite.cc:
##########
@@ -102,14 +108,18 @@ void DataflowBlockRewriteNode::ReplaceAllUses(Var
old_var, Var new_var) {
// new_var -> {?} | changed to old_var users
for (Var user : to_users_[old_var]) {
auto new_var_uses = to_users_[new_var];
- if (new_var_uses.end() == std::find(new_var_uses.begin(),
new_var_uses.end(), user)) {
+ if (new_var_uses.end() ==
+ std::find_if(new_var_uses.begin(), new_var_uses.end(),
+ [&](const Var& candidate) { return
candidate.same_as(user); })) {
new_var_uses.push_back(user);
}
}
Review Comment:
Confirmed. `ffi::Map::operator[]` returns the stored `ffi::Array` by value,
and `push_back` copy-on-write detached the local array, so the appended users
were not persisted.
Commit `f9a9281e66` hoists the destination user array, performs
identity-based deduplication across all transferred users, and writes it back
with `to_users_.Set(new_var, new_var_uses)` before clearing the old entry. It
also treats an existing `old_var.same_as(new_var)` replacement as a no-op after
the presence checks, preventing the shared map entry from being cleared.
The regression now proves that the destination cannot be removed after
replacement because its two transferred users remain recorded. The full Relax
binding-rewrite file passes 12/12 tests.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
