Skip to content

Add mangling support for @differentiable function types. #26595

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 2 commits into from
Aug 10, 2019
Merged

Add mangling support for @differentiable function types. #26595

merged 2 commits into from
Aug 10, 2019

Conversation

dan-zheng
Copy link
Contributor

Add mangling support for @differentiable and @differentiable(linear)
function types.

This fixes debug info builds. Previously, IRGenDebugInfo crashed because
@differentiable function types did not have full mangling support.
IRGenDebugInfoImpl::getOrCreateType computes llvm::DIType type debug
info by demangling type names, leading to a type size inconsistency for
@differentiable function types.

Change tensorflow_osx_base build preset to use --release-debuginfo.
macOS toolchains (with TensorFlow) now have debug info again.

Resolves TF-597.

@dan-zheng
Add mangling support for @differentiable function types.
53a7038 
Add mangling support for `@differentiable` and `@differentiable(linear)`
function types.

This fixes debug info builds. Previously, IRGenDebugInfo crashed because
`@differentiable` function types did not have full mangling support.
`IRGenDebugInfoImpl::getOrCreateType` computes `llvm::DIType` type debug
info by demangling type names, leading to a type size inconsistency for
`@differentiable` function types.

Change `tensorflow_osx_base` build preset to use `--release-debuginfo`.
macOS toolchains (with TensorFlow) now have debug info again.

Resolves TF-597.
@dan-zheng dan-zheng requested review from rxwei and compnerd August 10, 2019 18:57
@dan-zheng
Copy link
Contributor Author

Thank you @compnerd for spending hours to debug this together - it was a blast 🙂

@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Aug 10, 2019
@rxwei
Copy link
Contributor

rxwei commented Aug 10, 2019

Thank you @compnerd and @dan-zheng !

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@eaplatanios
Copy link

That's great! Thanks @dan-zheng and @compnerd !

Is it possible that this allows us to now build the TensorFlow module with optimizations enabled? If I recall correctly, the error when enabling optimizations previously was related to IRGen and auto-diff.

dan-zheng
dan-zheng commented Aug 10, 2019
View reviewed changes
test/SILGen/mangling.swift
@@ -184,3 +184,11 @@ func varargsVsArray(arr: [Int], n: String) { }

// CHECK-LABEL: sil hidden [ossa] @$s8mangling14varargsVsArray3arr1nySaySiGd_SStF : $@convention(thin) (@guaranteed Array<Array<Int>>, @guaranteed String) -> ()
func varargsVsArray(arr: [Int]..., n: String) { }

// SWIFT_ENABLE_TENSORFLOW
// CHECK-LABEL: sil hidden [ossa] @$s8mangling15funcVsDiffFunc12fnyS2fXE_tF : $@convention(thin) (@noescape @callee_guaranteed (Float) -> Float) -> ()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: these newly added tests actually test @differentiable function mangling logic added in #25804.

That logic is different from the mangling logic added in this patch (mangling modifiers for ImplFunctionType). I'm not sure what's the meaning of ImplFunctionType and why it exists separately from SILFunctionType mangling: why is there ImplEscaping (for ImplFunctionType) in addition to EscapingObjCBlock/EscapingAutoClosureType (for regular SILFunctionType)?

cc @slavapestov, wonder if you can clarify

@dan-zheng
Copy link
Contributor Author

Is it possible that this allows us to now build the TensorFlow module with optimizations enabled? If I recall correctly, the error when enabling optimizations previously was related to IRGen and auto-diff.

I'm not sure - could you please share the errors from building TensorFlow with -O?
The patch fixes "compiler crash when compiling @differentiable-function-typed parameters with -g and -O".

saeta
saeta reviewed Aug 10, 2019
View reviewed changes
Copy link
Contributor

@saeta saeta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @dan-zheng !!

dan-zheng
dan-zheng commented Aug 10, 2019
View reviewed changes
@eaplatanios
Copy link

@dan-zheng I'll test after this PR is merged because lots of changes have happened since last time I attempted enabling optimizations.

@dan-zheng dan-zheng merged commit 5bbe84b into swiftlang:tensorflow Aug 10, 2019
@dan-zheng dan-zheng deleted the TF-597 branch August 10, 2019 20:21
dan-zheng added a commit to dan-zheng/swift that referenced this pull request Aug 10, 2019
@dan-zheng
Add SWIFT_ENABLE_TENSORFLOW markers to docs/ABI/Mangling.rst.
a96e824 
Follow-up to:
- swiftlang#25804
- swiftlang#26595
@dan-zheng dan-zheng mentioned this pull request Aug 10, 2019
Merged
dan-zheng added a commit that referenced this pull request Aug 10, 2019
@dan-zheng
[NFC] Add SWIFT_ENABLE_TENSORFLOW markers to docs/ABI/Mangling.rst. (…
51dc28d 
…#26597)

Follow-up to:
- #25804
- #26595
compnerd
compnerd reviewed Aug 11, 2019
View reviewed changes
dan-zheng added a commit to dan-zheng/swift that referenced this pull request Aug 11, 2019
@dan-zheng
[NFC] [AutoDiff] Gardening: @differentiable function type mangling.
1968ea3 
- Add todo comments tracking TF-750.
- Optimize `SWIFT_ENABLE_TENSORFLOW` comments.
- Clarify comment in test/AutoDiff/differentiable_func_debuginfo.swift.

Addresses feedback from swiftlang#26595.
@dan-zheng dan-zheng mentioned this pull request Aug 11, 2019
Merged
dan-zheng added a commit that referenced this pull request Aug 11, 2019
@dan-zheng
[NFC] [AutoDiff] Gardening: @differentiable function type mangling. (…
8fc0992 
…#26600)

- Add todo comments tracking TF-750.
- Optimize `SWIFT_ENABLE_TENSORFLOW` comments.
- Clarify comment in test/AutoDiff/differentiable_func_debuginfo.swift.

Addresses feedback from #26595.
@dan-zheng dan-zheng mentioned this pull request Mar 27, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compnerd compnerd compnerd left review comments

@rxwei rxwei rxwei approved these changes

@saeta saeta saeta left review comments

Assignees
No one assigned
Labels
tensorflow This is for "tensorflow" branch PRs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dan-zheng @rxwei @eaplatanios @compnerd @saeta