Fix build LLVM with -D LLVM_USE_INTEL_JITEVENTS:BOOL=ON on Windows

[andrew.w.kaylor]

Summary

This patch updates the CMakeLists.txt file for the LLVM Intel JIT event listener to fix a problem I encountered when building on Windows. It also incorporates changes from D16883 which I was preparing for commit when I encountered the Windows problem (see https://llvm.org/bugs/show_bug.cgi?id=26433).

Rafael, I am asking you to review this because you authored the previous change which introduced the pthread and libdl dependencies (which are needed on Linux but cause failures on Windows). I've tested this on Windows and on Linux (with and without the -DBUILD_SHARED_LIBS=ON option on Linux), but I'm always finding new subtleties in the build system that I've overlooked so I figure it's worth having you take a look to make sure I didn't re-break something you fixed with the previous change.

I'm hoping these changes can be made in the 3.8 branch as well.

[• rafael]

This is fine by me, but you should probably get a review by someone familiar with the current CMake setup. Chris?

lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt

18 ↗

(On Diff #47065)

Why do you need this?

[andrew.w.kaylor]

lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt

18 ↗

(On Diff #47065)

That's a good question. There is no reason that IntelJITEvents should need this, but there is an include of attributes.inc a few levels deep in a file the gets included from IntelJITEventListener.cpp. Something has changed in trunk in the past few days to make the build failure go away, but I haven't been able to track down what it was. The build failure still exists in the 3.8 branch. It looks like this:

Building CXX object lib/ExecutionEngine/IntelJITEvents/CMakeFiles/LLVMIntelJITEvents.dir/IntelJITEventListener.cpp.o
In file included from /users/akaylor/llvm38/llvm/include/llvm/IR/CallSite.h:31: ,
                 from /users/akaylor/llvm38/llvm/include/llvm/Analysis/AliasAnalysis.h:42,
                 from /users/akaylor/llvm38/llvm/include/llvm/CodeGen/MachineInstr.h:26,
                 from /users/akaylor/llvm38/llvm/include/llvm/CodeGen/MachineBasicBlock.h:18,
                 from /users/akaylor/llvm38/llvm/include/llvm/CodeGen/MachineFunction.h:22,
                 from /users/akaylor/llvm38/llvm/lib/ExecutionEngine/IntelJITEvents/IntelJITEventListener.cpp:18:
/users/akaylor/llvm38/llvm/include/llvm/IR/Attributes.h:69:38: fatal error: llvm/IR/Attributes.inc: No such file or directory
     #include "llvm/IR/Attributes.inc"
                                      ^
compilation terminated.

It may be that having the dependency here isn't the best way to fix this.

[beanz]

lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt

3 ↗

(On Diff #47065)

You don't need to set the variable to empty before using it. Evaluating a variable that has never been set is equivalent to evaluating one that was set to nothing.

18 ↗

(On Diff #47065)

The correct way to handle this is probably to depend on CodeGen rather than intrinsics_gen. That dependency can be expressed either in CMake or in the LLVMBuild file. Doing that sets up the transitive dependency that a tool linking IntelJITEvents also needs CodeGen, Analysis and Core.

[andrew.w.kaylor]

Updated files based on review feedback

lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt

17 ↗

(On Diff #47712)

I'm not 100% sure that the change I made matches what you are suggesting, but it appears to fix the problem in the 3.8 branch.

[beanz]

One minor comment, but LGTM.

lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt

17 ↗

(On Diff #47712)

Just a nitpick. When linking LLVM components instead of calling add_dependencies you can set LLVM_LINK_COMPONENTS before calling add_llvm_library.

Adding something like this to the top of the file is the more normal way to handle this:

set(LLVM_LINK_COMPONENTS Support)

[andrew.w.kaylor]

lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt

17 ↗

(On Diff #47712)

That doesn't appear to fix the build problem.

I tried

set(LLLVM_LINK_COMPONENTS CodeGen)

and

set(LLVM_LINK_COMPONENTS Support CodeGen)

Could this be an issue because I am declaring a library and not a tool?

All the similar files I've looked at seem to be using some form of adding a dependency directly on intrinsics_gen. Many of them seem to have come from r239796 (http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150615/281739.html).

[beanz]

Sorry about that. Yea, go ahead and commit as is.

Thanks,
-Chris

llvm/trunk/lib/ExecutionEngine/IntelJITEvents/CMakeLists.txt

 include_directories( ${CMAKE_CURRENT_SOURCE_DIR}/.. )
 
+if( HAVE_LIBDL )
+    set(LLVM_INTEL_JIT_LIBS ${CMAKE_DL_LIBS})
+endif()
+if( HAVE_LIBPTHREAD )
+    set(LLVM_INTEL_JIT_LIBS pthread ${LLVM_INTEL_JIT_LIBS})
+endif()
+
 add_llvm_library(LLVMIntelJITEvents
   IntelJITEventListener.cpp
   jitprofiling.c
 
-  LINK_LIBS pthread ${CMAKE_DL_LIBS}
+  LINK_LIBS ${LLVM_INTEL_JIT_LIBS}
 )
+
+add_dependencies(LLVMIntelJITEvents LLVMCodeGen)

llvm/trunk/lib/ExecutionEngine/IntelJITEvents/LLVMBuild.txt

 ;===------------------------------------------------------------------------===;
 
 [common]
 
 [component_0]
 type = OptionalLibrary
 name = IntelJITEvents
 parent = ExecutionEngine
-required_libraries = Core DebugInfoDWARF Support Object ExecutionEngine
+required_libraries = CodeGen Core DebugInfoDWARF Support Object ExecutionEngine