[AutoDiff] Partially fix AD control flow memory leaks #25294
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR partially fixes the incorrect omission of memory cleanup in
AdjointEmitter
.Cleanups should be created per basic block in an adjoint function, starting with its arguments. Like function parameter arguments in the adjoint entry block, every phi argument except the pullback struct in a non-entry adjoint block also needs to possess a cleanup. We let the existing adjoint emission logic accumulate the cleanups until we are about to branch, where we disable the top-level cleanup for arguments in the terminator instruction and apply all of their child cleanups recursively. This logic is more general than the original single-block differentiation logic, and we should refactor
AdjointEmitter::run()
to eliminate special logic for adjoint entry and exit blocks as much as possible.This PR also unifies the argument order of the adjoint entry block with that of other adjoint blocks, making the pullback struct always the last argument. This helps us reduce special logic for the adjoint entry block later.