[PATCH] D74133: Fix for PR38025

Petr Hosek via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 7 11:31:32 PST 2020


phosek added a subscriber: chandlerc.
phosek added a comment.

In D74133#1864219 <https://reviews.llvm.org/D74133#1864219>, @RVP wrote:

> Also I have no way of testing this out on Windows (I noticed Petr's comment that a previous attempt to fix this bug broke Windows) . Please add anyone who can do that among the reviewers.


I've removed @chandlerc and added @beanz and @smeenai as reviewers as additional CMake build maintainers.

I have a Windows machine for testing these kind of changes, but I'm not sure if I'll be able to test this soon due to other high priority tasks that are on my plate right now.

I generally like the solution, but one concern I have is that as implemented, `find_compiler_rt_library` will be invoked for every library which is likely going to increase both CMake runtime and amount of output. I'd prefer if we would invoke `find_compiler_rt_library` only once per target. Two ways I could think to achieve that is to either (1) move these invocations to another location such as `compiler-rt/cmake/config-ix.cmake` and store the result in a global variable which would then be used in `add_compiler_rt_runtime`, or (2) cache the result value in `find_compiler_rt_library` and avoid re-invoking Clang if the value is already set. I'm leaning towards (1) which seems cleaner.


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

https://reviews.llvm.org/D74133





More information about the llvm-commits mailing list