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

Get rid of common syms #2040

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

luckytyphlosion
Copy link
Member

Also only build tools once per makefile usage, error out if the submakes fail (only make 4.2+), print truncated link statement in terminal

Description

luckytyphlosion — 06/16/2019
okay so this is how a common symbol is defined:
size <= 4: ALIGN by 4
4 < size <= 8: ALIGN by 8
size > 8: ALIGN by 16

then, define the symbol
you can't simply replace it with a section like common_data because it doesn't align correctly
it'll only align by 4
however, considering that ramscrgen just reads from the object file and manually outputs the symbols in the order they're defined based on common syms
you could probably make common_data anyway, but repurpose ramscrgen to fix the alignment of common_data symbols
and this would mean no common_syms/

Discord contact info

luckytyphlosion

Also only build tools once per makefile usage, error out if the submakes fail (only make 4.2+), print truncated link statement in terminal
@GriffinRichards
Copy link
Member

GriffinRichards commented Sep 30, 2024

Please fix the changes using .SHELLSTATUS to work for versions of Make < 4.2 or revert them. We don't need the error badly enough to break older versions of Make.

Comment on lines 44 to +45
INCLUDE "sym_common.ld"
src/*.o(COMMON);
Copy link
Member

@GriffinRichards GriffinRichards Sep 30, 2024

Choose a reason for hiding this comment

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

This won't include any COMMON_DATA symbols that users add to new files if they aren't explicitly included (i.e. we should wildcard common_data here too).

fwiw we had to revert wildcarding COMMON last time it was introduced here, I don't remember what the problem was specifically but I recall people with certain linker versions having issues with it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can imagine wildcarding COMMON not working previously because of the way that common symbols were previously included in the build (extracting them from the object files and then re-ordering them with alignment). However, since no variables in src/ use common variables anymore, we don't run into this problem.

The problem with wildcarding COMMON however occurs with common_data. src/*.o(common_data); in the linkerscript will cause the link process to run out of IWRAM, probably because the wildcarding isn't excluding files already in the build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants