Skip to content

Conversation

@kubamracek
Copy link
Contributor

We cannot allow the 'do not specialize' @_semantics attributes in embedded Swift because specialization is mandatory for soundness of the resulting compilations. The attached test crashes (triggers a "unexpected unspecialized function found during IRGen" assert) without the fix.

We have been a bit lucky that so far the parts of the stdlib that we have for embedded Swift didn't use these attributes, but I have observed that some other parts (e.g. Dictionary implementation) do use them, so we need to fix this problem.

@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek kubamracek added the embedded Embedded Swift label Jan 10, 2024
SubstitutionMap Subs = {}) {
// Ignore "do not specialize" markers in embedded Swift -- specialization is
// mandatory.
if (Callee->getModule().getOptions().EmbeddedSwift)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice if we could do this for perf annotations too, but that's probably a separate patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I wonder if there is actually anything to fix for perf annotations: (1) The stdlib only has a handful of functions with the "do not specialize" marker and they seem to mostly be slow paths that do cause allocations (and thus are not going to be usable in @noLocks or @noAllocations code anyway). (2) Non-stdlib code probably shouldn't use the "do not specialize" marker as it's an underscored attribute, and arguably it could be considered to be the user's problem if they use the marker and then try to use such a code in a perf-annotated function...

@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek
Copy link
Contributor Author

@swift-ci please test Windows platform

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

lgtm

@kubamracek kubamracek merged commit 3669b90 into swiftlang:main Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

embedded Embedded Swift

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants