[tarantool-patches] [PATCH 0/3] LTO && travis

[tarantool-patches] [PATCH 0/3] LTO && travis

[AKhatskevich]

Issue: https://github.com/tarantool/tarantool/issues/3117
Branch: https://github.com/tarantool/tarantool/tree/kh/gh-3117-lto-2

This patchset introduces LTO build feature for tarantool.

AKhatskevich (3):
  Fix: prevent guard-breaker optimization
  Add LTO support
  Add LTO testing && refactor travis.yml

 .travis.mk          |  2 +-
 .travis.yml         | 50 +++++++++---------------------------------------
 CMakeLists.txt      |  1 +
 cmake/lto.cmake     | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/lib/msgpuck     |  2 +-
 src/lib/small       |  2 +-
 test/unit/guard.cc  |  2 +-
 third_party/libyaml |  2 +-
 8 files changed, 70 insertions(+), 46 deletions(-)
 create mode 100644 cmake/lto.cmake

-- 
2.14.1

[tarantool-patches] [PATCH 1/3] Fix: prevent guard-breaker optimization

[AKhatskevich]

In case of very aggressive optimizations the compiler can
optimize guard-breaker function away and the `unit/guard`
test would fail.
---
 test/unit/guard.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/unit/guard.cc b/test/unit/guard.cc
index 231b44c7d..2082dfd48 100644
--- a/test/unit/guard.cc
+++ b/test/unit/guard.cc
@@ -13,7 +13,7 @@ static int __attribute__((noinline))
 stack_break_f(char *ptr)
 {
 	char block[2048];
-	char sum = 0;
+	volatile char sum = 0;
 	memset(block, 0xff, 2048);
 	sum += block[block[4]];
 	ptrdiff_t stack_diff = ptr > block ? ptr - block : block - ptr;
-- 
2.14.1

[tarantool-patches] [PATCH 2/3] Add LTO support

[AKhatskevich]

This commit introduces LTO support.

Changes:
  - update submodules (fix lto-related warnings)
  - add `TARANTOOL_LTO` cmake option (by default it is `False`)

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)
    - mac xcode >= 8 (earlier versions are not tested)
This small patch required fixing bug in GNU ld linker.
(link gcc.gnu.org/bugzilla/show_bug.cgi?id=84901)
`ld` was not exporting symbols from dynamic list when LTO enabled.

Closes #3117
---
 CMakeLists.txt      |  1 +
 cmake/lto.cmake     | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/lib/msgpuck     |  2 +-
 src/lib/small       |  2 +-
 third_party/libyaml |  2 +-
 5 files changed, 59 insertions(+), 3 deletions(-)
 create mode 100644 cmake/lto.cmake

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d0cae0f01..432d2a6fb 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -63,6 +63,7 @@ include(cmake/pod2man.cmake)
 include(cmake/arch.cmake)
 include(cmake/os.cmake)
 include(cmake/compiler.cmake)
+include(cmake/lto.cmake NO_POLICY_SCOPE)
 include(cmake/simd.cmake)
 include(cmake/atomic.cmake)
 include(cmake/profile.cmake)
diff --git a/cmake/lto.cmake b/cmake/lto.cmake
new file mode 100644
index 000000000..96129ebfd
--- /dev/null
+++ b/cmake/lto.cmake
@@ -0,0 +1,55 @@
+##
+## Manage LTO (Link-Time-Optimization) and IPO (Inter-Procedural-Optimization)
+##
+
+# 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 (bfd)
+
+if (NOT DEFINED TARANTOOL_LTO)
+    set(TARANTOOL_LTO FALSE)
+endif()
+
+if(NOT CMAKE_VERSION VERSION_LESS 3.9)
+    cmake_policy(SET CMP0069 NEW)
+    include(CheckIPOSupported)
+    check_ipo_supported(RESULT CMAKE_IPO_AVAILABLE)
+else()
+    set(CMAKE_IPO_AVAILABLE FALSE)
+endif()
+
+# Ensure that default value is false
+set(CMAKE_INTERPROCEDURAL_OPTIMIZATION FALSE)
+if (TARANTOOL_LTO AND CMAKE_IPO_AVAILABLE AND NOT TARGET_OS_DARWIN)
+    execute_process(COMMAND ld -v OUTPUT_VARIABLE linker_v)
+	message(STATUS "LTO linker_v ${linker_v}")
+
+    # e.g. GNU ld (GNU Binutils) 2.29
+    string(REGEX MATCH "^GNU ld [^)]+[)] ([0-9.]+).*$" linker_valid ${linker_v})
+
+    if (NOT linker_valid)
+        message(FATAL_ERROR "Unsuported linker (ld expected)")
+    endif()
+
+    set(linker_version ${CMAKE_MATCH_1})
+    message(STATUS "Found linker ld VERSION ${linker_version}")
+
+    if (NOT linker_version VERSION_LESS "2.31")
+        set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
+    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)
+        endif()
+    endif()
+endif()
+
+if (TARANTOOL_LTO AND CMAKE_IPO_AVAILABLE AND TARGET_OS_DARWIN)
+    set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
+endif()
+
+message(STATUS "LTO enabled ${CMAKE_INTERPROCEDURAL_OPTIMIZATION}")
diff --git a/src/lib/msgpuck b/src/lib/msgpuck
index 3b8f3e59b..1eb56a447 160000
--- a/src/lib/msgpuck
+++ b/src/lib/msgpuck
@@ -1 +1 @@
-Subproject commit 3b8f3e59b62d74f0198e01cbec0beb9c6a3082fb
+Subproject commit 1eb56a447ea35f9c403dbc8aa98a25336303c1cb
diff --git a/src/lib/small b/src/lib/small
index 22d1bad18..0627a7f98 160000
--- a/src/lib/small
+++ b/src/lib/small
@@ -1 +1 @@
-Subproject commit 22d1bad1873edcb9b1383a273e70c4cf881d5349
+Subproject commit 0627a7f986ff91701bd741908818749fef5e2266
diff --git a/third_party/libyaml b/third_party/libyaml
index 6bd4be1a7..e554afad0 160000
--- a/third_party/libyaml
+++ b/third_party/libyaml
@@ -1 +1 @@
-Subproject commit 6bd4be1a7751d6022a413864666880bef8a87c3c
+Subproject commit e554afad015b2c7df76f9d5213d883e931246fad
-- 
2.14.1

[tarantool-patches] [PATCH 3/3] Add LTO testing && refactor travis.yml

[AKhatskevich]

---
 .travis.mk  |  2 +-
 .travis.yml | 50 +++++++++-----------------------------------------
 2 files changed, 10 insertions(+), 42 deletions(-)

diff --git a/.travis.mk b/.travis.mk
index 66c921aa7..8077cddba 100644
--- a/.travis.mk
+++ b/.travis.mk
@@ -49,7 +49,7 @@ deps_osx:
 	brew install openssl readline curl icu4c --force
 
 test_osx: deps_osx
-	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo
+	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo ${CMAKE_EXTRA_PARAMS}
 	# Increase the maximum number of open file descriptors on macOS
 	sudo sysctl -w kern.maxfiles=20480 || :
 	sudo sysctl -w kern.maxfilesperproc=20480 || :
diff --git a/.travis.yml b/.travis.yml
index 997302e16..ee713be91 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -4,11 +4,7 @@ services:
 
 language: cpp
 
-os:
-  - linux
-  - osx
-
-osx_image: xcode9
+os: linux
 
 cache:
     directories:
@@ -37,44 +33,16 @@ env:
 matrix:
     allow_failures:
       - TARGET=coverage
-#      - env: OS=el DIST=6
-#      - env: OS=el DIST=7
-#      - env: OS=fedora DIST=26
-#      - env: OS=fedora DIST=27
-#      - env: OS=ubuntu DIST=artful
-#      - env: OS=ubuntu DIST=trusty
-#      - env: OS=ubuntu DIST=xenial
-#      - env: OS=ubuntu DIST=bionic
-#      - env: OS=debian DIST=wheezy
-#      - env: OS=debian DIST=jessie
-#      - env: OS=debian DIST=stretch
-    exclude:
-      - env: OS=el DIST=6
-        os: osx
-      - env: OS=el DIST=7
-        os: osx
-      - env: OS=fedora DIST=26
-        os: osx
-      - env: OS=fedora DIST=27
-        os: osx
-      - env: OS=ubuntu DIST=artful
-        os: osx
-      - env: OS=ubuntu DIST=trusty
-        os: osx
-      - env: OS=ubuntu DIST=xenial
-        os: osx
-      - env: OS=ubuntu DIST=bionic
-        os: osx
-      - env: OS=debian DIST=wheezy
-        os: osx
-      - env: OS=debian DIST=jessie
-        os: osx
-      - env: OS=debian DIST=stretch
-        os: osx
-      - env: TARGET=source
+    include:
+      - osx_image: xcode9
         os: osx
-      - env: TARGET=coverage
+        env:
+          - TARGET=test
+      - osx_image: xcode9
         os: osx
+        env:
+          - TARGET=test
+          - CMAKE_EXTRA_PARAMS="-DTARANTOOL_LTO=TRUE"
 
 script:
   - make -f .travis.mk ${TARGET}
-- 
2.14.1

[tarantool-patches] Re: [PATCH 1/3] Fix: prevent guard-breaker optimization

[Alexander Turenko]

On Wed, Aug 08, 2018 at 02:10:01PM +0300, AKhatskevich wrote:
> In case of very aggressive optimizations the compiler can
> optimize guard-breaker function away and the `unit/guard`
> test would fail.

