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

Smart linking #82

Open
bentley opened this issue Nov 27, 2015 · 14 comments · May be fixed by #1382
Open

Smart linking #82

bentley opened this issue Nov 27, 2015 · 14 comments · May be fixed by #1382
Labels
enhancement Typically new features; lesser priority than bugs rgblink This affects RGBLINK

Comments

@bentley
Copy link
Contributor

bentley commented Nov 27, 2015

20:51 <@beware> anjbe_: is "smart linking" something you can do? smart linking means: only include things that are actually referenced
20:51 <@beware> this mostly helps with libraries
20:51 <@beware> as not all of the library has to be linked
20:51 <@beware> libraries tend to contain a lot of things a project doesn't need
20:52 <@beware> i think a linker could do this transparently
20:52 <@beware> (as in, as part of the linker)
20:53 <@beware> to resolve "is symbol X being referenced", you need to trace it from the program's entry point
20:53 <@beware> because a library can reference a symbol, but if that library is never used, then this reference doesn't mean it should be included
20:54 <@beware> anything (that has a symbol) that is never referenced is "dead" (as in, dead code, dead data)
@PikalaxALT
Copy link
Contributor

PikalaxALT commented Feb 6, 2017

This would certainly reduce a lot of ROM bloat, such as in Pokemon Crystal EN-US.

@AntonioND
Copy link
Member

I'm not sure if this would be possible in all cases. Example:

    SECTION "A", ROMX[$4000], BANK[1]
LABEL_A:
    SECTION "B", ROMX[$4000], BANK[2]
LABEL_B:
    SECTION "C", ROMX[$4000], BANK[3]
LABEL_C:
    SECTION "D", ROMX[$4000], BANK[4]
LABEL_D:

SECTION "Code", ROM0
Function: ; a = Bank to read from
    ld [$2000],a
    ld a,[LABEL_A]
    ret

But maybe for floating sections...

@AntonioND AntonioND added enhancement Typically new features; lesser priority than bugs rgblink This affects RGBLINK labels Apr 2, 2018
@ISSOtm
Copy link
Member

ISSOtm commented Aug 28, 2019

Such sections could be specially marked as "preserve", couldn't they?

@JL2210
Copy link
Member

JL2210 commented May 3, 2020

Is there an IRC channel?

@ISSOtm
Copy link
Member

ISSOtm commented May 3, 2020

#gbdev on EFNet, or the GBDev Discord server, why?

@JL2210
Copy link
Member

JL2210 commented May 3, 2020 via email

@pinobatch
Copy link
Member

toxa on gbdev Discord reports that dropping unused sections (those not reachable from -s entry_label) still isn't implemented in ISSOtm's rewritten linker.

@JL2210
Copy link
Member

JL2210 commented Jul 25, 2020

I'm pretty sure it isn't a priority right now.

@Rangi42
Copy link
Contributor

Rangi42 commented Jul 25, 2020

This is not possible in general for non-floating sections, as AntonioND mentioned. People sometimes do write code that uses raw hex values for banks or addresses when they know what code/data to expect at those locations, so just looking at which labels are referenced wouldn't catch that.

There's also a case in pokecrystal where two data tables (for mon pics and Unown pics) are both orged to the same address in different banks, and only one label gets loaded into hl, but either table may be needed depending on which bank it switches to. And more generally, tricks like "I know hl == $4200 right now, so let's inc h to read the content I know is at $4300, which may not be referenced or labeled at all."

Perhaps floating sections could opt in to being left out if not referenced, like SECTION "Library Function Foo", ROMX, OPTIONAL, but even then the linker should print a notification if it gets omitted. (And any reference should count, whether it's ld hl, FunctionFoo or ld b, BANK("Library Function Foo") or dw FunctionFoo + 42.)

@pinobatch
Copy link
Member

We could assign -s - (or something like that) to mean treat every non-floating section as an entry label.

@aaaaaa123456789
Copy link
Member

Enabling a feature like this by default is downright insane; I don't think anyone was even proposing this. It's pretty clear something like this should only be enabled by a flag. Isn't there already a flag for this purpose (that perhaps doesn't work very well)?

@aaaaaa123456789
Copy link
Member

@Rangi42 labels like those are trivial to reference with an assert, and in fact that's a good idea anyway. (Adding assert(PokemonPicPointers == UnownPicPointers) or whatever the labels are to pokecrystal would probably be a smart call.)
That being said, assemblers/linkers automatically discarding code and data from assembly input by default is still insanity, and I don't think anyone would want that to be the default.

@Rangi42
Copy link
Contributor

Rangi42 commented Jul 26, 2020

@aaaaaa123456789 True, an assert would be ideal there (it falls under pret/pokecrystal#706), but the code is valid without one, and a "smart linker" should not break it.

It seems to me that since this would be a section-based feature, putting OPTIONAL instead of BANK[$XX] at the SECTION declaration would be a convenient method, since it relies on the developer to be explicit that they're not relying on the section. Rather than a -s linker flag that has to judge for all the sections (or at least for the floating ones).

@aaaaaa123456789
Copy link
Member

Maybe another section "kind", like we have SECTION UNION and SECTION FRAGMENT already?

ISSOtm added a commit to ISSOtm/rgbds that referenced this issue Dec 19, 2020
Long-broken feature, fell into disrepair, got removed...
Now it's back! Though, it needs testing.
Fixes gbdev#82
@ISSOtm ISSOtm mentioned this issue Feb 10, 2021
4 tasks
daid pushed a commit to daid/rgbds that referenced this issue Mar 4, 2021
Long-broken feature, fell into disrepair, got removed...
Now it's back! Though, it needs testing.
Fixes gbdev#82
daid pushed a commit to daid/rgbds that referenced this issue Mar 5, 2021
Long-broken feature, fell into disrepair, got removed...
Now it's back! Though, it needs testing.
Fixes gbdev#82
daid pushed a commit to daid/rgbds that referenced this issue Mar 10, 2021
Long-broken feature, fell into disrepair, got removed...
Now it's back! Though, it needs testing.
Fixes gbdev#82
@Rangi42 Rangi42 linked a pull request Mar 27, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Typically new features; lesser priority than bugs rgblink This affects RGBLINK
Projects
None yet
8 participants