Re: [Tarantool-patches] [PATCH] cmake: add LTO support for building luajit

[Olga Arkhangelskaia <arkholga@tarantool.org>]

Hello Igor!

Thank you for your patience and help.

I fixed previous comments:

   1 diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
   2 index d02c35432..473bfc5e9 100644
   3 --- a/cmake/luajit.cmake
   4 +++ b/cmake/luajit.cmake
   5 @@ -238,15 +238,15 @@ macro(luajit_build)
   6                  # See 
http://blog.llvm.org/2016/06/thinlto-scalable-and-incremental-lto.html
   7                  set (luajit_cflags ${luajit_cflags} -flto=thin)
   8 endif()
   9 -                # Since Darwin uses DYLD_LIBRARY_PATH instead of 
llvm-ar
  10 -                # ar should not be change.
  11 -                # See 
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160425/156799.html
  12 -                if (NOT TARGET_OS_DARWIN)
  13 -                    # llvm-ar is just a wrapper over ar to pass 
--plugin= option,
  14 -                    # so that ar can understand LTO objects.
  15 -                    # See 
https://lists.llvm.org/pipermail/llvm-dev/2018-March/122018.html
  16 -                    set (CMAKE_AR llvm-ar)
  17 - endif()
  18 +                # XCode linker dynamically load libLTO.dylib to 
perform
  19 +                # link-time optimization.
  20 +                # 
http://lists.llvm.org/pipermail/llvm-dev/2009-November/027103.html
  21 +            if (NOT TARGET_OS_DARWIN)
  22 +                # llvm-ar is just a wrapper over ar to pass 
--plugin= option,
  23 +                # so that ar can understand LTO objects.
  24 +                # See 
https://lists.llvm.org/pipermail/llvm-dev/2018-March/122018.html
  25 +                set (CMAKE_AR llvm-ar)
  26 + endif()
  27 else()
  28          # GNU opts to support lto
  29              # Due to some problems (bugs, slow work, etc) we 
support LTO

28.04.2020 2:04, Igor Munkin пишет:
> Olya,
>
> Thanks for the changes!
>
> On 17.04.20, Olga Arkhangelskaia wrote:
>> Hi Igor!
>>
>> It seems that I eventually had fixed white-space mess.
> Yes, there are no tabs in the current patch! However there is a
> misindent in the DARWIN-related part (see comments below).
>
>> While testing everything once again I came across reason why I separated
>> case with Darwin and
>>
>> llvm-ar. Fixed and added a comment.
>>
>> Diffs are below:
>>
>> P.S.
>>
>> Please, let me know whether new diff is better.
> Sorry, but it's not. We can discuss it in tg to not spoil the ml.
>
> <snipped>
>
> Here is the update patch from the upstream:
>
> ================================================================================
>
> From: Olga Arkhangelskaia <arkholga@tarantool.org>
> Date: Tue, 3 Mar 2020 12:05:49 +0300
> Subject: [PATCH] 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
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   cmake/luajit.cmake | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
>
> diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
> index 072db8269..d02c35432 100644
> --- a/cmake/luajit.cmake
> +++ b/cmake/luajit.cmake
> @@ -225,6 +225,45 @@ macro(luajit_build)
>           set(luajit_xcflags ${luajit_xcflags} -D${def})
>       endforeach()
>   
> +    # 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()
> +                # Since Darwin uses DYLD_LIBRARY_PATH instead of llvm-ar
> +                # ar should not be change.
> +                # See http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160425/156799.html
>
> I propose to mention the original patch[1] instead of the reply. By the
> way, this fix differs from the one you mentioned: DYLD_LIBRARY_PATH is
> set in the patch, but you just omit changing CMAKE_AR. Whether this fix
> is related to our problem? Additionaly this if scope below is
> misindented, please adjust it.
>
> +                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()
> +    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