search

libretro / FBNeo

FBNeo - We are Team FBNeo.
https://neo-source.com
Other
228 stars 136 forks source link

cleanup Emscripten build #1055

Closed ethanaobrien closed 1 year ago

ethanaobrien commented 1 year ago

Emscripten should be built with -O3, not -O2

barbudreadmon commented 1 year ago

The current makefile builds msvc with -O2 and anything else with -O3 (including emscripten), so i don't understand the purpose of this PR ?

Furthermore, what do you mean by that ? Is there some kind of limitation preventing -O2 to work with emscripten ?

ethanaobrien commented 1 year ago

The current makefile builds msvc with -O2 and anything else with -O3 (including emscripten), so i don't understand the purpose of this PR ?

From the looks of it, it's the other way around, am I understanding this incorrectly?

Furthermore, what do you mean by that ? Is there some kind of limitation preventing -O2 to work with emscripten ?

With the way emscripten works, a higher number results in higher compilation time, but faster runtime, which is a concern for running in the browser. See https://emscripten.org/docs/optimizing/Optimizing-Code.html

barbudreadmon commented 1 year ago

am I understanding this incorrectly?

Yes you are, i just confirmed from the libretro buildbot that everything is built using -O3.

With the way emscripten works, a higher number results in higher compilation time, but faster runtime, which is a concern for running in the browser.

Then that's no different from all other platforms.

On a sidenote, using -O3 is not something that should be done lightly. Back around 2016, FinalBurn used to have quite a lot of crashes with it, and we spent weeks tracking and fixing all of those issues. I don't recommend sending that kind of PR without first being extremely familiar with a project's source code.

ethanaobrien commented 1 year ago

Yes you are, i just confirmed from the libretro buildbot that everything is built using -O3.

Alright, I'll close this then