Skip to content

Commit d907997

Browse files
Alfie-EdwardsFrederik Mellbye
authored andcommitted
Reducing excessive control dependencies from inplacing
Summary: Preventing inplace finder from creating unneeded control dependencies which sometimes resulted in cycles. - Operation `A`, has two inplace read/write users `B` and `C`. - Inplace finder looks at inplace op `B` first, and adds control dependencies to guarantee it happens after all of its peers (other users of its inplace operand, in this case `C`). - Next inplace finder looks at inplace op `C`. It sees that one of its peers (`B`) is reachable from it (because of the control dependency) so the situation cannot be resolved by inserting control dependencies. Instead it inserts a copy (`A` -> `copy` -> `C`). - Previously `copy` copied control successors from `A`, and control predecessors from `C`. Both of these are unnecessary. Test Plan: Existing inplace tests. Reviewers: #tensorflow, #framework_ip_review_-_any_oss_or_third-party_code_use_has_been_approved, samuelh Reviewed By: #tensorflow, #framework_ip_review_-_any_oss_or_third-party_code_use_has_been_approved, samuelh Maniphest Tasks: T66881 Differential Revision: https://phabricator.sourcevertex.net/D74373
1 parent 9c8ac6a commit d907997

File tree

1 file changed

+0
-11
lines changed

1 file changed

+0
-11
lines changed

tensorflow/compiler/plugin/poplar/driver/passes/inplace_finder.cc

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -377,17 +377,6 @@ StatusOr<bool> InsertCopies(InplacingState& state) {
377377
copy->shape(), CloneMethod_PreserveOrderUnlessAliases)));
378378
}
379379

380-
// If we copy result of the instruction, we have to guarantee that
381-
// result was copied before any control successors (that potentially may
382-
// modify this buffer inplace).
383-
for (auto succ : op->control_successors()) {
384-
TF_RETURN_IF_ERROR(copy->AddControlDependencyTo(succ));
385-
}
386-
// If the instruction has control predecessors, we have to guarantee
387-
// that copies are made after all predecessors.
388-
for (HloInstruction* pred : inst->control_predecessors()) {
389-
TF_RETURN_IF_ERROR(pred->AddControlDependencyTo(copy));
390-
}
391380
op->SetupDerivedInstruction(copy);
392381
TF_RETURN_IF_ERROR(inst->ReplaceOperandWith(op_index, copy));
393382
changed = true;

0 commit comments

Comments
 (0)