[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