[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