Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git: Force use -O2 when lto enabled #24470

Closed
wants to merge 1 commit into from
Closed

Conversation

Ra2-IFV
Copy link
Contributor

@Ra2-IFV Ra2-IFV commented Jun 27, 2024

Git doen't support lto with gcc flag -O3 and -Ofast. This is discussed in #24366.
If CONFIG_USE_LTO=y, filter -O% from CPPFLAGS and set to -O2.

Maintainer: @krant @neheb
Compile tested: aarch64 f637cf5ef73bbecaf6183e5c416e3043a5f1e202
Run tested: aarch64 f637cf5ef73bbecaf6183e5c416e3043a5f1e202 looks good

Description:

Git doen't support lto with gcc flag -O3 and -Ofast.
This is discussed in openwrt#24366.
If CONFIG_USE_LTO=y, filter -O% from CPPFLAGS and set to -O2.

Signed-off-by: Ryan Keane <[email protected]>
@Ra2-IFV
Copy link
Contributor Author

Ra2-IFV commented Sep 28, 2024

🤔

@krant
Copy link
Contributor

krant commented Sep 28, 2024

That would switch git to -O2 even for those who have TARGET_OPTIMIZATION=-Os which is the default.
Another issue - it overwrites TARGET_CPPFLAGS which could contain important bits set by build system, like here.
Overall, you put filtered-out CFLAGS into CPPFLAGS and then pass them both to git's make, full gcc cmdline must be a mess.

@Ra2-IFV
Copy link
Contributor Author

Ra2-IFV commented Sep 28, 2024

Yeah that't not expected behavior... Closing but keep it here in case someone has the same problem.

@Ra2-IFV Ra2-IFV closed this Sep 28, 2024
@Ra2-IFV
Copy link
Contributor Author

Ra2-IFV commented Nov 12, 2024

@krant Sorry for the ping, but I have some progress on this...

ifeq ($(CONFIG_USE_LTO),y)
	ifneq (,$(filter -O3 -Ofast,$(TARGET_CFLAGS)))
		TARGET_CFLAGS := $(filter-out -O%,$(TARGET_CFLAGS)) -O2
	endif
        ifneq (,$(filter -O3 -Ofast,$(TARGET_CXXFLAGS)))
		TARGET_CXXFLAGS := $(filter-out -O%,$(TARGET_CXXFLAGS)) -O2
	endif
endif

How about this one? At least it seems won't break the flags...
Or we can assume CFLAGS and CXXFLAGS have the same optimization levels (at least in most cases it does) and remove one if statements...
I was just back from learning Makefile syntax and the difference between CFLAGS, CXXFLAGS and CPPFLAGS, and, for best practices, never put flags that aren't for the C pre-processor, if you are trying to apply a flag for both C and CPP files...

@Ra2-IFV Ra2-IFV deleted the git branch November 13, 2024 06:00
@Ra2-IFV Ra2-IFV restored the git branch November 13, 2024 06:00
@Ra2-IFV
Copy link
Contributor Author

Ra2-IFV commented Nov 13, 2024

New update on this:
This is a reproducible bug, reported in this mailing list https://lore.kernel.org/git/CAHfWF5mjquES-nocQaK+CAEsqWgdy-_OYdGtN82heYs0eJP3eQ@mail.gmail.com/T/#t
And I was wrong, it's caused by fortify headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants