Skip to content

[AutoDiff] Add cloned curry thunks to generated function list. #27720

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

Merged
merged 3 commits into from
Oct 17, 2019

Conversation

dan-zheng
Copy link
Contributor

@dan-zheng dan-zheng commented Oct 16, 2019

Add cloned curry thunks to generated function list so that they will be
deleted upon clean up if any diagnostics are emitted.

Resolves TF-918: crash due to invalid cloned curry thunk that was not deleted
during clean up.


Before:

tf-918.swift:1:44: error: function is not differentiable
 _ = gradient(at: Float(1), Float(2), in: (+) as @differentiable (Float, @nondiff Float) -> Float)
                                          ~^~
<unknown>:0: note: cannot convert a direct method reference to a '@differentiable' function; use an explicit closure instead
SIL verification failed: return value type does not match return type of function: functionResultType == instResultType
... # Crash after verification failure.

After:

tf-918.swift:1:44: error: function is not differentiable
 _ = gradient(at: Float(1), Float(2), in: (+) as @differentiable (Float, @nondiff Float) -> Float)
                                          ~^~
tf-918.swift:1:44: note: cannot convert a direct method reference to a '@differentiable' function; use an explicit closure instead
 _ = gradient(at: Float(1), Float(2), in: (+) as @differentiable (Float, @nondiff Float) -> Float)
                                           ^
# No crash.

Add cloned curry thunks to generated function list so that they will be
deleted upon clean up if any diagnostics are emitted.

Resolves TF-918: crash due to invalid cloned curry thunk that was not deleted
during clean up.
@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Oct 16, 2019
@dan-zheng dan-zheng requested a review from rxwei October 16, 2019 08:50
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

The `function_ref` must also be deleted during clean up to avoid crashes
downstream SIL passes.
Use SIL function location as a fallback when SILValue has invalid location.
Fixes unknown location for lit test.
@@ -933,8 +932,8 @@ class ADContext {
return generatedFunctions;
}

SmallVector<SILValue, 32> &getGeneratedDerivativeFunctionReferences() {
return generatedDerivativeFunctionReferences;
SmallVector<SILValue, 32> &getGeneratedFunctionReferences() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: function references to generated subset parameter thunks (for linear map and derivative functions) are not tracked. This is not a problem because all references to such thunks are in other generated functions, so cleanup already deletes all such references.

I decided not to track references to subset parameter thunks for now, since there's no problem.

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

1 similar comment
@rxwei
Copy link
Contributor

rxwei commented Oct 16, 2019

@swift-ci Please test tensorflow

@rxwei rxwei merged commit 7fdc5d0 into swiftlang:tensorflow Oct 17, 2019
@dan-zheng dan-zheng deleted the TF-918 branch June 18, 2020 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tensorflow This is for "tensorflow" branch PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants