Skip to content

Fix two issues related with emission of differentiability witnesses #80983

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
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix two issues related with emission of differentiability witnesses #80983

wants to merge 6 commits into from
+155 −29

Conversation

asl
Copy link
Contributor

@asl asl commented Apr 22, 2025

  1. When differentiable nested function (closure) is specialized by capture promotion pass ensure we generate a differentiability witness for the specialized function as well. Ensure the original witness is removed if the original function becomes dead.
  2. Differentiability witnesses for a function could originate either from its @differentiable attribute or from explicit @derivative(of:) attribute on the derivative. In the latter case the derivative itself might not be emitted, while original function is (e.g. original function is @inlineable, but derivative is @usableFromInline). Previously both cases were handled only when function body was emitted. As a result we missed witness in the aforementioned case. Ensure the differentiability witness originating from @derivative(of:) is emitted even if we're not going to emit body of the derivative.

Fixes #59135

@asl asl requested a review from rxwei April 22, 2025 09:36
@asl asl requested review from eeckstein and jckarter as code owners April 22, 2025 09:36
@asl
Copy link
Contributor Author

asl commented Apr 22, 2025

@swift-ci please test

@asl
Copy link
Contributor Author

asl commented Apr 22, 2025 •
edited
Loading

Linux build seems to be broken due to unrelated issue:

... lib/libFoundationInternationalization.so  lib/libFoundationEssentials.so  lib/lib_FoundationCollections.a  lib/lib_FoundationCShims.a  lib/lib_FoundationICU.so  /home/build-user/build/buildbot_linux/libdispatch-linux-x86_64/libswiftDispatch.so  /home/build-user/build/buildbot_linux/libdispatch-linux-x86_64/libdispatch.so  /home/build-user/build/buildbot_linux/libdispatch-linux-x86_64/libBlocksRuntime.so  -lgcc  -lgcc_s  -lc  -lgcc  -lgcc_s && :
error: link command failed with exit code 1 (use -v to see invocation)
Sources/plutil/CMakeFiles/plutil.dir/PLUContext_KeyPaths.swift.o:main.swift.o:function $sSS6plutilE19escapedKeyPathSplitSaySSGyF: error: undefined reference to '$sSy12RegexBuilderSs11SubSequenceRtzrlE5split9separator9maxSplits25omittingEmptySubsequencesSaySsGSS_SiSbtF'

Windows too:

FAILED: bin/FoundationInternationalization.dll lib/FoundationInternationalization.lib 
C:\Windows\system32\cmd.exe /C "cd . && T:\5\bin\swiftc.exe -target x86_64-unknown-windows-msvc -j 36 -num-threads 36 -emit-library -sdk "T:/Program Files/Swift/Platforms/Windows.platform/Developer/SDKs/Windows.sdk" -gnone -Xlinker /INCREMENTAL:NO -Xlinker /OPT:REF -Xlinker /OPT:ICF -O -libc MD   -Xlinker -implib:lib\FoundationInternationalization.lib   -o bin\FoundationInternationalization.dll @CMakeFiles\FoundationInternationalization.rsp && cd ."
<unknown>:0: warning: using (deprecated) legacy driver, Swift installation does not contain swift-driver at: 'C:\Users\swift-ci\jenkins\workspace\swift-PR-windows\build\5\bin\swift-driver-new.exe'
   Creating library lib\FoundationInternationalization.lib and object lib\FoundationInternationalization.exp
ICUDateFormatter.swift.obj : error LNK2019: unresolved external symbol __imp_$sSy12RegexBuilderSs11SubSequenceRtzrlE5split9separator9maxSplits25omittingEmptySubsequencesSaySsGSS_SiSbtF referenced in function $s20FoundationEssentials4DateV0A20InternationalizationE11FormatStyleV0C15FieldCollectionV14updateScheduleAD16ICUDateFormatterC0cE4InfoV06UpdateJ0Ovg
ICUDateFormatter.swift.obj : error LNK2019: unresolved external symbol __imp_$sSS17_StringProcessing14RegexComponent0C7BuilderMc referenced in function $sS2S17_StringProcessing14RegexComponent0C7BuilderWl
Duration+UnitsFormatStyle.swift.obj : error LNK2001: unresolved external symbol __imp_$sSS17_StringProcessing14RegexComponent0C7BuilderMc

