-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[AutoDiff] Fix SR-12641: Handle address-only types in derivative fn types #31496
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! This fixes some original vs tangent convention mismatch issues - do you think any holes remain in the codebase?
I didn't do any exhaustive thinking while writing this PR, I just stumbled on these two cases by accident. So it's possible that there are some more holes somewhere. There is this inelegant diagnostic that simply rejects a lot of these cases: https://github.com/apple/swift/blob/a021e6ca020e667ce4bc8ee174e2de1cc0d9be73/include/swift/AST/DiagnosticsSIL.def#L500 |
@swift-ci please test |
Merging now to unblock progress. Edit: actually, unblocking progress isn't deathly urgent. I'm not sure whether you'd like to merge/squash commits, so I'll hold off. |
@swift-ci Please test |
Build failed |
Build failed |
…ypes (swiftlang#31496) * The update in `SILFunctionType.cpp` fixes SR-12641 by making address-only parameters/results in differentials have indirect convention. * I updated the crasher test to use a resilient struct defined in the test, instead of `Tracked<Float>`, so that the test does not need to depend on `DifferentiationUnittest`. * The update in `VJPEmitter.cpp` fixes a similar issue with pullbacks that I discovered while investigating. * I added code that exposes this new issue to the SR-12641 crasher test.
SILFunctionType.cpp
fixes SR-12641 by making address-only parameters/results in differentials have indirect convention.Tracked<Float>
, so that the test does not need to depend onDifferentiationUnittest
.VJPEmitter.cpp
fixes a similar issue with pullbacks that I discovered while investigating.