Skip to content

Conversation

@tshortli
Copy link

Add __isPlatformOrVariantPlatformVersionAtLeast() which is used to check OS version availability from zippered libraries on macOS. This routine is needed for complete macCatalyst support in the Swift compiler.

@egorzhdan egorzhdan requested a review from ahatanaka July 25, 2024 11:12
Copy link

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

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

I think this patch could be submitted to the upstream LLVM repo, since it isn't very specific to Swift. That would help us avoid auto-merger conflicts in the future.

@ldionne
Copy link

ldionne commented Jul 25, 2024

Indeed, please upstream this to compiler-rt directly. Not having this in upstream LLVM will cause problems if we ever want to make sure of this functionality in other components.

@tshortli
Copy link
Author

I will upstream this to upstream LLVM. However, this PR is still needed to make the change available to Swift's main branch short term.

@tshortli
Copy link
Author

Created the upstream PR here: llvm#100605

@ldionne
Copy link

ldionne commented Jul 25, 2024

This assumes that upstream will take the PR exactly as-is. If changes are needed to this function to make it acceptable upstream, we will end up not only with a merge conflict but potentially with internal stuff that relies on one definition, when upstream has a slightly different definition. This happened for e.g. __builtin_verbose_trap.

IMO the fastest way to proceed is to not rush, and to land this upstream first.

@tshortli
Copy link
Author

@ldionne I am waiting for llvm#100605 to land first but I wanted to get CI feedback for the change here.

Add `__isPlatformOrVariantPlatformVersionAtLeast()` which is used to check OS
version availability from zippered libraries on macOS. This routine is needed
for complete macCatalyst support in the Swift compiler.
@tshortli tshortli force-pushed the upstream-is-platform-or-variant-platform-version-at-least-20230725 branch from 5014f4d to f37c638 Compare July 30, 2024 21:42
@tshortli
Copy link
Author

@swift-ci test llvm

@tshortli
Copy link
Author

@swift-ci test

@tshortli
Copy link
Author

@swift-ci test Linux

@tshortli tshortli merged commit a70f7a5 into swiftlang:stable/20230725 Aug 1, 2024
@tshortli tshortli deleted the upstream-is-platform-or-variant-platform-version-at-least-20230725 branch August 1, 2024 15:33
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