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

Workaround mingw-gcc LTO ICE by re-adding some dead code... #102506

Merged

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented Feb 7, 2025

Not my finest work, but without that code that was removed in #102179, mingw-gcc 14.2.1 on Fedora 41 (but also confirmed with versions on macOS and WSL) crashes when linking with LTO with the following command:

scons production=yes p=windows arch=x86_64 module_mono_enabled=yes target=editor

And this error:

lto1: internal compiler error: cannot read 'LTO_section_decls' from /tmp/ccf5RNMk.ltrans115.o
Please submit a full bug report, with preprocessed source (by using -freport-bug).
See <http://bugzilla.redhat.com/bugzilla> for instructions.
make: *** [/tmp/cct9c3iK.mk:347: /tmp/ccf5RNMk.ltrans115.ltrans.o] Error 1
make: *** Waiting for unfinished jobs....

We need to dig deeper to understand the bug, report it upstream and work it around in a cleaner way. But for now this unblocks building Windows binaries with LTO, and should be harmless. This could likely be made more minimal by cutting through the code I'm re-adding until it fails linking again, but it's pretty tedious and I need to start a 4.4.beta3 build now :)

I'll write a proper bug report tomorrow so we can keep tracking that issue and make sure to revert this hack and replace it by an actual fix.

More details in this Rocket.chat thread: https://chat.godotengine.org/channel/buildsystem?msg=3LzNsqenWK55Bb8gt

Comment on lines 2424 to 2429
// HACK: We don't want to run the simulator library generation code anymore after GH-102179, but removing it
// triggers an internal compiler error with latest mingw-gcc... For now we bring it back as a workaround
// with a condition that should never be true (that project setting doesn't exist).

// Check and generate missing ARM64 simulator library.
if (ProjectSettings::get_singleton()->has_setting("ios_generate_simulator_library_if_missing")) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main difference with the previous code before #102179. Instead of checking a preset option (which #102179 removed), I'm checking a non-existing project setting. This should prevent the compiler from optimizing all this code away (it doesn't know that the project setting doesn't exist), but should also prevent it from running when exporting to iOS (unless a user decides to create this magic project setting manually).

This should really be a temporary hack (famous last words).

@akien-mga
Copy link
Member Author

Note for when we revert this to replace it by a better fix: the lipo.h and macho.h includes can be removed, as after #102179 they're unused. Sadly, just removing them isn't enough to fix the lto-wrapper crash.

Not my finest work, but without that code removed in godotengine#102179, mingw-gcc 14.2.1 on Fedora 41
(but also confirmed with versions on macOS and WSL) crashes when linking with LTO.

We need to dig deeper to understand the bug, report it upstream and work it around in a
cleaner way. But for now this unblocks building Windows binaries with LTO, and should be
harmless.
@akien-mga akien-mga force-pushed the mingw-gcc-lto-one-the-rocks branch from b05b3ca to e12a424 Compare February 7, 2025 00:16
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfection

@akien-mga akien-mga merged commit 06acfcc into godotengine:master Feb 7, 2025
20 checks passed
@akien-mga akien-mga deleted the mingw-gcc-lto-one-the-rocks branch February 7, 2025 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants