godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.97k stars 20.18k forks source link

LTO for web builds failing: attempt to add bitcode file after LTO (`htonl.o`) #92371

Open Swarkin opened 3 months ago

Swarkin commented 3 months ago

Tested versions

Current master [be56cab58c056c074d1e02cd0b38641204e39f41]

System information

Windows 10; Tested with emsdk 3.1.59 and 3.1.53

Issue description

I am encountering the following error when trying to compile with lto=full for wasm:

wasm-ld: error: C:\Users\Me\Documents\emsdk\upstream\emscripten\cache\sysroot\lib\wasm32-emscripten\lto\libc-debug.a(htonl.o): attempt to add bitcode file after LTO (htonl)

full output.txt

Steps to reproduce

Clone Godot source Set up web compilation requirements Run:

scons p="web" arch="wasm32" target="template_release" optimize="size" lto="full" deprecated="no" disable_3d="yes" vulkan="no" use_volk="no" openxr="no" minizip="no" brotli="no" graphite="no" threads="no" javascript_eval="no" modules_enabled_by_default="no" module_gdscript_enabled="yes" module_freetype_enabled="yes" module_text_server_adv_enabled="no" module_text_server_fb_enabled="yes"

Minimal reproduction project (MRP)

see above

Swarkin commented 3 months ago

I can confirm that the same build args work fine for non-web compilations. (p=windows arch=x86_64).

Swarkin commented 3 months ago

adding linkflags="-Wl,-u,htonl" seems to be a workaround to it, but increases file size by a bit. the docs explicitly mention lto should be used as it drastically reduces file size. not sure whats going on here.

AThousandShips commented 3 months ago

Does it happen for you without all the remaining options disabling various features? Or just with that specific set of options? Because I can't confirm, I can build just fine with lto

Testing with the specific instructions and will see if it works on my end

AThousandShips commented 3 months ago

Can confirm with those options, will try pin down what specifically causes it tomorrow

AThousandShips commented 3 months ago

I've narrowed it down to modules_enabled_by_default="no", so that will be a starting point for working things out with this, don't know what specifically would cause that though, something for the team to take a look at

akien-mga commented 3 months ago

See:

    if env["lto"] != "none":
        # Workaround https://github.com/emscripten-core/emscripten/issues/19781.
        if cc_semver >= (3, 1, 42) and cc_semver < (3, 1, 46):
            env.Append(LINKFLAGS=["-Wl,-u,scalbnf"])
        # Workaround https://github.com/emscripten-core/emscripten/issues/16836.
        if cc_semver >= (3, 1, 47):
            env.Append(LINKFLAGS=["-Wl,-u,_emscripten_run_callback_on_thread"])

It's not the first time we need to workaround Emscripten LTO bugs where it doesn't include a symbol when linking which is actually needed by the code LLVM is emitting.

htonl is one such case like scalbnf, which you'll find references to in the Emscripten issue tracker, notably: https://github.com/emscripten-core/emscripten/issues/16836#issuecomment-1556187350

BTW @Swarkin I see you commented there already, this kind of information is important to bring up in a downstream Godot bug report so that our issue triagers don't spend time redoing research that you already did.

akien-mga commented 3 months ago

I saw a suggestion that one might be able to get most benefits from LTO by compiling the objects with -flto, but linking without: https://github.com/emscripten-core/emscripten/issues/19781#issuecomment-1682710178

So I tried that, which works. But the resulting binaries need to be benchmarked to see if there's any impact on performance (notably in the GDScript VM, which benefits a lot from it).

On the size, for the minimal template built with the command from OP, disabling LTO actually keeps the binary smaller by over 600 KiB. With -flto on compiling + linking and -Wl,-u,htonl, the binary is slightly smaller than -flto on compiling but not on linking.

-rwxr-xr-x. 1 akien akien 29700770 May 30 12:35 bin/godot.web.template_release.wasm32.nothreads.wasm.lto_but_no_link_flag
-rwxr-xr-x. 1 akien akien 29681033 May 30 12:58 bin/godot.web.template_release.wasm32.nothreads.wasm.lto_with_u-htonl
-rwxr-xr-x. 1 akien akien 29065652 May 30 12:39 bin/godot.web.template_release.wasm32.nothreads.wasm.no_lto

All in all LTO might not really help with binary size on web. Last I checked ThinLTO was even worse.

What we need to assess is how much it helps on performance. If it's not huge on Web (compared to other platforms like Linux and Windows where we tested performance increases in the GDScript VM of up to 20%), then it might actually be best to make web builds without LTO (and that should save a ton of build time, as Emscripten/LLVM LTO linking is super slow because single-threaded).

baldierot commented 1 month ago

attempt to add bitcode file after LTO (htonl) occurs if enet is disabled using module_enet_enabled="no" or modules_enabled_by_default="no". Happens with Godot 4.3 Beta 3 and Godot 3.6 RC 1.

Calinou commented 1 month ago

I'm surprised ENet is also compiled in web builds, considering web browsers can't send or receive UDP packets (which is ENet's only transport layer). Disabling it on the web platform would help reduce the default binary size a bit.