[Tarantool-patches] [PATCH] cmake: add LTO support for building luajit
Olga Arkhangelskaia
arkholga at tarantool.org
Tue Jun 16 15:01:07 MSK 2020
Update branch and checked output on release_lto and release_lto_clang8.
They are ok.
16.06.2020 14:36, Olga Arkhangelskaia пишет:
> 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