search

minetest / irrlicht

Minetest's fork of Irrlicht
Other
114 stars 87 forks source link

Keep RTTI for Minetest IPO/LTO #268

Closed okias closed 9 months ago

okias commented 9 months ago

Minetest using RTTI, so we cannot apply the flag here if we want to start using IPO/LTO.

If we keep -fno-rtti here, compiler loses part of informations and cannot do efficient optimizations.

The benefit of not using RTTI on Irrlicht disappears with when Minetest and Irrlicht gets IPO/LTO linked together.

sfan5 commented 9 months ago

Not that it ultimately matters but this is my first time hearing of RTTI being relevant for optimization/LTO and I couldn't find anything on it. Do you have some reference material for me?

okias commented 9 months ago

Do you have some reference material for me?

The best I see is https://stackoverflow.com/questions/15101859/subclassing-class-from-shared-library-compiled-with-fno-rtti/15213718#15213718

Without LTO: We compile Irrlicht, we compile minetest, and link it together. That's all. Linker doesn't care (two types of separate objects in one file)

With LTO: Both code gets compiled and at linking time another set of optimizations is being done. The linker sees one set of definitions from minetest (RTTI) objects and a second from Irrlicht (no-RTTI), which makes it impossible to do right optimizations + triggers ODR warnings because it sees two a bit different definitions.

sfan5 commented 9 months ago

Hm, that's a bit weird because I'm pretty sure we don't subclass most types.

To be clear, did you check that the warnings disappear when enabling RTTI in Irrlicht?

okias commented 9 months ago

To be clear, did you check that the warnings disappear when enabling RTTI in Irrlicht?

Yup.