I think it is good to mention the specific compiler options (and a
certain compiler you are using to reproduce it) to give a user an idea
when things are going wrong and so what is the fix does.

Is it due to discarding the noinline attribute in case of LTO on gcc? So
'volatile' prevents inlining the function? I just guessing here, but I
think the commit message should clarify such things if possible.

> ---
>  test/unit/guard.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/unit/guard.cc b/test/unit/guard.cc
> index 231b44c7d..2082dfd48 100644
> --- a/test/unit/guard.cc
> +++ b/test/unit/guard.cc
> @@ -13,7 +13,7 @@ static int __attribute__((noinline))
>  stack_break_f(char *ptr)
>  {
>  	char block[2048];
> -	char sum = 0;
> +	volatile char sum = 0;

I think a reason of using 'volatile' keyword should be always properly
commented in the code, because it always fix some unobvious compiler
behaviour.

>  	memset(block, 0xff, 2048);
>  	sum += block[block[4]];
>  	ptrdiff_t stack_diff = ptr > block ? ptr - block : block - ptr;
> -- 
> 2.14.1
> 
> 

[tarantool-patches] Re: [PATCH 2/3] Add LTO support

[Alexander Turenko]

On Wed, Aug 08, 2018 at 02:10:02PM +0300, AKhatskevich wrote:
> This commit introduces LTO support.
> 
> Changes:
>   - update submodules (fix lto-related warnings)

The msgpack submodule update contains a commit that are not related to
your change. We should not hide such changes in the commit history.
Maybe it is better to update submodules in a separate commit with proper
description of all changes it pulls into the main repository.

>   - add `TARANTOOL_LTO` cmake option (by default it is `False`)

It seems such options usually looks like ENABLE_XXX=[ON/OFF].

> 
> 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)
>     - mac xcode >= 8 (earlier versions are not tested)
> This small patch required fixing bug in GNU ld linker.
> (link gcc.gnu.org/bugzilla/show_bug.cgi?id=84901)
> `ld` was not exporting symbols from dynamic list when LTO enabled.
> 
> Closes #3117
> ---
>  CMakeLists.txt      |  1 +
>  cmake/lto.cmake     | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/lib/msgpuck     |  2 +-
>  src/lib/small       |  2 +-
>  third_party/libyaml |  2 +-
>  5 files changed, 59 insertions(+), 3 deletions(-)
>  create mode 100644 cmake/lto.cmake
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index d0cae0f01..432d2a6fb 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -63,6 +63,7 @@ include(cmake/pod2man.cmake)
>  include(cmake/arch.cmake)
>  include(cmake/os.cmake)
>  include(cmake/compiler.cmake)
> +include(cmake/lto.cmake NO_POLICY_SCOPE)

Is it for propagating cmake_policy(SET CMP0069 NEW)? Please, comment it.

>  include(cmake/simd.cmake)
>  include(cmake/atomic.cmake)
>  include(cmake/profile.cmake)
> diff --git a/cmake/lto.cmake b/cmake/lto.cmake
> new file mode 100644
> index 000000000..96129ebfd
> --- /dev/null
> +++ b/cmake/lto.cmake
> @@ -0,0 +1,55 @@
> +##
> +## Manage LTO (Link-Time-Optimization) and IPO (Inter-Procedural-Optimization)
> +##
> +
> +# 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 (bfd)
> +
> +if (NOT DEFINED TARANTOOL_LTO)
> +    set(TARANTOOL_LTO FALSE)
> +endif()
> +
> +if(NOT CMAKE_VERSION VERSION_LESS 3.9)
> +    cmake_policy(SET CMP0069 NEW)
> +    include(CheckIPOSupported)
> +    check_ipo_supported(RESULT CMAKE_IPO_AVAILABLE)
> +else()
> +    set(CMAKE_IPO_AVAILABLE FALSE)
> +endif()
> +
> +# Ensure that default value is false
> +set(CMAKE_INTERPROCEDURAL_OPTIMIZATION FALSE)

Nit: separate this variable initialization from the `if` below (because
it related to two `if`s below).

> +if (TARANTOOL_LTO AND CMAKE_IPO_AVAILABLE AND NOT TARGET_OS_DARWIN)
> +    execute_process(COMMAND ld -v OUTPUT_VARIABLE linker_v)

Maybe it is better to ask for ld.bfd (instead of just ld) explicitly and
set -fuse-ld-bfd explicitly. Despite it is discouraged (afaik) to use
gold as the default system linker such environments are possible. I had
use one for couple of years, because of some problem with bfd.

I think it would be more natural to check for ld.bfd existence as we do
for ld.gold.

> +	message(STATUS "LTO linker_v ${linker_v}")

A tab instead of a whitespace.

> +
> +    # e.g. GNU ld (GNU Binutils) 2.29
> +    string(REGEX MATCH "^GNU ld [^)]+[)] ([0-9.]+).*$" linker_valid ${linker_v})
> +

