Skip to content

[5.10][AutoDiff] Fix linear map tuple types computation #69499

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 1, 2023
Merged

[5.10][AutoDiff] Fix linear map tuple types computation #69499

merged 1 commit into from
Nov 1, 2023

Conversation

asl
Copy link
Contributor

@asl asl commented Oct 30, 2023 •
edited
Loading

  • Explanation: Fixes important issue with autodiff linear map tuple type calculation & SIL lowering. Before that we may treat some tuple field types as being trivial while there were not. The issue was exposed by previous patches (already present in 5.10 branch) that fixed memory leaks in autodiff code.
  • Scope: Wide for AutoDiff code. Affects every differentiable function with control flow. W/o this patch we might end with extra releases without retains with subsequent crashes.
  • Risk: Very low: impacts only autodiff-generated code.
  • Original pull request: [AutoDiff] Fix quite subtle but nasty bug in linear map tuple types computation #68413

@asl
Fix quite subtle but nasty bug in linear map tuple types computation:
8d3e5ce 
we need lowered type for branch trace enum in order to compute linear map
tuple type. However, the lowering of branch trace enum type depends on the
types of its elements (the payloads are linear map tuples of predecessor BB).

As lowered types are cached, we cannot populate branch trace enum entries
in the end as we did before: we already used wrong lowered types for linear
map tuples.

Traverse basic blocks in reverse post-order traver order building linear
map tuples and branch tracing enumns in one go, ensuring that we've done
with predecessor BBs before processing the BB itself.
@asl asl requested a review from a team as a code owner October 30, 2023 07:44
@asl
Copy link
Contributor Author

asl commented Oct 30, 2023

@swift-ci please test

@asl asl changed the title [5.10][AutoDiff] Cherry pick 8ae5c53f6efa2bdac69f56f6be3c63c27abf8a24 to 5.10 release branch [5.10][AutoDiff] Fix linear map tuple types computation Oct 30, 2023
@asl asl added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.10 labels Oct 30, 2023
@BradLarson
Copy link
Contributor

Thank you for initiating this! This is indeed a significant concern for us, and we do have specific code in our use cases that will experience segfaults in the 5.10 branch / release without this cherry-picked fix from main.

@asl
Copy link
Contributor Author

asl commented Nov 1, 2023

@apple/swift5-branch-managers ping

@asl asl merged commit 31ecef1 into release/5.10 Nov 1, 2023
@asl asl deleted the 5.10-68413 branch November 1, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slavapestov slavapestov slavapestov approved these changes

@hborla hborla hborla approved these changes

@rxwei rxwei Awaiting requested review from rxwei

@airspeedswift airspeedswift Awaiting requested review from airspeedswift

Assignees
No one assigned
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@asl @BradLarson @slavapestov @hborla