Skip to content

Implement value witness table for @differentiable functions #60875

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
Sep 1, 2022
Merged

Conversation

asl
Copy link
Contributor

@asl asl commented Aug 31, 2022

@differentiable function is actually a triple (function, jvp, vjp). Previously normal thick function value witness table was used. As a result, for example, only function was copied, but none of differential components.

This was the cause of uninitialized memory accesses and subsequent segfaults.

Should fix (now unavailable) TF-1122

@asl
Copy link
Contributor Author

asl commented Aug 31, 2022

@swift-ci Please test

@asl asl requested review from rxwei and dan-zheng August 31, 2022 15:09
@asl
Copy link
Contributor Author

asl commented Aug 31, 2022

Tagging @BradLarson

@asl
Copy link
Contributor Author

asl commented Aug 31, 2022

@swift-ci Please test macOS platform

@asl
Copy link
Contributor Author

asl commented Aug 31, 2022

@dan-zheng @rxwei I checked locally and for me all disabled parts with #if REQUIRES_SR14042 work now. Do you happen to remember where they failed? On Windows?

@dan-zheng
Copy link
Contributor

Checking the blame: #35384

These tests fail on Windows due to an invalid parameter during the reabstraction of the generic differentiable parameters.

Since Windows tests pass, removing #if REQUIRES_SR14042 seems good.

@asl
Copy link
Contributor Author

asl commented Aug 31, 2022

Checking the blame: #35384

These tests fail on Windows due to an invalid parameter during the reabstraction of the generic differentiable parameters.

Since Windows tests pass, removing #if REQUIRES_SR14042 seems good.

Right. I need to include the last disabled test then. And likely re-run everything on CI.

I was a bit surprised to see that value witness table was missed :)

@differentiable function is actually a triple (function, jvp, vjp).
Previously normal thick function value witness table was used. As
a result, for example,  only function was copied, but none of
differential components.

This was the cause of uninitialized memory accesses and subsequent
segfaults.

Should fix now unavailable TF-1122
@asl
Copy link
Contributor Author

asl commented Aug 31, 2022

@swift-ci Please test

@asl asl merged commit c89e270 into main Sep 1, 2022
@asl asl deleted the tf1122 branch September 1, 2022 10:09
@AnthonyLatsis
Copy link
Collaborator

@rxwei does this resolve #56433?

@asl
Copy link
Contributor Author

asl commented Sep 8, 2022

@AnthonyLatsis I believe some tests still fail. But I reenabled ones that were fixed

@AnthonyLatsis
Copy link
Collaborator

@asl At the very least there aren't any more references to this issue in the codebase.

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.

4 participants