Skip to content

[AutoDiff] Eliminate flow-sensitivity in '@differentiable' closure conversions. #25688

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 1 commit into from
Jun 23, 2019

Conversation

rxwei
Copy link
Contributor

@rxwei rxwei commented Jun 23, 2019

@differentiable function conversion from a non-@differentiable function is handled by inserting an AutoDiffFunctionExpr in CSApply when the contextual type is @differentiable. This is similar to C function poster conversion. One thing that we have not checked during these conversions is whether the original function expression is a literal closure expression or a reference to a func declaration. Lacking this check made @differentiable closure conversion flow-sensitive, which caused many semantic inconsistencies as described in TF-577.

This patch makes @differentiable closure conversion only valid when the from-expression is a literal closure expression or a reference to a func declaration. This eliminates all flow-sensitivity in @differentiable closure conversions.

In common cases where a user writes a higher-order function that differentiates the argument closure but forgets to make the parameter have a @differentiable or @differentiable(linear) function type, we produce a note and a fix-it to insert a @differentiable or @differentiable(linear) attribute to the parameter's type signature.

@rxwei rxwei added the tensorflow This is for "tensorflow" branch PRs. label Jun 23, 2019
@rxwei
Copy link
Contributor Author

rxwei commented Jun 23, 2019

@lattner you wanted this done a few months ago :)

@rxwei
Copy link
Contributor Author

rxwei commented Jun 23, 2019

@swift-ci please test tensorflow

Copy link
Contributor

@dan-zheng dan-zheng left a comment

Choose a reason for hiding this comment

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

Nice step towards semantic consistency!

@rxwei rxwei merged commit 6b822d8 into swiftlang:tensorflow Jun 23, 2019
@rxwei rxwei deleted the enforce-literal-conversion branch June 23, 2019 03:12
@lattner
Copy link
Contributor

lattner commented Jun 24, 2019

Awesome, this is a great step forward!

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.

3 participants