#75 - Inject flags to enable LTO by default
Closed 3 years ago by law. Opened 4 years ago by law.
Unknown source lto-inject-2  into  master

file modified
+10 -1
@@ -230,7 +230,16 @@

  %_ld_as_needed		1

  %_ld_as_needed_flags	%{?_ld_as_needed:-Wl,--as-needed}

  

- %__global_compiler_flags	-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches %{_hardened_cflags} %{_annotated_cflags}

+ # Packages should use "%define _lto_cflags %{nil}" in their .spec

+ # file to opt-out of LTO

+ #

+ # Packages that need special handling for archive libraries should

+ # use something like this in their .spec file

+ # "%global _lto_cflags %{?_lto_cflags} -ffat-lto-objects -flto-partition=one"

this example does not make sense to me since it won't change anything because all those optiosn are in %_lto_cflags already

+ #

+ %_lto_cflags -flto -ffat-lto-objects -flto-partition=one

+ 

+ %__global_compiler_flags	-O2 %{?_lto_cflags} -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches %{_hardened_cflags} %{_annotated_cflags}

  

  # Automatically trim changelog entries after 2 years

  %_changelog_trimtime	%{lua:print(os.time() - 2 * 365 * 86400)}

file modified
+4 -1
@@ -6,7 +6,7 @@

  

  Summary: Red Hat specific rpm configuration files

  Name: redhat-rpm-config

- Version: 146

+ Version: 147

  Release: 1%{?dist}

  # No version specified.

  License: GPL+
@@ -207,6 +207,9 @@

  %{_rpmconfigdir}/macros.d/macros.kmp

  

  %changelog

+ * Wed Jan 08 2019 Jeff Law <law@redhat.com> - 147-1

+ - Inject flags to enable LTO by default

+ 

  * Thu Dec 05 2019 Denys Vlasenko <dvlasenk@redhat.com> - 146-1

  - kmod.prov: fix and speed it up

  

Not recommending this be merged at this time. Goal right now is to get consensus on the change, but defer merging until the configure fixes are also agreed upon and gcc-10 is in rawhide.

Please use a conditional (ie %{?_lto_cflags}) here to avoid unnecessary problems in case somebody/something undefines the macro.

I assume you mean in the reference to _lto_cflags in __global_compiler_flags? Easy enough. Done.

1 new commit added

  • Use a conditional for the reference to _lto_cflags
4 years ago

@law can you describe why we need those 3 options and document what they do?

this example does not make sense to me since it won't change anything because all those optiosn are in %_lto_cflags already

Funny timing Igor. I just finished a rebuild of rawhide on x86_64 and reevaluation of every configure difference with/without LTO -- I expect to restart discussions around integration of the redhat-rpm-config bits next week :-)

Great! Let me know once things are ready.

@law any updates here? I'd like to get this finally merged :)

Enabling of system-wide LTO is too dangerous. It can break lots of things. For example Clang+Qt+LTO == broken package due to upstream bug.

LTO must be an opt-in feature, not opt-out.

Enabling of system-wide LTO is too dangerous. It can break lots of things. For example Clang+Qt+LTO == broken package due to upstream bug.
LTO must be an opt-in feature, not opt-out.

It has been already approved by FESCo. https://fedoraproject.org/wiki/LTOByDefault

Igor, I wonder if you're somehow reading my todo list -- you're a couple days ahead of me again.

The way I'd like to proceed is not to merge this, but get agreement on the change. That allows us to get the relevant opt-outs in place first.

I'm currently looking at the i686 LTO results and I'm hoping to spin an aarch64 run next week.

WRT xvitaly's comments. Much of the QT system is not LTO ready and I expect it'll be on the opt-out list. A variety of issues are known upstream QT. Some are fixed, some are not. To my knowledge all the issues with QT and LTO are QT issues, not issues with LTO itself.

I don't think that fundamentally changes the trajectory of the LTO feature for Fedora. It's fully expected that some packages are going to need to opt-out.

Igor, I wonder if you're somehow reading my todo list -- you're a couple days ahead of me again.

Of course I do! :wine_glass:

The way I'd like to proceed is not to merge this, but get agreement on the change. That allows us to get the relevant opt-outs in place first.

Fair enough.

Metadata Update from @ignatenkobrain:
- Request assigned

3 years ago

@law please rebase this ASAP so that it is ready for mass rebuild which will start very soon.

I think I blew away the repo that I used for this pull request. I'm making a fresh one now.

Pull-Request has been closed by law

3 years ago