Skip to content

[AutoDiff] [SR-14240] Align name mangling for Clang-imported declarations #38850

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 2 commits into from
Aug 12, 2021

Conversation

BradLarson
Copy link
Contributor

When importing Clang declarations, DifferentiationMangler handles their non-mangled names differently than other mangled functions. ASTMangler currently does not, and as a consequence createEmptyVJP() and createEmptyJVP() add empty JVPs and VJPs with different mangled names for IR and TBD.

When you try to add a custom VJP to one of these imported functions, like in the following:

import Darwin

@inlinable
@derivative(of: pow)
func powVJP(
  _ base: Double, _ exponent: Double
) -> (value: Double, pullback: (Double) -> (Double, Double)) {
  fatalError()
}

You hit an error of error: symbol 'powTJfSSpSr' (powTJfSSpSr) is in generated IR file, but not in TBD file due to the matching empty JVP that is created.

This PR has ASTMangler::beginManglingWithAutoDiffOriginalFunction() detect imported Clang functions and mangles them in the same way as DifferentiationMangler. That then prevents the difference in symbols that triggers the above error.

Resolves SR-14240.

@BradLarson
Copy link
Contributor Author

@swift-ci Please test.

@BradLarson BradLarson requested a review from dan-zheng August 12, 2021 19:24
@dan-zheng dan-zheng requested a review from rxwei August 12, 2021 19:35
Copy link
Contributor

@dan-zheng dan-zheng left a comment

Choose a reason for hiding this comment

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

The fix description makes sense to me, thank you!

I haven't touched mangling in a while, perhaps @rxwei could also help review?

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.

3 participants