[llvm-commits] [Patch][CMake] Implement --enable-libffi option in CMake build.

[Óscar Fuentes]

arrowdodger <6yearold-Re5JQEeQqe8AvxtiuMwx3w at public.gmane.org> writes:

> Attached patch introduces option LLVM_ENABLE_FFI. If enabled, CMake will
> search for libffi and it's headers and link it with
> lib/ExecutionEngine/Interpreter.
> I've built LLVM/Clang with this patch on FreeBSD and Ubuntu Linux.
>
> Index: cmake/config-ix.cmake
> ===================================================================
> --- cmake/config-ix.cmake	(revision 123973)
> +++ cmake/config-ix.cmake	(working copy)
> @@ -190,6 +190,51 @@
>  llvm_find_program(dot)
>  llvm_find_program(dotty)
>  
> +if( LLVM_ENABLE_FFI )
> +  if( FFI_INCLUDE_DIR )
> +    find_path(FFI_INCLUDE_PATH ffi.h PATHS ${FFI_INCLUDE_DIR})
> +  else()
> +    find_path(FFI_INCLUDE_PATH ffi.h)
> +  endif( FFI_INCLUDE_DIR )
> +  if( FFI_INCLUDE_PATH )
> +    set(FFI_HEADER ffi.h CACHE INTERNAL "")
> +    set(HAVE_FFI_H 1 CACHE INTERNAL "")
> +  else()
> +    if( FFI_INCLUDE_DIR )
> +      find_path(FFI_INCLUDE_PATH ffi/ffi.h PATHS ${FFI_INCLUDE_DIR})
> +    else()
> +      find_path(FFI_INCLUDE_PATH ffi/ffi.h)
> +    endif()
> +    if( FFI_INCLUDE_PATH )
> +      set(FFI_HEADER ffi/ffi.h CACHE INTERNAL "")
> +      set(HAVE_FFI_FFI_H 1 CACHE INTERNAL "")
> +    endif()
> +  endif()
> +  
> +  if( NOT FFI_HEADER )
> +    message(FATAL_ERROR "libffi includes are not found. You may run cmake again with -D FFI_INCLUDE_DIR:PATH=/path/to/ffi/include")
> +  endif()
> +
> +  if( FFI_LIBRARY_PATH )
         ^^^^^^^^^^^^^^^^
I guess you meant FFI_LIBRARY_DIR

> +    find_library(FFI_LIBRARY_PATH ffi PATHS ${FFI_LIBRARY_DIR})
> +  else()
> +    find_library(FFI_LIBRARY_PATH ffi)
> +  endif()
> +  if( NOT FFI_LIBRARY_PATH )
> +    message(FATAL_ERROR "libffi is not found. You may run cmake again with -D FFI_LIBRARY_DIR:PATH=/path/to/ffi/lib")
> +  endif()
[snip]
>  
>  /* Set to 1 if the finite function is found in <ieeefp.h> */
>  #cmakedefine HAVE_FINITE_IN_IEEEFP_H ${HAVE_FINITE_IN_IEEEFP_H}
> Index: lib/ExecutionEngine/Interpreter/CMakeLists.txt
> ===================================================================
> --- lib/ExecutionEngine/Interpreter/CMakeLists.txt	(revision 123973)
> +++ lib/ExecutionEngine/Interpreter/CMakeLists.txt	(working copy)
> @@ -1,3 +1,7 @@
> +if( FFI_INCLUDE_PATH )
> +  include_directories( ${FFI_INCLUDE_PATH} )
> +endif()
> +
>  add_llvm_library(LLVMInterpreter
>    Execution.cpp
>    ExternalFunctions.cpp
> @@ -3,2 +7,6 @@
>    Interpreter.cpp
>    )
> +
> +if( FFI_LIBRARY_PATH )
> +  target_link_libraries( LLVMInterpreter ${FFI_LIBRARY_PATH} )
> +endif()

This is not good style. Using full paths for adding libraries to the
link command should be avoided, otherwise the resulting command would be
very hard to read. Please use the something like the code that I sent
you on our private discussion.

> \ No newline at end of file
> Index: CMakeLists.txt
> ===================================================================
> --- CMakeLists.txt	(revision 123973)
> +++ CMakeLists.txt	(working copy)
> @@ -114,6 +114,8 @@
>    set(ENABLE_TIMESTAMPS 1)
>  endif()
>  
> +option(LLVM_ENABLE_FFI "Use libffi to call external functions from the interpreter" OFF)
> +

You need to declare FFI_INCLUDE_DIR and FFI_LIBRARY_DIR as user-settable
variables:

set(FFI_LIBRARY_DIR "" CACHE PATH "<description>")

You need to document LLVM_ENABLE_FFI on docs/CMake.html too. Mention
FFI_LIBRARY_DIR and FFI_INCLUDE_DIR there.