Fix ownership issues with sequences of partial_apply's in AutoDiff closure specialization pass
Each partial_apply consumes its arguments, therefore we should never release intermediate ones in the sequence of closures.
Fixes #78847
I also added end-to-end swift test to test the pass, apparently there are no tests for this.
@swift-ci please test
The explanation of wrong behavior is in https://github.com/swiftlang/swift/issues/78847#issuecomment-2726038126
Reenabling it back also gives huge performance boost. We are having (on benchmark from https://github.com/swiftlang/swift/pull/80665)
Before:
Benchmark_O-arm64-apple-macosx13.0 AutoDiffBuildingSimulator.Reverse
# TEST SAMPLES MIN MEDIAN MAX
53 AutoDiffBuildingSimulator.Reverse 1 17.174 17.174 17.174
After:
Benchmark_O-arm64-apple-macosx13.0 AutoDiffBuildingSimulator.Reverse
# TEST SAMPLES MIN MEDIAN MAX
53 AutoDiffBuildingSimulator.Reverse 1 4.670 4.670 4.670
@swift-ci please test
Tagging @JaapWijnen
Nice! Also the performance boost. Is this boost related to the loss in performance we saw in January @asl ? Or separate and thus an improvement on top of the performance we had in early January?
@JaapWijnen Correct. This brings the performance back to January figures
@eeckstein Here is what happened:
- https://github.com/swiftlang/swift/issues/78847 disabled the autodiff closure specialization for SIL in OSSA form. But still it triggered earlier for code in non-OSSA form and required transformation happened (not sure why we did not see lifetime checker errors though...)
- Later, the conversion to OSSA become much earlier. And therefore the pass always run on OSSA input. That together with https://github.com/swiftlang/swift/issues/78847 effectively disabled it
- Turned out there were no e2e tests for the functionality (only unit tests for the pass itself), so this went unnoticed for quite some time.
Looks good from what I can tell. Thanks for fixing!
@swift-ci please test
@swift-ci please smoke test