As discussed in the code review here, we have way too many ways to build compiler-rt, and LLVM_BUILD_EXTERNAL_COMPILER_RT should be able to be fully subsumed by LLVM_ENABLE_RUNTIMES=compiler-rt.
As such, I’m proposing that we immediately deprecate LLVM_BUILD_EXTERNAL_COMPILER_RT=On and update our CMake configurations to emit a warning on use, and after the LLVM-15 release we delete the option entirely.
This will give plenty of time for users to migrate accordingly.
(note: the missing _ in the title is intentional because Discourse wouldn’t let me post with the full option in the title)
@beanz I see the patch you’ve posted assumes that support for LLVM_BUILD_EXTERNAL_COMPILER_RT will be remove during development of LLVM 16.0. Based on previous releases I’m guessing that means you’re suggesting that support be removed from the main branch between October 2022 and March 2023, correct?
While that sounds reasonable I think this will need some internal discussion with my colleagues as I’m not sure who would be doing that work here.
My inclination would be to remove it soon after LLVM 16 branches. That should give ample time for any migration. There are some CMake cache files that use it which I’ll update myself, others like the Apple Clang ones I will leave to someone who can test with Apple’s internal build tooling.
Edit: Also, downstream users like Swift will need to update themselves.
As an extra data point, the only uses of LLVM_BUILD_EXTERNAL_COMPILER_RT in-tree today are in CMake cache files that haven’t been updated since the transition to the mono repo. None of them work out of the box today, so removing the option (or making use of it an error) wouldn’t break any in-tree use case that I can see.
I’m very much in favor. Deprecating LLVM_BUILD_EXTERNAL_COMPILER_RT and supporting only LLVM_ENABLE_RUNTIMES would enable a lot of cleanup and simplification in the compiler-rt build.
For example, compiler-rt supports building tests with a just built Clang, but does so in a way that duplicates a lot of the existing CMake functionality and introduces additional maintenance toil. LLVM_ENABLE_RUNTIMES provides a cleaner alternative that’s already well supported and used elsewhere.
I have no opinion on the speed of things, as long we can do all we want to do (upstream and downstream) with the new method, it should be easy to get people using the previous one to move to the new one.
I’d just be careful with final deprecation and removing too much on the interim implementation, as downstream projects can take their time to move (internal deadlines as well as complexity of the move).
A little more information. I just pushed a change to update the in-tree CMake cache files (excluding the Apple-specific ones). The change was mechanical and works for my local testing.
I don’t believe this migration is difficult in most cases.
As a further point that the new infrastructure largely works as a drop-in replacement, and changes to adopt are simple. I’ve posted a PR to swift to migrate off the old option:
I’m happy to work with other downstream users, but my opinion is the October deadline for moving off seems reasonable.
If build bots can migrate away, such simplification to the number of distinct build configurations looks good to me.
Also, gcc seems to import the sanitizer part of compiler-rt into their repository. I am unfamiliar with that, but I cannot find an occurrence of LLVM_BUILD_EXTERNAL_COMPILER_RT in the gcc repo, so I assume the removal is fine to them.