This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Support compiler-rt builtins library in tests
ClosedPublic

Authored by phosek on Jan 14 2019, 8:29 PM.

Details

Summary

We're building tests with -nostdlib which means that we need to
explicitly include the builtins library. When using libgcc (default)
we can simply include -lgcc_s on the link line, but when using
compiler-rt builtins we need a complete path to the builtins library.

This path is already available in CMake as <PROJECT>_BUILTINS_LIBRARY,
so we just need to pass that path to lit and if config.compiler_rt is
true, link it to the test.

Prior to this patch, running tests when compiler-rt is being used as
the builtins library was broken as all tests would fail to link, but
with this change running tests when compiler-rt bultins library is
being used should be supported.

Diff Detail

Repository
rCXX libc++

Event Timeline

phosek created this revision.Jan 14 2019, 8:29 PM
phosek updated this revision to Diff 184991.Feb 3 2019, 9:46 PM
phosek retitled this revision from [libc++] Support compiler-rt builtins library in tests to [CMake] Support compiler-rt builtins library in tests.
phosek added reviewers: smeenai, beanz.

Can you document how that LIT setting works and what is its intended use case?

phosek edited the summary of this revision. (Show Details)Feb 4 2019, 8:49 AM
phosek edited the summary of this revision. (Show Details)Feb 4 2019, 8:52 AM

Can you document how that LIT setting works and what is its intended use case?

Done

Can you document how that LIT setting works and what is its intended use case?

Done

I meant document it in libcxx/docs/TestingLibcxx.rst.

phosek updated this revision to Diff 185111.Feb 4 2019, 11:42 AM

Can you document how that LIT setting works and what is its intended use case?

Done

I meant document it in libcxx/docs/TestingLibcxx.rst.

Sorry about that, I wasn't aware of that file. I've also added documentation for llvm_unwinder and compiler_rt which were missing.

ldionne added inline comments.Feb 4 2019, 1:34 PM
libcxx/docs/TestingLibcxx.rst
190 ↗(On Diff #185111)

In light of this.. would it make sense to remove compiler_rt in favour of just builtins_library? Basically, compiler_rt == True if and only if Bool(builtins_library).

phosek updated this revision to Diff 185166.Feb 4 2019, 2:53 PM
phosek marked an inline comment as done.
ldionne accepted this revision.Feb 5 2019, 7:34 AM

Thanks for the cleanup!

This revision is now accepted and ready to land.Feb 5 2019, 7:34 AM
This revision was automatically updated to reflect the committed changes.