Skip to content

IRGen: internalise well known types with static linking #82170

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 1 commit into from
Jun 24, 2025

Conversation

compnerd
Copy link
Member

When statically linking the standard library ensure that we emit the well known metadata and accessors with internal linkage. This fixes a number of warnings about incorrect DLL storage when building Foundation on Windows for static linking.

@compnerd compnerd requested a review from rjmccall as a code owner June 10, 2025 23:58
@compnerd
Copy link
Member Author

@swift-ci please smoke test

When statically linking the standard library ensure that we emit the
well known metadata and accessors with internal linkage. This fixes a
number of warnings about incorrect DLL storage when building Foundation
on Windows for static linking.
@compnerd
Copy link
Member Author

@swift-ci please smoke test

@compnerd
Copy link
Member Author

@swift-ci please smoke test macOS platform

@compnerd
Copy link
Member Author

@swift-ci please smoke test Linux platform

@compnerd
Copy link
Member Author

compnerd commented Jun 20, 2025

@asl would you happen to have any ideas of why this might be causing the Differentiation build to fail only on Linux?

@asl
Copy link
Contributor

asl commented Jun 20, 2025

Interesting. Tagging @kovdan01 – he is on Linux doing stuff around autodiff.

@kovdan01
Copy link
Contributor

@asl would you happen to have any ideas of why this might be causing the Differentiation build to fail only on Linux?

@compnerd @asl TLDR: I've submitted #82412 fixing that.

Details. It looks like that we have a reinterpret_cast of SILDifferentiabilityWitness * to TypeBase *, which, to the best of my knowledge, looks totally incorrect - TypeBase is not a base class for SILDifferentiabilityWitness, and I've not found any evidence of TypeBase being stored at the beginning of memory layout of SILDifferentiabilityWitness via some dirty hacks or smth.

Such an unsafe incorrect casts explains why we see the issue only on Linux. We just try to run isCanonical on smth which is just not TypeBase, so we can get any result depending on the memory contents (and it just happens that on Mac and Windows builds we had this assertion passing, and on Linux builds - not). Also note that when investigating this issue locally, I had linux build finishing successfully or failing randomly - such non-determinism in builds passing or non-passing seems to have the same root cause.

#82412 contains a fix for that. In LinkEntity, we have Pointer and SecondaryPointer for storing payload pointers. I believe that for SIL differentiability witness, the latter should be used, while leaving Pointer as nullptr. This way, we'll not need any special checks against Kind::DifferentiabilityWitness.

@compnerd
Copy link
Member Author

Please test with following PRs:
#82412

@swift-ci please smoke test Linux platform

Copy link
Contributor

@aschwaighofer aschwaighofer left a comment

Choose a reason for hiding this comment

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

lgtm

@asl
Copy link
Contributor

asl commented Jun 23, 2025

@compnerd I'm not sure, but it seems #82412 is not pulled into your test. I was under impression that this only works for cross-repo PRs?

@compnerd
Copy link
Member Author

@swift-ci please smoke test Linux platform

@compnerd compnerd enabled auto-merge June 23, 2025 23:19
@compnerd compnerd merged commit d0b34c2 into swiftlang:main Jun 24, 2025
3 checks passed
@compnerd compnerd deleted the internalise branch June 24, 2025 02:31
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.

4 participants