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

Properly set EMAR for Interprocedural Optimization #14

Closed
wants to merge 2 commits into from

Conversation

Skylion007
Copy link
Contributor

This properly sets the EMAR for InterproceduralOptimization as described here: emscripten-core/emscripten#11143 and here mosra/magnum#490

@Squareys
Copy link
Contributor

Looks great, thanks @Skylion007 !

@mosra FYI, I do the -flto for Wonderland Engine, too (though, I set those at project level, not for compiling magnum). And faintly remember setting em-ar somewhere also.

Comment on lines +60 to +63
set(CMAKE_C_COMPILER_AR "${CMAKE_AR}")
set(CMAKE_CXX_COMPILER_AR "${CMAKE_AR}")
set(CMAKE_C_COMPILER_RANLIB "${CMAKE_RANLIB}")
set(CMAKE_CXX_COMPILER_RANLIB "${CMAKE_RANLIB}")
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have suspected that CMake initializes these automatically based on CMAKE_AR and CMAKE_RANLIB right after the toolchain file, but I assume that is not the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Squareys I wish. Emscriptem's toolchain does properly handle it, but Magnum's doesn't sadly.

set(CMAKE_CXX_FLAGS_RELEASE_INIT "-DNDEBUG -O3")
set(CMAKE_EXE_LINKER_FLAGS_RELEASE_INIT "-O3 --llvm-lto 1")
set(CMAKE_CXX_FLAGS_RELEASE_INIT "-DNDEBUG -O3 -flto")
set(CMAKE_EXE_LINKER_FLAGS_RELEASE_INIT "-O3 --llvm-lto 1 -flto")
Copy link
Contributor

Choose a reason for hiding this comment

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

@mosra For your reference, here's the documentation for that: https://emscripten.org/docs/compiling/WebAssembly.html#backends

--llvm-lto 1 is for the deprecated fastcomp backend (removed in emsc 2.0.0), -flto(=full|thin) is the new way of achieving link-time optimization

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, found #13, so you are already aware

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Squareys Yeah, unfortunately @mosra mentioned we need to support the fastcomp backend. Do you know an easy to way to check if we are using fastcomp or the more modern clang?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I was having trouble with the default build running into issues with ThinLTO. @Squareys have you gotten ThinLTO working with build? I got complaints from the wasm-ld.

@Skylion007 Skylion007 mentioned this pull request May 24, 2021
@Skylion007
Copy link
Contributor Author

Ping @mosra

@mosra
Copy link
Owner

mosra commented Sep 3, 2023

This was finally merged in 98bdb68 and b25ca89. The extra delay allowed me to not have to care about fastcomp compatibility anymore.

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.

3 participants