Skip to content

[AutoDiff] Fix return type of subset parameters thunk function #67487

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
Jul 25, 2023

Conversation

asavonic
Copy link
Contributor

@asavonic asavonic commented Jul 24, 2023

The patch resolves #67402.

When the original function has a tuple result type, we should append thunkedLinearMap as the last element of the tuple to match the function declaration. Before this patch, the compiler used to wrap the original result tuple and thunkedLinearMap into another tuple, and caused the verifier error.

Before the patch:

  return %{{.*}} : $((Float, Double), @callee_guaranteed (Float) -> X.TangentVector)

After the patch:

  return %{{.*}} : $(Float, Double, @callee_guaranteed (Float) -> X.TangentVector)


// Verify the result type of a subset parameters thunk matches the declaration:
//
// CHECK: // autodiff subset parameters thunk for forward-mode derivative from f(x:)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if demangled names are printed in all build configurations. Please let me know if it is not the case.
Mangled names are a bit too long and unreadable, but we can check them instead if we have to.

The patch resolves swiftlang#67402.

When the original function has a tuple result type, we should append
thunkedLinearMap as the last element of the tuple to match the function
declaration. Before this patch, the compiler used to wrap the original result
tuple and thunkedLinearMap into another tuple, and caused the verifier error.

Before the patch:

  return %{{.*}} : $((Float, Double), @callee_guaranteed (X.TangentVector) -> Float)

After the patch:

  return %{{.*}} : $(Float, Double, @callee_guaranteed (Float) -> X.TangentVector)
@asavonic asavonic force-pushed the param-thunk-tuple branch from 7311068 to bfe56a1 Compare July 24, 2023 16:56
@asl
Copy link
Contributor

asl commented Jul 24, 2023

@swift-ci please test

@asl asl added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. AutoDiff labels Jul 24, 2023
@jkshtj
Copy link
Contributor

jkshtj commented Jul 24, 2023

Before the patch:

  return %{{.*}} : $((Float, Double), @callee_guaranteed (X.TangentVector) -> Float)

After the patch:

  return %{{.*}} : $(Float, Double, @callee_guaranteed (Float) -> X.TangentVector)

Is there a typo in the signature of the linear map in the after section?

@jkshtj
Copy link
Contributor

jkshtj commented Jul 24, 2023

Looks good otherwise. Thanks Andrew!

@asavonic
Copy link
Contributor Author

Is there a typo in the signature of the linear map in the after section?

Good catch, thank you.
I've updated the PR description, but not the commit message to avoid re-running CI. We can update it before landing.

@asl asl merged commit 5abb580 into swiftlang:main Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoDiff bug A deviation from expected or documented behavior. Also: expected but undesirable behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[AutoDiff] SILVerification failure when directly extracting arguments from a tuple received from a function
3 participants