Skip to content

Conversation

@Il-Capitano
Copy link
Contributor

@Il-Capitano Il-Capitano commented Jun 11, 2025

Based on the discussion in https://discourse.llvm.org/t/rfc-execute-only-code-support-for-runtime-libraries-on-aarch64/86180 a single, global configuration option should be used to enable execute-only code generation for the runtime libraries. The new option RUNTIMES_EXECUTE_ONLY_CODE adds the -mexecute-only flag to CMAKE_C_FLAGS and CMAKE_CXX_FLAGS, which simplifies the library-level configuration needed for the runtime libraries.

Project-specific changes are still needed to handle assembly sources, e.g. in compiler-rt and libunwind.

@Il-Capitano
Copy link
Contributor Author

@ldionne
In a comment on #140555 @petrhosek suggested I change the CMake option's name to RUNTIMES_EXECUTE_ONLY_CODE. You also suggested this name previously on the discourse RFC.
Should I make that change? My main hold-up on making that change is that I don't see any other CMake option following that naming convention in current main, but if this is the convention that should be used going forward I'm happy to make that change.

@Il-Capitano Il-Capitano requested a review from petrhosek June 25, 2025 13:07
@petrhosek
Copy link
Member

@ldionne In a comment on #140555 @petrhosek suggested I change the CMake option's name to RUNTIMES_EXECUTE_ONLY_CODE. You also suggested this name previously on the discourse RFC. Should I make that change? My main hold-up on making that change is that I don't see any other CMake option following that naming convention in current main, but if this is the convention that should be used going forward I'm happy to make that change.

That's because we don't yet have a runtimes-specific variables but we expect to have more soon (see for example #134893 which introduces RUNTIMES_USE_LIBC).

@Il-Capitano
Copy link
Contributor Author

That's because we don't yet have a runtimes-specific variables but we expect to have more soon (see for example #134893 which introduces RUNTIMES_USE_LIBC).

Thanks for the explanation, I updated the name.

@Il-Capitano
Copy link
Contributor Author

Gentle ping.

I would like to get this approved and merged by the end of the month, before the LLVM 21 release. Can you review, or add other relevant reviewers, please?

@Il-Capitano
Copy link
Contributor Author

Ping

@Il-Capitano
Copy link
Contributor Author

Ping @ldionne @petrhosek

1 similar comment
@Il-Capitano
Copy link
Contributor Author

Ping @ldionne @petrhosek

@Il-Capitano
Copy link
Contributor Author

Ping

@Il-Capitano
Copy link
Contributor Author

Ping @ldionne @petrhosek

1 similar comment
@Il-Capitano
Copy link
Contributor Author

Ping @ldionne @petrhosek

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

I really dislike that we're setting CMAKE_CXX_FLAGS explicitly here, but that is something we can clean up when we have a better way to share common flags across runtimes.

@petrhosek
Copy link
Member

I really dislike that we're setting CMAKE_CXX_FLAGS explicitly here, but that is something we can clean up when we have a better way to share common flags across runtimes.

I agree, we shouldn't be directly modifying CMAKE_{C,CXX}_FLAGS, those flags are intended for the user. Instead, we should either set the flags directly on the target with target_compile_options or per-directory with add_compile_options. In this particular case, it might be possible to simply replace the set commands with add_compile_options. However, I'd be also fine merging the change as is and cleaning it up in a follow up change.

…tion on AArch64

Based on the discussion in https://discourse.llvm.org/t/rfc-execute-only-code-support-for-runtime-libraries-on-aarch64/86180
a single, global configuration option should be used to enable
execute-only code generation for the runtime libraries. The new option
`LLVM_EXECUTE_ONLY_CODE` adds the `-mexecute-only` flag to
`CMAKE_C_FLAGS` and `CMAKE_CXX_FLAGS`, which simplifies the
library-level configuration needed for the runtime libraries.

Project-specific changes are still needed to handle assembly sources,
e.g. in compiler-rt and libunwind.
@Il-Capitano
Copy link
Contributor Author

Thanks for taking a look, and approving this PR!

I've rebased to current main to make sure everything works, and I'll merge this at the end of the week.

@Il-Capitano Il-Capitano merged commit 927092b into llvm:main Nov 7, 2025
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmake Build system in general and CMake in particular

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants