Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Alex Khatskevich <avkhatskevich@tarantool.org>
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH] Add LTO support
Date: Tue, 15 May 2018 14:11:28 +0300	[thread overview]
Message-ID: <20180515111128.sxythzpmmw4sg6td@tkn_work_nb> (raw)
In-Reply-To: <20180515102328.11908-1-avkhatskevich@tarantool.org>

Hi, Alex!

My comments are below.

WBR, Alexander Turenko.

On Tue, May 15, 2018 at 01:23:28PM +0300, AKhatskevich wrote:
 
> Changes:
>   - update submodules (fix lto-related warnings)
>   - add `TARANTOOL_LTO` cmake option (by default it is `True` only for
>     non-Debug builds)
> 

Don't remember any discussions re enabling this by default. Is it Kirill
or Kostya decision?

> LTO speeds up cpu-intensive workloads by up to 20%.
> Requirements for LTO enabling:
>   - cmake >= 3.9
>   - linker
>     - ld >= 2.31 (or latest 2.30)
>     - gold >= 1.15 (binutils 2.30)
> This small patch required fixing bug in GNU ld linker.

If you menting the bug here, so add the link.

> +# Tarantool uses both dynamic-list and lto link options, which works only
> +# since binutils:
> +#  - 2.30 for linking with gold (gold version is 1.15)
> +#  - last 2.30 or 2.31 in case of ld (dbf)
> +#    (ld was fixed expecially for Tarantool,
> +#    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84901)
> +

dbf -> bfd

'Expecially' is 'especially'? I feel that as the boast in not
appropriate place.

> +    if (NOT linker_version VERSION_LESS "2.31")
> +	set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)

Strange indent. It seems that you mix tabs and spaces within that file.

> +    elseif(NOT linker_version VERSION_LESS "2.30")
> +      # Use gold if LTO+dynamic-list is available in gold & not in ld
> +      find_program(gold_available "ld.gold")
> +      if (gold_available)
> +	message(WARNING "Use gold linker (to enable LTO)")
> +	SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fuse-ld=gold")
> +	set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)

Indent.

      reply	other threads:[~2018-05-15 11:11 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-15 10:23 [tarantool-patches] " AKhatskevich
2018-05-15 11:11 ` Alexander Turenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180515111128.sxythzpmmw4sg6td@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=avkhatskevich@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH] Add LTO support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox