[Tarantool-patches] [PATCH] cmake: add LTO support for building luajit
Olga Arkhangelskaia
arkholga at tarantool.org
Tue Jun 16 14:36:15 MSK 2020
Hi!
Alexander! I am very thankful for the detailed review of the issue.
I have spent a lot of time reading about LTO and how it is supported in
others projects. Ad did not
think about cmake itself.
I have applied your patch and edit luajit.cmake according to newly
exposed variables. It has changed completely.
Diff:
- # Add LTO option to luajit
- if (CMAKE_INTERPROCEDURAL_OPTIMIZATION)
- message("Setting LTO flags for building luajit")
- # Clang opt to support LTO
- if (CMAKE_COMPILER_IS_CLANG)
- if (${CMAKE_CXX_COMPILER_VERSION} VERSION_LESS 3.9)
- set (luajit_cflags ${luajit_cflags} -flto=full)
- else()
- # ThinLTO that is both scalable and incremental
- # due to parallel IPO, available since 3.9 and above.
- # See
http://blog.llvm.org/2016/06/thinlto-scalable-and-incremental-lto.html
- set (luajit_cflags ${luajit_cflags} -flto=thin)
- endif()
- # XCode linker dynamically load libLTO.dylib to perform
- # link-time optimization.
- #
http://lists.llvm.org/pipermail/llvm-dev/2009-November/027103.html
- if (NOT TARGET_OS_DARWIN)
- # llvm-ar is just a wrapper over ar to pass --plugin=
option,
- # so that ar can understand LTO objects.
- # See
https://lists.llvm.org/pipermail/llvm-dev/2018-March/122018.html
- set (CMAKE_AR llvm-ar)
- endif()
- else()
- # GNU opts to support lto
- # Due to some problems (bugs, slow work, etc) we support LTO
- # only for 5.0+. The same is for binutils prior to 2.27
- # See comments in scripts/Makefile.lto in scope of
- # the patch: https://patchwork.kernel.org/patch/10078207/
- if (NOT ${CMAKE_CXX_COMPILER_VERSION} VERSION_LESS 5.0
- AND NOT ${linker_version} VERSION_LESS 2.27)
- set (luajit_cflags ${luajit_cflags} -flto
-fuse-linker-plugin -fno-fat-lto-objects)
- endif()
- # gcc-ar is just a wrapper over ar to pass --plugin option
- # so ar can understand LTO objects. For further info see
- # -flto and -ffat-lto-objects options descriptions:
- # https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html
- set (CMAKE_AR gcc-ar)
- endif()
+ # Enablibg LTO for luajit if DENABLE_LTO set.
+ if (${ENABLE_LTO})
+`......message(STATUS "Enablig LTO for luajit")
+ set (luajit_ld ${CMAKE_C_LINK_OPTIONS_IPO})
+`......set (luajit_cflags ${luajit_cflags} ${CFLAGS_LTO})
+`......set (CMAKE_AR ${AR_LTO})
endif()
- # Pass the same toolchain that is used for building of
I have test it with MacOS and debian. And will do full-ci for it.
16.06.2020 4:02, Alexander Turenko пишет:
> I vote to use the same build tools and flags that CMake uses to build
> files under its control, because otherwise we would write the same logic
> again. It may be not as accurate as one that CMake provides (see example
> below). It may become outdated in a future. And, last but not least,
> duplicated code is painful to maintain.
>
> However the way I proposed requires to use undocumented CMake variables
> (see [0]). But I think that's evil of the lesser kind.
>
> I think it would be good to expose related build tool names and flags
> from cmake/lto.cmake and use them in cmake/luajit.cmake. I implemented
> the former part (I would even left STATUS messages as is, they provide
> useful information):
>
> | diff --git a/cmake/lto.cmake b/cmake/lto.cmake
> | index 95ade75f4..79b908e26 100644
> | --- a/cmake/lto.cmake
> | +++ b/cmake/lto.cmake
> | @@ -90,8 +90,40 @@ if (NOT TARGET_OS_DARWIN)
> | endif()
> | endif()
> |
> | -# gh-3742: investigate LTO warnings.
> | -set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wno-lto-type-mismatch")
> | +# {{{ Expose build tools and flags
> | +#
> | +# It is convenient for building non-cmake targets with the same
> | +# flags as we use for sources under CMake control.
> | +#
> | +# It leans on uncodumented variables that are set in the following
> | +# CMake modules: Compiler/GNU.cmake and Compiler/Clang.cmake.
> | +
> | +# CFLAGS_LTO (list)
> | +set(CFLAGS_LTO ${CMAKE_C_COMPILE_OPTIONS_IPO})
> | +message(STATUS "CFLAGS_LTO: ${CFLAGS_LTO}")
> |
> | +# LDFLAGS_LTO (list)
> | +set(LDFLAGS_LTO ${CMAKE_C_LINK_OPTIONS_IPO})
> | +# FIXME: gh-3742: investigate LTO warnings.
> | +list(APPEND LDFLAGS_LTO -Wno-lto-type-mismatch)
> | +message(STATUS "LDFLAGS_LTO: ${LDFLAGS_LTO}")
> | +
> | +# AR_LTO (string)
> | +#
> | +# Note: Platform/Linux-Intel.cmake and Platform/Windows-MSVC.cmake
> | +# set CMAKE_C_CREATE_STATIC_LIBRARY_IPO, but not
> | +# CMAKE_C_ARCHIVE_CREATE_IPO. So this snippet is only for GCC and
> | +# clang.
> | +set(_ar_command ${CMAKE_C_ARCHIVE_CREATE_IPO})
> | +separate_arguments(_ar_command)
> | +list(GET _ar_command 0 AR_LTO)
> | +unset(_ar_command)
> | +message(STATUS "AR_LTO: ${AR_LTO}")
> | +
> | +# }}}
> | +
> | +# Set build tools and flags for files that are built using CMake.
> | +set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wno-lto-type-mismatch")
> | set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
> | +
> | message(STATUS "Enabling LTO: TRUE")
>
> Olga, Igor, what do you think about this way?
>
> Or, maybe, we should just add LJCORE_O=ljamalg.o to the Make command
> (just like `make amalg` does)? I was curious and tried it:
>
> | diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
> | index dd9d748ca..d3df66652 100644
> | --- a/cmake/luajit.cmake
> | +++ b/cmake/luajit.cmake
> | @@ -284,6 +284,7 @@ macro(luajit_build)
> | CCOPT="${luajit_ccopt}"
> | CCDEBUG="${luajit_ccdebug}"
> | XCFLAGS="${luajit_xcflags}"
> | + LJCORE_O=ljamalg.o
> | Q=''
> | # We need to set MACOSX_DEPLOYMENT_TARGET to at least 10.6,
> | # because 10.4 SDK (which is set by default in LuaJIT's
>
> And got undefined reference errors from src/lua/utils.c code that use
> LuaJIT internals.
>
> Despite way we'll choose, I would try some benchmarks with jit enabled
> and disabled (`jit.off()`) to see whether compiling of LuaJIT with LTO
> gives some gains. Technically it is not part of the task (my bad), so
> skip it if you're not curious. There is the good candidate to look into:
> [3].
>
> Placed the actual version from the branch [1] to comment it.
>
>> commit 62e5d0af7f7a5e44f27b741d43c6c035f20e93b4
>> Author: Olga Arkhangelskaia <arkholga at tarantool.org>
>> Date: Tue Mar 3 12:05:49 2020 +0300
>>
>> cmake: add LTO support for building luajit
>>
>> Tarantool has LTO support, however while building luajit this opt. was
>> omitted. Patch adds necessary flag to turn it on.
>>
>> Minimum compiler/linker versions: clang 3.4, gcc 5.0+, ld 2.27+ due
>> errors and slow work.
>>
>> Closes #3743
>>
>> diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
>> index 10df633d5..dd9d748ca 100644
>> --- a/cmake/luajit.cmake
>> +++ b/cmake/luajit.cmake
>> @@ -224,6 +224,45 @@ macro(luajit_build)
>> set(luajit_xcflags ${luajit_xcflags} -D${def})
>> endforeach()
>>
>> + # Add LTO option to luajit
>> + if (CMAKE_INTERPROCEDURAL_OPTIMIZATION)
> I would lean on ENABLE_LTO variable and the fact that all necessary
> checks already performed by cmake/lto.cmake. I guess it would be more
> intuitive.
>
>> + message("Setting LTO flags for building luajit")
> I would use STATUS mode, the message is informational and should not
> attract user's attention. STATUS mode is used for such messages in
> cmake/lto.cmake. See [2] for information about message types.
>
>> + # Clang opt to support LTO
>> + if (CMAKE_COMPILER_IS_CLANG)
>> + if (${CMAKE_CXX_COMPILER_VERSION} VERSION_LESS 3.9)
>> + set (luajit_cflags ${luajit_cflags} -flto=full)
>> + else()
>> + # ThinLTO that is both scalable and incremental
>> + # due to parallel IPO, available since 3.9 and above.
>> + # See http://blog.llvm.org/2016/06/thinlto-scalable-and-incremental-lto.html
>> + set (luajit_cflags ${luajit_cflags} -flto=thin)
>> + endif()
>> + # XCode linker dynamically load libLTO.dylib to perform
>> + # link-time optimization.
>> + # http://lists.llvm.org/pipermail/llvm-dev/2009-November/027103.html
>> + if (NOT TARGET_OS_DARWIN)
>> + # llvm-ar is just a wrapper over ar to pass --plugin= option,
>> + # so that ar can understand LTO objects.
>> + # See https://lists.llvm.org/pipermail/llvm-dev/2018-March/122018.html
>> + set (CMAKE_AR llvm-ar)
>> + endif()
> I guess it reflects the logic of the following snippet from
> Compiler/Clang.cmake CMake module:
>
> | string(COMPARE EQUAL "${CMAKE_${lang}_COMPILER_ID}" "AppleClang" __is_apple_clang)
> | <...>
> | if(ANDROID OR __is_apple_clang)
> | set(__ar "${CMAKE_AR}")
> | set(__ranlib "${CMAKE_RANLIB}")
> | else()
> | set(__ar "${CMAKE_${lang}_COMPILER_AR}")
> | set(__ranlib "${CMAKE_${lang}_COMPILER_RANLIB}")
> | endif()
> |
> | set(CMAKE_${lang}_ARCHIVE_CREATE_IPO
> | "\"${__ar}\" cr <TARGET> <LINK_FLAGS> <OBJECTS>"
> | )
> |
> | set(CMAKE_${lang}_ARCHIVE_APPEND_IPO
> | "\"${__ar}\" r <TARGET> <LINK_FLAGS> <OBJECTS>"
> | )
> |
> | set(CMAKE_${lang}_ARCHIVE_FINISH_IPO
> | "\"${__ranlib}\" <TARGET>"
> | )
>
> So TARGET_OS_DARWIN check is not accurate: we should check whether Clang
> is built by Apple or, say, installed from brew; not just OS.
>
>> + else()
>> + # GNU opts to support lto
>> + # Due to some problems (bugs, slow work, etc) we support LTO
>> + # only for 5.0+. The same is for binutils prior to 2.27
>> + # See comments in scripts/Makefile.lto in scope of
>> + # the patch: https://patchwork.kernel.org/patch/10078207/
>> + if (NOT ${CMAKE_CXX_COMPILER_VERSION} VERSION_LESS 5.0
>> + AND NOT ${linker_version} VERSION_LESS 2.27)
>> + set (luajit_cflags ${luajit_cflags} -flto -fuse-linker-plugin -fno-fat-lto-objects)
>> + endif()
>> + # gcc-ar is just a wrapper over ar to pass --plugin option
>> + # so ar can understand LTO objects. For further info see
>> + # -flto and -ffat-lto-objects options descriptions:
>> + # https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html
>> + set (CMAKE_AR gcc-ar)
>> + endif()
>> + endif()
>> # Pass the same toolchain that is used for building of
>> # tarantool itself, because tools from different toolchains
>> # can be incompatible. A compiler and a linker are already set
> [0]: https://gitlab.kitware.com/cmake/cmake/-/issues/16808
> [1]: OKriw/gh-3743-LuaJIT-does-not-use-LTO-with-DENABLE_LTO=ON-full-ci
> [2]: https://cmake.org/cmake/help/latest/command/message.html
> [3]: https://gitspartv.github.io/LuaJIT-Benchmarks/
>
> WBR, Alexander Turenko.
More information about the Tarantool-patches
mailing list