TimeZone_ICU.swift.obj : error LNK2001: unresolved external symbol __imp_$sSS17_StringProcessing14RegexComponent0C7BuilderMc

@asl asl added the AutoDiff label Apr 29, 2025
@asl
Copy link
Contributor Author

asl commented Apr 29, 2025

@swift-ci please test

1 similar comment
@asl
Copy link
Contributor Author

asl commented May 2, 2025

@swift-ci please test

@asl
Copy link
Contributor Author

asl commented May 19, 2025

@swift-ci please test

@asl
Copy link
Contributor Author

asl commented May 21, 2025

@swift-ci please test windows platform

1 similar comment
@asl
Copy link
Contributor Author

asl commented Jun 3, 2025

@swift-ci please test windows platform

@JaapWijnen
Copy link
Contributor

I'm running into this one more often nowadays. Would be great if we could move this PR forward. Is there anything left to do to make the windows tests pass?

@asl
Copy link
Contributor Author

asl commented Jun 27, 2025

@JaapWijnen windows was just broken on main IIRC.

@asl
Copy link
Contributor Author

asl commented Jun 27, 2025

@swift-ci please test windows platform

asl added 5 commits June 27, 2025 17:33
@asl
Ensure we create differentiability witness when the closure is specia…
8ad3bb2 
…lized.

We can just re-create one with exactly same parameter and result indices
as specialization only touches capture parameters (which are always non-wrt).
@asl
Drop differentiability witnesses for dead functions for which bodies …
28bc664 
…are erased.

This way we won't need to check if function is dead each time we canonicalize / emit code
for differentiability witnesses later on.
@asl
Emit differentiability witness originating from custom derivative dec…
220d9bd 
…laration regardless

whether derivative body is emitted or not. This handles the cases when the function
itself if @inlinable, but the derivative is @usableFromInline, so its body it is not
emitted in the module.
@asl
Add tests
38a6225
@asl
Try to pacify Windows
9c04e62
@asl
Copy link
Contributor Author

asl commented Jun 28, 2025

@swift-ci please test

@asl
Copy link
Contributor Author

asl commented Jun 28, 2025

@swift-ci please test macos platform

1 similar comment
@asl
Copy link
Contributor Author

asl commented Jun 30, 2025

@swift-ci please test macos platform

@JaapWijnen
Copy link
Contributor

I'd love to get this merged and perhaps we can still cherry pick this into 6.2? @clackary

@asl @clackary
Typo
6d391e4 
Co-authored-by: Zachary 'Clack' Cole <[email protected]>
@asl
Copy link
Contributor Author

asl commented Jul 2, 2025

@eeckstein @jckarter Any objections on this PR?

@asl
Copy link
Contributor Author

asl commented Jul 2, 2025

@swift-ci please test

1 similar comment
@asl
Copy link
Contributor Author

asl commented Jul 3, 2025

@swift-ci please test

@asl
Copy link
Contributor Author

asl commented Jul 7, 2025

@swift-ci please test windows platform

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clackary clackary clackary left review comments

@rxwei rxwei Awaiting requested review from rxwei

@eeckstein eeckstein Awaiting requested review from eeckstein eeckstein is a code owner

@jckarter jckarter Awaiting requested review from jckarter jckarter is a code owner

Assignees
No one assigned
Labels
AutoDiff
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[AutoDiff][SILGen] Did not create differentiability witness for function
3 participants
@asl @JaapWijnen @clackary