Skip to content

[compiler-rt] Adds builtins support for xros. #83484

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
Mar 9, 2024
Merged

Conversation

rohit-rao
Copy link
Contributor

Adds support for xros when compiling builtins. This is disabled by default and controlled with COMPILER_RT_ENABLE_XROS, similar to watchOS/tvOS.

@rohit-rao
Copy link
Contributor Author

@delcypher Would like to have support for xros, please take a look when you have a minute. Thanks!

@nico
Copy link
Contributor

nico commented Mar 1, 2024

@jroelofs fyi

@nico
Copy link
Contributor

nico commented Mar 2, 2024

@yln @kubamracek fyi too :)

@dtellenbach dtellenbach self-requested a review March 5, 2024 04:20
@delcypher delcypher requested a review from yln March 5, 2024 07:52
@yln yln requested review from rsundahl and thetruestblue March 5, 2024 17:27
@nico
Copy link
Contributor

nico commented Mar 6, 2024

  1. I think you want to edit compiler-rt/cmake/base-config-ix.cmake to add an option() for this, right after option(COMPILER_RT_ENABLE_TVOS "Enable building for tvOS - Experimental" Off)
  2. You probably want to make a similar change to yours in compiler-rt/cmake/config-ix.cmake as well
  3. (I'll note that you don't add a min ver flag like tvOS and watchOS do, but as far as I can tell, not even Xcode clang supports -mros-version-min= yet, so that'll have to wait until that's added and this is fine as-is.)

@rohit-rao
Copy link
Contributor Author

Updated to add the option and fix supported archs. I'll add sanitizer support in a separate CL, if that's ok.

Agreed, as far as I can tell, there is no min version flag for xros, so I left it out for now.

@ributzka
Copy link
Collaborator

ributzka commented Mar 7, 2024

  1. (I'll note that you don't add a min ver flag like tvOS and watchOS do, but as far as I can tell, not even Xcode clang supports -mros-version-min= yet, so that'll have to wait until that's added and this is fine as-is.)

I don't think there ever will be a new -m*-version-min=, so there is no need to add a new option for every new platform. The new recommended approach is to use -target (as used by the Xcode build system) or alternatively -arch in combination with -mtargetos=.

@ributzka
Copy link
Collaborator

ributzka commented Mar 7, 2024

See https://reviews.llvm.org/D106316

@nico
Copy link
Contributor

nico commented Mar 9, 2024

This has been sitting for a while and got some comments that got addressed, so I'll merge this. Shout if you want follow-up work to happen :)

@nico nico merged commit 57a2229 into llvm:main Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants