From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E868A469710 for ; Wed, 6 May 2020 13:47:22 +0300 (MSK) References: <20200312100549.31608-1-arkholga@tarantool.org> <20200414091803.GJ5713@tarantool.org> <184a4359-9531-a35e-b77a-554394346cdd@tarantool.org> <20200414170024.GL5713@tarantool.org> <20200417114720.GK8314@tarantool.org> <30019d9a-bf60-eff2-9628-5346a76f9953@tarantool.org> <20200427230411.GP11314@tarantool.org> From: Olga Arkhangelskaia Message-ID: Date: Wed, 6 May 2020 13:47:16 +0300 MIME-Version: 1.0 In-Reply-To: <20200427230411.GP11314@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH] cmake: add LTO support for building luajit List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Munkin Cc: tarantool-patches@dev.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. > > > > Here is the update patch from the upstream: > > ================================================================================ > > From: Olga Arkhangelskaia > 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 > --- > 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