[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