On centos-7:

$ ld.bfd -v
GNU ld version 2.27-10.el7

It seems it will not match, because of lack of the closing parenthesis.

BTW, [)] is redundant, we can use \( (at least in PCRE).

> +    if (NOT linker_valid)
> +        message(FATAL_ERROR "Unsuported linker (ld expected)")

Typo: Unsuported -> Unsupported.

> +    endif()
> +
> +    set(linker_version ${CMAKE_MATCH_1})
> +    message(STATUS "Found linker ld VERSION ${linker_version}")
> +
> +    if (NOT linker_version VERSION_LESS "2.31")
> +        set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
> +    elseif(NOT linker_version VERSION_LESS "2.30")

Here you are check bfd version to make a decision about using gold. The
approach seems to be broken.

Yep, I saw environments with ability to tweak PATH to obtain a specific
bfd/gold version (separately).

> +        # 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)
> +        endif()
> +    endif()
> +endif()
> +
> +if (TARANTOOL_LTO AND CMAKE_IPO_AVAILABLE AND TARGET_OS_DARWIN)
> +    set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
> +endif()
> +

I don't get why here we don't check anything? Are exporting symbol
problems actual here?

> +message(STATUS "LTO enabled ${CMAKE_INTERPROCEDURAL_OPTIMIZATION}")
> diff --git a/src/lib/msgpuck b/src/lib/msgpuck
> index 3b8f3e59b..1eb56a447 160000
> --- a/src/lib/msgpuck
> +++ b/src/lib/msgpuck
> @@ -1 +1 @@
> -Subproject commit 3b8f3e59b62d74f0198e01cbec0beb9c6a3082fb
> +Subproject commit 1eb56a447ea35f9c403dbc8aa98a25336303c1cb
> diff --git a/src/lib/small b/src/lib/small
> index 22d1bad18..0627a7f98 160000
> --- a/src/lib/small
> +++ b/src/lib/small
> @@ -1 +1 @@
> -Subproject commit 22d1bad1873edcb9b1383a273e70c4cf881d5349
> +Subproject commit 0627a7f986ff91701bd741908818749fef5e2266
> diff --git a/third_party/libyaml b/third_party/libyaml
> index 6bd4be1a7..e554afad0 160000
> --- a/third_party/libyaml
> +++ b/third_party/libyaml
> @@ -1 +1 @@
> -Subproject commit 6bd4be1a7751d6022a413864666880bef8a87c3c
> +Subproject commit e554afad015b2c7df76f9d5213d883e931246fad
> -- 

We should not update submodules to non-master branches. Please, send
peliminary patches for submodules first.

> 2.14.1
> 
> 

[tarantool-patches] Re: [PATCH 3/3] Add LTO testing && refactor travis.yml

[Alexander Turenko]

Need to rebase it on top of current 1.10, because RelWithDebInfoWError
build type was introduced.

Also keep in the mind that the branch
Totktonada/gh-3673-add-linux-clang-ci-target made similar CI targets
refactoring and likely will be in 1.10 soon.

WBR, Alexander Turenko.

On Wed, Aug 08, 2018 at 02:10:03PM +0300, AKhatskevich wrote:
> ---
>  .travis.mk  |  2 +-
>  .travis.yml | 50 +++++++++-----------------------------------------
>  2 files changed, 10 insertions(+), 42 deletions(-)
> 
> diff --git a/.travis.mk b/.travis.mk
> index 66c921aa7..8077cddba 100644
> --- a/.travis.mk
> +++ b/.travis.mk
> @@ -49,7 +49,7 @@ deps_osx:
>  	brew install openssl readline curl icu4c --force
>  
>  test_osx: deps_osx
> -	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo
> +	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo ${CMAKE_EXTRA_PARAMS}
>  	# Increase the maximum number of open file descriptors on macOS
>  	sudo sysctl -w kern.maxfiles=20480 || :
>  	sudo sysctl -w kern.maxfilesperproc=20480 || :
> diff --git a/.travis.yml b/.travis.yml
> index 997302e16..ee713be91 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -4,11 +4,7 @@ services:
>  
>  language: cpp
>  
> -os:
> -  - linux
> -  - osx
> -
> -osx_image: xcode9
> +os: linux
>  
>  cache:
>      directories:
> @@ -37,44 +33,16 @@ env:
>  matrix:
>      allow_failures:
>        - TARGET=coverage
> -#      - env: OS=el DIST=6
> -#      - env: OS=el DIST=7
> -#      - env: OS=fedora DIST=26
> -#      - env: OS=fedora DIST=27
> -#      - env: OS=ubuntu DIST=artful
> -#      - env: OS=ubuntu DIST=trusty
> -#      - env: OS=ubuntu DIST=xenial
> -#      - env: OS=ubuntu DIST=bionic
> -#      - env: OS=debian DIST=wheezy
> -#      - env: OS=debian DIST=jessie
> -#      - env: OS=debian DIST=stretch
> -    exclude:
> -      - env: OS=el DIST=6
> -        os: osx
> -      - env: OS=el DIST=7
> -        os: osx
> -      - env: OS=fedora DIST=26
> -        os: osx
> -      - env: OS=fedora DIST=27
> -        os: osx
> -      - env: OS=ubuntu DIST=artful
> -        os: osx
> -      - env: OS=ubuntu DIST=trusty
> -        os: osx
> -      - env: OS=ubuntu DIST=xenial
> -        os: osx
> -      - env: OS=ubuntu DIST=bionic
> -        os: osx
> -      - env: OS=debian DIST=wheezy
> -        os: osx
> -      - env: OS=debian DIST=jessie
> -        os: osx
> -      - env: OS=debian DIST=stretch
> -        os: osx
> -      - env: TARGET=source
> +    include:
> +      - osx_image: xcode9
>          os: osx
> -      - env: TARGET=coverage
> +        env:
> +          - TARGET=test
> +      - osx_image: xcode9
>          os: osx
> +        env:
> +          - TARGET=test
> +          - CMAKE_EXTRA_PARAMS="-DTARANTOOL_LTO=TRUE"
>  
>  script:
>    - make -f .travis.mk ${TARGET}
> -- 
> 2.14.1
> 
> 

[tarantool-patches] Re: [PATCH 2/3] Add LTO support

[Alex Khatskevich]

On 10.10.2018 17:29, Alexander Turenko wrote:
> On Wed, Aug 08, 2018 at 02:10:02PM +0300, AKhatskevich wrote:
>> This commit introduces LTO support.
>>
>> Changes:
>>    - update submodules (fix lto-related warnings)
> The msgpack submodule update contains a commit that are not related to
> your change. We should not hide such changes in the commit history.
> Maybe it is better to update submodules in a separate commit with proper
> description of all changes it pulls into the main repository.
That commit is just a testcase fix. Nothing special.
>>    - add `TARANTOOL_LTO` cmake option (by default it is `False`)
> It seems such options usually looks like ENABLE_XXX=[ON/OFF].
Renamed to ENABLE_LTO
>> 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)
>>      - mac xcode >= 8 (earlier versions are not tested)
>> This small patch required fixing bug in GNU ld linker.
>> (link gcc.gnu.org/bugzilla/show_bug.cgi?id=84901)
>> `ld` was not exporting symbols from dynamic list when LTO enabled.
>>
>> Closes #3117
>> ---
>>   CMakeLists.txt      |  1 +
>>   cmake/lto.cmake     | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   src/lib/msgpuck     |  2 +-
>>   src/lib/small       |  2 +-
>>   third_party/libyaml |  2 +-
>>   5 files changed, 59 insertions(+), 3 deletions(-)
>>   create mode 100644 cmake/lto.cmake
>>
>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>> index d0cae0f01..432d2a6fb 100644
>> --- a/CMakeLists.txt
>> +++ b/CMakeLists.txt
>> @@ -63,6 +63,7 @@ include(cmake/pod2man.cmake)
>>   include(cmake/arch.cmake)
>>   include(cmake/os.cmake)
>>   include(cmake/compiler.cmake)
>> +include(cmake/lto.cmake NO_POLICY_SCOPE)
> Is it for propagating cmake_policy(SET CMP0069 NEW)? Please, comment it.
Add commit at the beginning of `lto.cmake` file. I suppose it is the 
most suitable place.
>>   include(cmake/simd.cmake)
>>   include(cmake/atomic.cmake)
>>   include(cmake/profile.cmake)
>> diff --git a/cmake/lto.cmake b/cmake/lto.cmake
>> new file mode 100644
>> index 000000000..96129ebfd
>> --- /dev/null
>> +++ b/cmake/lto.cmake
>> @@ -0,0 +1,55 @@
>> +##
>> +## Manage LTO (Link-Time-Optimization) and IPO (Inter-Procedural-Optimization)
>> +##
>> +
>> +# 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 (bfd)
>> +
>> +if (NOT DEFINED TARANTOOL_LTO)
>> +    set(TARANTOOL_LTO FALSE)
>> +endif()
>> +
>> +if(NOT CMAKE_VERSION VERSION_LESS 3.9)
>> +    cmake_policy(SET CMP0069 NEW)
>> +    include(CheckIPOSupported)
>> +    check_ipo_supported(RESULT CMAKE_IPO_AVAILABLE)
>> +else()
>> +    set(CMAKE_IPO_AVAILABLE FALSE)
>> +endif()
>> +
>> +# Ensure that default value is false
>> +set(CMAKE_INTERPROCEDURAL_OPTIMIZATION FALSE)
> Nit: separate this variable initialization from the `if` below (because
> it related to two `if`s below).
done
>> +if (TARANTOOL_LTO AND CMAKE_IPO_AVAILABLE AND NOT TARGET_OS_DARWIN)
>> +    execute_process(COMMAND ld -v OUTPUT_VARIABLE linker_v)
> Maybe it is better to ask for ld.bfd (instead of just ld) explicitly and
> set -fuse-ld-bfd explicitly. Despite it is discouraged (afaik) to use
> gold as the default system linker such environments are possible. I had
> use one for couple of years, because of some problem with bfd.
>
> I think it would be more natural to check for ld.bfd existence as we do
> for ld.gold.
TODO
>> +	message(STATUS "LTO linker_v ${linker_v}")
> A tab instead of a whitespace.
fixed
>> +
>> +    # e.g. GNU ld (GNU Binutils) 2.29
>> +    string(REGEX MATCH "^GNU ld [^)]+[)] ([0-9.]+).*$" linker_valid ${linker_v})
>> +
> On centos-7:
>
> $ ld.bfd -v
> GNU ld version 2.27-10.el7
>
> It seems it will not match, because of lack of the closing parenthesis.
>
> BTW, [)] is redundant, we can use \( (at least in PCRE).
Fixed
>> +    if (NOT linker_valid)
>> +        message(FATAL_ERROR "Unsuported linker (ld expected)")
> Typo: Unsuported -> Unsupported.
fixed
>
>> +    endif()
>> +
>> +    set(linker_version ${CMAKE_MATCH_1})
>> +    message(STATUS "Found linker ld VERSION ${linker_version}")
>> +
>> +    if (NOT linker_version VERSION_LESS "2.31")
>> +        set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
>> +    elseif(NOT linker_version VERSION_LESS "2.30")
> Here you are check bfd version to make a decision about using gold. The
> approach seems to be broken.
>
> Yep, I saw environments with ability to tweak PATH to obtain a specific
> bfd/gold version (separately).
discussed.
Now the minimal supported binutils version is 2.31, and the ld is not 
changed implicitly.
>> +        # 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)
>> +        endif()
>> +    endif()
>> +endif()
>> +
>> +if (TARANTOOL_LTO AND CMAKE_IPO_AVAILABLE AND TARGET_OS_DARWIN)
>> +    set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
>> +endif()
>> +
> I don't get why here we don't check anything? Are exporting symbol
> problems actual here?
Because it worked well on all mac environments that I found.
>> +message(STATUS "LTO enabled ${CMAKE_INTERPROCEDURAL_OPTIMIZATION}")
>> diff --git a/src/lib/msgpuck b/src/lib/msgpuck
>> index 3b8f3e59b..1eb56a447 160000
>> --- a/src/lib/msgpuck
>> +++ b/src/lib/msgpuck
>> @@ -1 +1 @@
>> -Subproject commit 3b8f3e59b62d74f0198e01cbec0beb9c6a3082fb
>> +Subproject commit 1eb56a447ea35f9c403dbc8aa98a25336303c1cb
>> diff --git a/src/lib/small b/src/lib/small
>> index 22d1bad18..0627a7f98 160000
>> --- a/src/lib/small
>> +++ b/src/lib/small
>> @@ -1 +1 @@
>> -Subproject commit 22d1bad1873edcb9b1383a273e70c4cf881d5349
>> +Subproject commit 0627a7f986ff91701bd741908818749fef5e2266
>> diff --git a/third_party/libyaml b/third_party/libyaml
>> index 6bd4be1a7..e554afad0 160000
>> --- a/third_party/libyaml
>> +++ b/third_party/libyaml
>> @@ -1 +1 @@
>> -Subproject commit 6bd4be1a7751d6022a413864666880bef8a87c3c
>> +Subproject commit e554afad015b2c7df76f9d5213d883e931246fad
>> -- 
> We should not update submodules to non-master branches. Please, send
> peliminary patches for submodules first.
Ok. I will attach patches to the submodules to this tread.
>
>> 2.14.1

new diff:

commit dfd9c70a525477299cc075b5f0e6acb6ed14e335
Author: AKhatskevich <avkhatskevich@tarantool.org>
Date:   Thu Mar 22 11:37:06 2018 +0300

     Add LTO support

     This commit introduces LTO support.

     Changes:
       - update submodules (fix lto-related warnings)
       - add `TARANTOOL_LTO` cmake option (by default it is `False`)

     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)
         - mac xcode >= 8 (earlier versions are not tested)
     This small patch required fixing bug in GNU ld linker.
     (link gcc.gnu.org/bugzilla/show_bug.cgi?id=84901)
     `ld` was not exporting symbols from dynamic list when LTO enabled.

     Closes #3117

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 439a2750a..08ca1bbfd 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -63,6 +63,7 @@ include(cmake/pod2man.cmake)
  include(cmake/arch.cmake)
  include(cmake/os.cmake)
  include(cmake/compiler.cmake)
