Skip to content
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

[AutoDiff] Use auto-generated locations for temporary allocations. #40503

Merged
merged 1 commit into from
Dec 11, 2021

Conversation

rxwei
Copy link
Contributor

@rxwei rxwei commented Dec 10, 2021

In TangentBuilder temporary allocations are emitted in order to make generic calls to AdditiveArithmetic.zero and AdditiveArithmetic.+. When we perform accumulation of two adjoint values that correspond to a lexical value in the original function, the stack allocations require a debug variable, as we use the original instruction's location. However such a debug variable wouldn't really make sense as these temporary allocations do not represent the original variable's adjoint. With lexical borrow scopes, we hit a crasher when creating temporary allocations when accumulating adjoints for lexical begin_borrow instructions.

This PR makes TangentBuilder's temporary allocations use an auto-generated location. The existing debug info emission behavior is not changed, as we still emit debug_value when materializing an AdjointValue after any necessary adjoint accumulation.

@rxwei rxwei requested a review from gottesmm December 10, 2021 11:33
@rxwei
Copy link
Contributor Author

rxwei commented Dec 10, 2021

@swift-ci please smoke test

@BradLarson
Copy link
Contributor

This also resolves SR-15566, thanks!

@gottesmm
Copy link
Contributor

@rxwei you missed one it is also AutoDiff/validation-test/optional_property.swift

In `TangentBuilder` temporary allocations are emitted in order to make generic calls to `AdditiveArithmetic.zero` and `AdditiveArithmetic.+`. When we perform accumulation of two adjoint values that correspond to a lexical value in the original function, the stack allocations require a debug variable, as we use the original instruction's location. However such a debug variable wouldn't really make sense as these temporary allocations do not represent the original variable's adjoint. With lexical borrow scopes, we hit a crasher when creating temporary allocations when accumulating adjoints for lexical `begin_borrow` instructions.

This PR makes `TangentBuilder`'s temporary allocations use an auto-generated location. The existing debug info emission behavior is not changed, as we still emit `debug_value` when materializing an `AdjointValue` after any necessary adjoint accumulation.
@rxwei rxwei force-pushed the fix-ad-lexical-borrow-scopes branch from 3b1c327 to cf2ccf1 Compare December 10, 2021 20:32
@rxwei
Copy link
Contributor Author

rxwei commented Dec 10, 2021

Fixed. And added a test for SR-15566.

@rxwei
Copy link
Contributor Author

rxwei commented Dec 10, 2021

@swift-ci please smoke test

@rxwei
Copy link
Contributor Author

rxwei commented Dec 10, 2021

@swift-ci please test windows platform

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants