This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] Split the builtins CMake build to a separate file
AcceptedPublic

Authored by phosek on Nov 26 2019, 2:08 PM.

Details

Summary

When building builtins separately as part of the runtime build, we
also want to include crt. To enable that, move the builtins CMake
build into a separate file: compiler-rt/builtins/CMakeLists.txt,
which includes both builtins and crt.

Diff Detail

Event Timeline

phosek created this revision.Nov 26 2019, 2:08 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 26 2019, 2:08 PM
Herald added subscribers: llvm-commits, Restricted Project, s.egerton and 4 others. · View Herald Transcript

I assume that builtins and crt would be eventually moved to their own top-level directory (e.g. builtins or just compiler-rt) separate from sanitizers at which point this new CMakeLists.txt would become the root of that directory.

If I read this correctly, this would break existing setups that build builtins only, by pointing cmake to compiler-rt/lib/builtins? Is it possible to handle the transition more gracefully?

phosek updated this revision to Diff 234821.Dec 19 2019, 5:56 PM

If I read this correctly, this would break existing setups that build builtins only, by pointing cmake to compiler-rt/lib/builtins? Is it possible to handle the transition more gracefully?

Done

smeenai accepted this revision.Feb 4 2020, 5:33 PM

LGTM

This revision is now accepted and ready to land.Feb 4 2020, 5:33 PM
krisb added a subscriber: krisb.Oct 10 2020, 3:23 AM
daltenty added inline comments.Oct 13 2020, 2:21 PM
compiler-rt/builtins/CMakeLists.txt
30

just an fyi, looks like there are some minor changes, such as those from D87113, that need to be merged here.

daltenty accepted this revision.Oct 15 2020, 8:47 AM

LGTM, this should still be good to land (with just the minor updates)

compiler-rt/builtins/CMakeLists.txt
7

Minor nit: The version bump to 3.13.4 should be reflected here too.

Just a note on status: This patch is potentially being replaced by https://reviews.llvm.org/D89492