From 35a83c3514f38003f656274cc6946baca0d0724c Mon Sep 17 00:00:00 2001 From: David Heidelberg Date: Fri, 9 Feb 2024 00:01:12 +0100 Subject: [PATCH] Enable IPO/LTO by default except for debug builds (#14198) Test case: ``` $ cmake . -DRUN_IN_PLACE=TRUE -DCMAKE_BUILD_TYPE=Release -DBUILD_SERVER=TRUE -DENABLE_TOUCH=FALSE minetest minetestserver W/o LTO: 13M 7.3M W/ LTO: 11M 5.9M difference: 15% 19% ``` Also fixes various compiler warnings resulting from compilation using LTO. --------- Signed-off-by: David Heidelberg --- CMakeLists.txt | 30 ++++++++++++++++++------- doc/compiling/README.md | 1 + src/client/particles.cpp | 4 ++-- src/gui/guiTable.cpp | 2 +- src/nodedef.cpp | 2 +- src/script/lua_api/l_server.cpp | 40 ++++++++++++++++----------------- util/ci/build.sh | 3 ++- 7 files changed, 49 insertions(+), 33 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c86aff85b..4b5f4e896 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,11 +1,4 @@ -cmake_minimum_required(VERSION 3.5) - -# Set policies up to 3.9 since we want to enable the IPO option -if(${CMAKE_VERSION} VERSION_LESS 3.9) - cmake_policy(VERSION ${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION}) -else() - cmake_policy(VERSION 3.9) -endif() +cmake_minimum_required(VERSION 3.12) # This can be read from ${PROJECT_NAME} after project() is called project(minetest) @@ -44,6 +37,13 @@ set(BUILD_UNITTESTS TRUE CACHE BOOL "Build unittests") set(BUILD_BENCHMARKS FALSE CACHE BOOL "Build benchmarks") set(BUILD_DOCUMENTATION TRUE CACHE BOOL "Build documentation") +set(DEFAULT_ENABLE_LTO TRUE) +# by default don't enable on Debug builds to get faster builds +if(CMAKE_BUILD_TYPE STREQUAL "Debug") + set(DEFAULT_ENABLE_LTO FALSE) +endif() +set(ENABLE_LTO ${DEFAULT_ENABLE_LTO} CACHE BOOL "Use Link Time Optimization") + set(DEFAULT_RUN_IN_PLACE FALSE) if(WIN32) set(DEFAULT_RUN_IN_PLACE TRUE) @@ -66,6 +66,20 @@ if(NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE Release CACHE STRING "Build type: Debug or Release" FORCE) endif() +# FIXME: Windows build fails in multiple places to link, needs to be investigated. +if (ENABLE_LTO AND NOT WIN32) + include(CheckIPOSupported) + check_ipo_supported(RESULT lto_supported OUTPUT lto_output) + if(lto_supported) + set(CMAKE_INTERPROCEDURAL_OPTIMIZATION TRUE) + message(STATUS "LTO/IPO is enabled") + else() + message(STATUS "LTO/IPO requested but it is not supported by the compiler: ${lto_output}") + endif() +else() + message(STATUS "LTO/IPO is not enabled") +endif() + set(ENABLE_UPDATE_CHECKER (NOT ${DEVELOPMENT_BUILD}) CACHE BOOL "Whether to enable update checks by default") diff --git a/doc/compiling/README.md b/doc/compiling/README.md index f4812e77d..cdc39b217 100644 --- a/doc/compiling/README.md +++ b/doc/compiling/README.md @@ -28,6 +28,7 @@ General options and their default values: ENABLE_REDIS=ON - Build with libhiredis; Enables use of Redis map backend ENABLE_SPATIAL=ON - Build with LibSpatial; Speeds up AreaStores ENABLE_SOUND=ON - Build with OpenAL, libogg & libvorbis; in-game sounds + ENABLE_LTO=ON - Build with IPO/LTO optimizations (smaller and more efficient than regular build) ENABLE_LUAJIT=ON - Build with LuaJIT (much faster than non-JIT Lua) ENABLE_PROMETHEUS=OFF - Build with Prometheus metrics exporter (listens on tcp/30000 by default) ENABLE_SYSTEM_GMP=ON - Use GMP from system (much faster than bundled mini-gmp) diff --git a/src/client/particles.cpp b/src/client/particles.cpp index 0697763ca..14384f3b8 100644 --- a/src/client/particles.cpp +++ b/src/client/particles.cpp @@ -213,10 +213,10 @@ void Particle::step(float dtime) if (m_p.animation.type != TAT_NONE) { m_animation_time += dtime; - int frame_length_i, frame_count; + int frame_length_i = 0; m_p.animation.determineParams( m_material.getTexture(0)->getSize(), - &frame_count, &frame_length_i, NULL); + NULL, &frame_length_i, NULL); float frame_length = frame_length_i / 1000.0; while (m_animation_time > frame_length) { m_animation_frame++; diff --git a/src/gui/guiTable.cpp b/src/gui/guiTable.cpp index b5042802a..b2c8e6abc 100644 --- a/src/gui/guiTable.cpp +++ b/src/gui/guiTable.cpp @@ -222,7 +222,7 @@ void GUITable::setTable(const TableOptions &options, s32 colcount = columns.size(); assert(colcount >= 1); // rowcount = ceil(cellcount / colcount) but use integer arithmetic - s32 rowcount = (content.size() + colcount - 1) / colcount; + s32 rowcount = std::min(((u32)content.size() + colcount - 1) / colcount, (u32)S32_MAX); assert(rowcount >= 0); // Append empty strings to content if there is an incomplete row s32 cellcount = rowcount * colcount; diff --git a/src/nodedef.cpp b/src/nodedef.cpp index cbfc77055..85b779429 100644 --- a/src/nodedef.cpp +++ b/src/nodedef.cpp @@ -716,7 +716,7 @@ static void fillTileAttribs(ITextureSource *tsrc, TileLayer *layer, int frame_count = 1; if (layer->material_flags & MATERIAL_FLAG_ANIMATION) { assert(layer->texture); - int frame_length_ms; + int frame_length_ms = 0; tiledef.animation.determineParams(layer->texture->getOriginalSize(), &frame_count, &frame_length_ms, NULL); layer->animation_frame_count = frame_count; diff --git a/src/script/lua_api/l_server.cpp b/src/script/lua_api/l_server.cpp index 12e5a1a5d..635d88d70 100644 --- a/src/script/lua_api/l_server.cpp +++ b/src/script/lua_api/l_server.cpp @@ -167,26 +167,6 @@ int ModApiServer::l_get_player_information(lua_State *L) return 1; } - /* - Be careful not to introduce a depdendency on the connection to - the peer here. This function is >>REQUIRED<< to still be able to return - values even when the peer unexpectedly disappears. - Hence all the ConInfo values here are optional. - */ - - auto getConInfo = [&] (con::rtt_stat_type type, float *value) -> bool { - return server->getClientConInfo(player->getPeerId(), type, value); - }; - - float min_rtt, max_rtt, avg_rtt, min_jitter, max_jitter, avg_jitter; - bool have_con_info = - getConInfo(con::MIN_RTT, &min_rtt) && - getConInfo(con::MAX_RTT, &max_rtt) && - getConInfo(con::AVG_RTT, &avg_rtt) && - getConInfo(con::MIN_JITTER, &min_jitter) && - getConInfo(con::MAX_JITTER, &max_jitter) && - getConInfo(con::AVG_JITTER, &avg_jitter); - ClientInfo info; if (!server->getClientInfo(player->getPeerId(), info)) { warningstream << FUNCTION_NAME << ": no client info?!" << std::endl; @@ -211,6 +191,26 @@ int ModApiServer::l_get_player_information(lua_State *L) } lua_settable(L, table); + /* + Be careful not to introduce a depdendency on the connection to + the peer here. This function is >>REQUIRED<< to still be able to return + values even when the peer unexpectedly disappears. + Hence all the ConInfo values here are optional. + */ + + auto getConInfo = [&] (con::rtt_stat_type type, float *value) -> bool { + return server->getClientConInfo(player->getPeerId(), type, value); + }; + + float min_rtt, max_rtt, avg_rtt, min_jitter, max_jitter, avg_jitter; + bool have_con_info = + getConInfo(con::MIN_RTT, &min_rtt) && + getConInfo(con::MAX_RTT, &max_rtt) && + getConInfo(con::AVG_RTT, &avg_rtt) && + getConInfo(con::MIN_JITTER, &min_jitter) && + getConInfo(con::MAX_JITTER, &max_jitter) && + getConInfo(con::AVG_JITTER, &avg_jitter); + if (have_con_info) { // may be missing lua_pushstring(L, "min_rtt"); lua_pushnumber(L, min_rtt); diff --git a/util/ci/build.sh b/util/ci/build.sh index 88349b852..944cc2124 100755 --- a/util/ci/build.sh +++ b/util/ci/build.sh @@ -1,7 +1,8 @@ -#! /bin/bash -e +#!/bin/bash -e cmake -B build \ -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE:-Debug} \ + -DENABLE_LTO=FALSE \ -DRUN_IN_PLACE=TRUE \ -DENABLE_GETTEXT=${CMAKE_ENABLE_GETTEXT:-TRUE} \ -DBUILD_SERVER=${CMAKE_BUILD_SERVER:-TRUE} \