[PATCH] D74133: Fix for PR38025
Riyaz V Puthiyapurayil via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 7 11:58:21 PST 2020
RVP added a comment.
> 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.
Agreed. I have noticed that in the Cmake output though the build time impact isn't much. But better to fix it while we are at it.
> 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.
(1) looks like a better solution. The current change is something I have had in our build for over a year and I uploaded it because someone else recently pinged. If I have to refactor this change, I have to allocate some time to test it properly as well (same boat as you with other higher priority stuff). Let me see...
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74133/new/
https://reviews.llvm.org/D74133
More information about the llvm-commits
mailing list