[PATCH] D45436: [llvm-exegesis] Add a flag to disable libpfm even if present.
Clement Courbet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 9 06:49:19 PDT 2018
courbet added inline comments.
================
Comment at: cmake/config-ix.cmake:93
# Check for libpfm.
-check_library_exists(pfm pfm_initialize "" HAVE_LIBPFM_INITIALIZE)
-if(HAVE_LIBPFM_INITIALIZE)
- check_include_file(perfmon/perf_event.h HAVE_PERFMON_PERF_EVENT_H)
- check_include_file(perfmon/pfmlib.h HAVE_PERFMON_PFMLIB_H)
- check_include_file(perfmon/pfmlib_perf_event.h HAVE_PERFMON_PFMLIB_PERF_EVENT_H)
- if(HAVE_PERFMON_PERF_EVENT_H AND HAVE_PERFMON_PFMLIB_H AND HAVE_PERFMON_PFMLIB_PERF_EVENT_H)
- set(HAVE_LIBPFM 1)
+if (LLVM_ENABLE_LIBPFM)
+ check_library_exists(pfm pfm_initialize "" HAVE_LIBPFM_INITIALIZE)
----------------
lebedev.ri wrote:
> courbet wrote:
> > lebedev.ri wrote:
> > > One more piece is missing, `LLVM_ENABLE_LIBPFM` needs to be checked where linking to pfm happens.
> > Thanks Roman,
> >
> > This is already done by checking on HAVE_LIBPFM, see tools/llvm-exegesis/CMakeLists.txt:
> >
> > ```
> > if(HAVE_LIBPFM)
> > target_link_libraries(llvm-exegesis PRIVATE pfm)
> > endif()
> > ```
> Exactly, and that is insufficient.
> It should be
> ```
> if(LLVM_ENABLE_LIBPFM AND HAVE_LIBPFM)
> target_link_libraries(llvm-exegesis PRIVATE pfm)
> endif()
> ```
> I'm not sure if LLVM can be built as part of some parent CMake project, so *this* may be not-an-issue,
> but will //just// give a bad example for future changes. The problem is, what will happen if one
> defines `HAVE_LIBPFM` (e.g. by using it in some parent cmake project), but disables `LLVM_ENABLE_LIBPFM`?
>
> Similar problem: https://github.com/google/googletest/pull/975
I see, that makes sense, thanks for the explanation.
Repository:
rL LLVM
https://reviews.llvm.org/D45436
More information about the llvm-commits
mailing list