pokeemerald icon indicating copy to clipboard operation
pokeemerald copied to clipboard

Have pokeemerald use OFast, aapcs, and ftoplevel-reorder for MODERN by default

Open AreaZeroArven opened this issue 1 year ago • 4 comments

The big question is: why make these changes?

Let's take a look: OFast is a compiler flag that allows for some standard-breaking optimizations, mostly in the floating point category. Pokeemerald rarely uses floating points and where it does, it does not require extreme accuracy. In addition, OFast optimizes for speed, not space, of which there is always 32 MB of anyway.

AAPCS is the ABI that supports arm7tdmi chips and up. It mostly mandated how functions start and end in order to interwork with thumb code. apcs-gnu was back when pokeemerald was still being decompiled.

Finally, ftoplevel-reorder, it is not able to be disabled on clang, and without disabling top-level reordering, the game does not boot. Or rather, it doesn't boot because only one file, m4a.c, does not play nicely with the compiler, causing the game to crash when initializing the sound. For this reason, m4a.c gets its own exception in the Makefile.

These changes should be beneficial to anyone compiling on MODERN and there does not seem to be any downsides. These changes were thoroughly tested for weeks on my end, to ensure no side effects happened. I hope you enjoy this!

Signed-off-by: Arven

AreaZeroArven avatar Nov 03 '23 21:11 AreaZeroArven

We cannot assume that people are okay with non-standard floating point optimizations. If users believe that they need the extra speed, then they can enable it themselves. Also, -Ofast implies loop unrolling, which might produce larger code (whether that matters is another thing).

I'm not fully sure on the history of apcs-gnu vs aapcs.

fno-toplevel-reorder mainly exists for the case where the game does UB with adjacency of variables. The example I thought of (saveblock ASLR) appears to have been fixed, but there may be other instances available. Also, some people may prefer functions and variables to be in the order that they are defined. I'd imagine that the majority of people would prefer that behaviour since it is deterministic and knowable (although the the modern loadscript doesn't have a deterministic object order anyway so maybe that is precedence against fno-toplevel-reorder, but perhaps a nondeterministic object order was a mistake anyway).

luckytyphlosion avatar Nov 15 '23 03:11 luckytyphlosion

@PikalaxALT says that apcs-gnu is required to link with agbcc's libgcc, but I'm not sure if current pokeemerald uses agbcc libraries anyway (I don't think so but didn't do thorough investigation)

luckytyphlosion avatar Nov 15 '23 03:11 luckytyphlosion

I predict that this PR will rot because nobody here knows enough about modern gcc stuff to accurately determine whether these changes will break anything.

luckytyphlosion avatar Nov 15 '23 03:11 luckytyphlosion

I was genuinely interested in this, partly at least because -fno-toplevel-reorder seems kind of useless. fwiw it seems like the author did not test the changes because it just crashes the game on boot.

About aapcs: We probably don't want to do that, because it imposes a double-word stack alignment that is very non-beneficial for the arm7tdmi.

About -Ofast I think it was already discussed, but -ffast-math is not really something you'd want. Additionally this generates quite aggressive loop unrollings due to O3.

SBird1337 avatar Jun 08 '24 12:06 SBird1337