+include(cmake/lto.cmake NO_POLICY_SCOPE)
  include(cmake/simd.cmake)
  include(cmake/atomic.cmake)
  include(cmake/profile.cmake)
diff --git a/cmake/lto.cmake b/cmake/lto.cmake
new file mode 100644
index 000000000..8e00ad302
--- /dev/null
+++ b/cmake/lto.cmake
@@ -0,0 +1,52 @@
+##
+## Manage LTO (Link-Time-Optimization) and IPO
+## (Inter-Procedural-Optimization)
+##
+
+# 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 (bfd)
+
+# This cmake module exports CMP0069 policy and should be included
+# with NO_POLICY_SCOPE option.
+
+if (NOT DEFINED ENABLE_LTO)
+    set(ENABLE_LTO FALSE)
+endif()
+
+if(NOT CMAKE_VERSION VERSION_LESS 3.9)
+    cmake_policy(SET CMP0069 NEW)
+    include(CheckIPOSupported)
+    check_ipo_supported(RESULT CMAKE_IPO_AVAILABLE)
+else()
+    set(CMAKE_IPO_AVAILABLE FALSE)
+endif()
+
+# Ensure that default value is false
+set(CMAKE_INTERPROCEDURAL_OPTIMIZATION FALSE)
+
+if (ENABLE_LTO AND CMAKE_IPO_AVAILABLE AND NOT TARGET_OS_DARWIN)
+    execute_process(COMMAND ld.bfd -v OUTPUT_VARIABLE linker_v)
+    message(STATUS "LTO linker_v ${linker_v}")
+
+    # e.g. GNU ld (GNU Binutils) 2.29
+    string(REGEX MATCH "^GNU (go)?ld [^0-9]+ ([2-3][.][0-9]+).*$" 
linker_valid ${linker_v})
+
+    if (NOT linker_valid)
+        message(FATAL_ERROR "Unsupported linker version format")
+    endif()
+
+    set(linker_version ${CMAKE_MATCH_2})
+    message(STATUS "Found binutils VERSION ${linker_version}")
+
+    if (NOT linker_version VERSION_LESS "2.31")
+        set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
+    endif()
+endif()
+
+if (ENABLE_LTO AND CMAKE_IPO_AVAILABLE AND TARGET_OS_DARWIN)
+    set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE)
+endif()
+
+message(STATUS "LTO enabled ${CMAKE_INTERPROCEDURAL_OPTIMIZATION}")
diff --git a/src/lib/msgpuck b/src/lib/msgpuck
index 3b8f3e59b..1eb56a447 160000
--- a/src/lib/msgpuck
+++ b/src/lib/msgpuck
@@ -1 +1 @@
-Subproject commit 3b8f3e59b62d74f0198e01cbec0beb9c6a3082fb
+Subproject commit 1eb56a447ea35f9c403dbc8aa98a25336303c1cb
diff --git a/src/lib/small b/src/lib/small
index 22d1bad18..0627a7f98 160000
--- a/src/lib/small
+++ b/src/lib/small
@@ -1 +1 @@
-Subproject commit 22d1bad1873edcb9b1383a273e70c4cf881d5349
+Subproject commit 0627a7f986ff91701bd741908818749fef5e2266
diff --git a/third_party/libyaml b/third_party/libyaml
index 6bd4be1a7..e554afad0 160000
--- a/third_party/libyaml
+++ b/third_party/libyaml
@@ -1 +1 @@
-Subproject commit 6bd4be1a7751d6022a413864666880bef8a87c3c
+Subproject commit e554afad015b2c7df76f9d5213d883e931246fad

