[PATCH] D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler.

Dan Liew via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 4 10:56:15 PDT 2021


delcypher added a comment.

In D101681#2735499 <https://reviews.llvm.org/D101681#2735499>, @phosek wrote:

> Do we know for sure that people are using compiler-rt test suite to test runtime libraries shipped with their compiler? I'm a bit surprised by that since it's not a case I've ever considered.

Yep. I'm using it. To make testing on various Apple platforms scale we are building the toolchain once and then distributing it to various testing machines that test the runtimes of the built toolchain. Each test machine is doing a standalone configure of compiler-rt without building anything and then running tests

> I'd prefer if we always used `-nodefaultlibs` to prevent that case altogether since compiler-rt build is already exceedingly complicated and we should ideally strive to reduce the number of modes and options we have to support.

That's not how the test suites work today (apart from the `builtins/Unit` tests).

If you ignore the `config.compiler_rt_libdir = compiler_libdir` line added by this patch, then the remainder of this patch is actually restricting behaviour. If the compiler's runtime directory doesn't match the compiler-rt library directory then with this patch we actually prevent (by default) tests running with an error. Previously we just allowed testing to proceed even though we were testing the wrong runtime libraries. It just so happens that I do want to allow this "test the toolchain's runtime libraries" behaviour but I do not think it should be allowed by default. This is what the `COMPILER_RT_TEST_BUILT_LIBS` option does and why it is `ON` by default.

You're right that we could change the other test suites to pass `-nodefaultlibs` but I consider that out of scope for this patch. I am (mostly) trying to restrict behaviours that already existed. Should we need to support testing the just built libraries in a standalone build? Probably, but this doesn't work right now and I think it's much better for testing to immediately fail rather than silently testing the wrong thing. We could certainly look at changing the way the test suites find the runtime libraries in future patches.

Now the `config.compiler_rt_libdir = compiler_libdir` line is adding additional behaviour. This change

- Makes the builtin tests work when  `COMPILER_RT_TEST_BUILT_LIBS` is set to `OFF`.
- Fixes an ASan test case when  `COMPILER_RT_TEST_BUILT_LIBS` is set to `OFF`.

I technically could add this line as a separate patch but it was such a tiny change just doing that I thought it made more sense to do it in the same patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101681/new/

https://reviews.llvm.org/D101681



More information about the llvm-commits mailing list