jrouwe / JoltPhysics

A multi core friendly rigid body physics and collision detection library. Written in C++. Suitable for games and VR applications. Used by Horizon Forbidden West.
MIT License
6.46k stars 418 forks source link

Release build for mac will enable bitcode #1261

Open shiena opened 3 days ago

shiena commented 3 days ago

In Release/Distribution builds for macOS/iOS/visionOS, INTERPROCEDURAL_OPTIMIZATION is ON so bitcode is enabled. I would like to see bitcode disabled by default as it has been deprecated since Xcode14. Also, there are cases where bitcode is required for older models, so I think it would be better to be able to enable it as an option.

Probably related to the following issue and pull req. https://github.com/jrouwe/JoltPhysics/issues/418 https://github.com/jrouwe/JoltPhysics/pull/357

mihe commented 3 days ago

What errors/warnings are you seeing exactly?

Distributing (and thus building for) bitcode is indeed deprecated, and while LTO/IPO in LLVM does similarly emit LLVM bitcode into the object files in order to optimize across them, I see no mention of LTO in AppleClang being deprecated along with it, and frankly I have a hard time seeing that being the case.

shiena commented 3 days ago

INTERPROCEDURAL_OPTIMIZATION causes the -flto=thin option to the compiler and generates an object file with bitcode output. This causes an Unknown header: 0xb17c0de error when creating the xcframework from the .a file. If INTERPROCEDURAL_OPTIMIZATION is turned off, bitcode is not output.

mihe commented 2 days ago

I assume then that you're not generating/building your Xcode project with CMake (and referencing the Jolt CMake target as part of that) but instead manually bundling the statically linked XCFramework?

If that's the case, I suppose there is some argument for disabling LTO by default when targeting Apple platforms, since there's the possibility of the static libraries being bundled into an XCFramework bundle. It could also be conditionally disabled only when PROJECT_IS_TOP_LEVEL is true and BUILD_SHARED_LIBS is false, in order to cater to FetchContent and shared libraries, but I'm not sure it's worth the hassle.

Frankly I'm not seeing much of a difference at all in PerformanceTest on my M2 Mac Mini with or without LTO, for reasons I can't quite understand, so that would seem to make this decision a lot simpler. I don't have an x86 Mac to test with anymore though, but those are quickly becoming irrelevant anyway I guess.