[tarantool-patches] Re: [PATCH 1/3] Fix: prevent guard-breaker optimization

[Alex Khatskevich]

On 10.10.2018 17:26, Alexander Turenko wrote:
> On Wed, Aug 08, 2018 at 02:10:01PM +0300, AKhatskevich wrote:
>> In case of very aggressive optimizations the compiler can
>> optimize guard-breaker function away and the `unit/guard`
>> test would fail.
> I think it is good to mention the specific compiler options (and a
> certain compiler you are using to reproduce it) to give a user an idea
> when things are going wrong and so what is the fix does.
>
> Is it due to discarding the noinline attribute in case of LTO on gcc? So
> 'volatile' prevents inlining the function? I just guessing here, but I
> think the commit message should clarify such things if possible.
>
>> ---
>>   test/unit/guard.cc | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/test/unit/guard.cc b/test/unit/guard.cc
>> index 231b44c7d..2082dfd48 100644
>> --- a/test/unit/guard.cc
>> +++ b/test/unit/guard.cc
>> @@ -13,7 +13,7 @@ static int __attribute__((noinline))
>>   stack_break_f(char *ptr)
>>   {
>>   	char block[2048];
>> -	char sum = 0;
>> +	volatile char sum = 0;
> I think a reason of using 'volatile' keyword should be always properly
> commented in the code, because it always fix some unobvious compiler
> behaviour.
Discussed verbally.
Added a comment near volatile keyword.
>>   	memset(block, 0xff, 2048);
>>   	sum += block[block[4]];
>>   	ptrdiff_t stack_diff = ptr > block ? ptr - block : block - ptr;
>> -- 
>> 2.14.1
>>
>>

New diff:

commit d7acdc09aa471ea15b3821bab205d8fcdb9280c1
Author: AKhatskevich <avkhatskevich@tarantool.org>
Date:   Tue Aug 7 16:47:39 2018 +0300

     Fix: prevent guard-breaker optimization

     In case of very aggressive optimizations the compiler can
     optimize guard-breaker function away and the `unit/guard`
     test would fail.

diff --git a/test/unit/guard.cc b/test/unit/guard.cc
index 231b44c7d..3d42fee31 100644
--- a/test/unit/guard.cc
+++ b/test/unit/guard.cc
@@ -13,7 +13,11 @@ static int __attribute__((noinline))
  stack_break_f(char *ptr)
  {
      char block[2048];
-    char sum = 0;
+    /*
+     * Make sum volatile to prevent a compiler from
+     * optimizing away call to this function.
+     */
+    volatile char sum = 0;
      memset(block, 0xff, 2048);
      sum += block[block[4]];
      ptrdiff_t stack_diff = ptr > block ? ptr - block : block - ptr;

[tarantool-patches] [PATCH] Enable 0069 policy

[AKhatskevich]

From: AKhatskevich <avkhatskevich@gmail.com>

Without this policy cmake >= 3.9 produces warnings when its parent
project uses LTO.
---
 CMakeLists.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c129b77..9e41751 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,6 +1,12 @@
 project(msgpuck)
 cmake_minimum_required(VERSION 2.8.5)
 
+# Avoid warnings when building with cmake>=3.9 and LTO is enabled in parent
+# project.
+if(NOT CMAKE_VERSION VERSION_LESS 3.9)
+    cmake_policy(SET CMP0069 NEW)
+endif()
+
 set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=c99 -fPIC -fstrict-aliasing")
 set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -Wall -Wextra -Werror")
 
-- 
2.14.1

[tarantool-patches] [tarantool-small] Enable 0069 policy

[AKhatskevich]

From: AKhatskevich <avkhatskevich@gmail.com>

Without this policy cmake >= 3.9 produces warnings when its parent
project uses LTO.
---
 CMakeLists.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index ad27423..8094023 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,6 +1,12 @@
 project(small C CXX)
 cmake_minimum_required(VERSION 2.8 FATAL_ERROR)
 
+# Avoid warnings when building with cmake>=3.9 and LTO is enabled in parent
+# project.
+if(NOT CMAKE_VERSION VERSION_LESS 3.9)
+    cmake_policy(SET CMP0069 NEW)
+endif()
+
 if(NOT CMAKE_BUILD_TYPE)
     set(CMAKE_BUILD_TYPE Debug)
 endif()
-- 
2.14.1

[tarantool-patches] [tarantool-libyaml] Enable 0069 policy

[AKhatskevich]

From: AKhatskevich <avkhatskevich@gmail.com>

Without this policy cmake >= 3.9 produces warnings when its parent
project uses LTO.
---
 CMakeLists.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e20a494..362e33f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -2,6 +2,12 @@
 cmake_minimum_required(VERSION 2.8)
 project (yaml C)
 
+# Avoid warnings when building with cmake>=3.9 and LTO is enabled in parent
+# project.
+if(NOT CMAKE_VERSION VERSION_LESS 3.9)
+    cmake_policy(SET CMP0069 NEW)
+endif()
+
 set (YAML_VERSION_MAJOR 0)
 set (YAML_VERSION_MINOR 1)
 set (YAML_VERSION_PATCH 7)
-- 
2.14.1

[tarantool-patches] [tarantool-msgpuck] Enable 0069 policy

[AKhatskevich]

From: AKhatskevich <avkhatskevich@gmail.com>

Without this policy cmake >= 3.9 produces warnings when its parent
project uses LTO.
---
 CMakeLists.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c129b77..9e41751 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,6 +1,12 @@
 project(msgpuck)
 cmake_minimum_required(VERSION 2.8.5)
 
+# Avoid warnings when building with cmake>=3.9 and LTO is enabled in parent
+# project.
+if(NOT CMAKE_VERSION VERSION_LESS 3.9)
+    cmake_policy(SET CMP0069 NEW)
+endif()
+
 set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=c99 -fPIC -fstrict-aliasing")
 set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -Wall -Wextra -Werror")
 
-- 
2.14.1