rgbds
rgbds copied to clipboard
Implement smart linking, take 3
Fixes #82.
Replaces #654 and #782. Many thanks to @ISSOtm and @daid for getting smart linking to this point!
- [ ] Allow referencing sections from the linkerscript? Or even from the assembly too?
- [ ] Add a mechanism so that RGBASM-elided references are still emitted (lacking this is why test/link/smart/chain-wram.asm fails)
- [ ] Add more tests covering more edge cases
- [ ] Multiple
-sreferenced sections - [ ] Any still-uncovered code (use contrib/coverage.bash)
- [ ] Multiple
- [x] Sections should not be purged if fully constrained
- [x] Apply smart linking after linker script (avoids errors & interacts with above)
One of the test cases from #782 currently fails. Note that #782 failed it as well, so that's a TODO.
smart/chain-wram.smart.bin /tmp/tmp.ETsdvFO98G differ: char 3, line 1
00:0000
< 00:0000: 00c0 0102 0000 0000 0000 0000 0000 0000 ................
---
00:0000
> 00:0000: 00c0 0000 0000 0000 0000 0000 0000 0000 ................
smart/chain-wram.smart.out.bin mismatch!
This is the asm:
SECTION "root", ROM0[0]
dw WRAMLabel
; This section should be kept thanks to the reference from the WRAM section
SECTION "A", ROM0
Label:
db $01, $02
.end:
SECTION "wram", WRAM0
WRAMLabel:
ds Label.end - Label
SECTION "UnRef", ROM0
UnRef:
db UnRef
One of the test cases from #782 currently fails. Note that #782 failed it as well, so that's a TODO.
smart/chain-wram.smart.bin /tmp/tmp.ETsdvFO98G differ: char 3, line 1 00:0000 < 00:0000: 00c0 0102 0000 0000 0000 0000 0000 0000 ................ --- 00:0000 > 00:0000: 00c0 0000 0000 0000 0000 0000 0000 0000 ................ smart/chain-wram.smart.out.bin mismatch!This is the asm:
SECTION "root", ROM0[0] dw WRAMLabel ; This section should be kept thanks to the reference from the WRAM section SECTION "A", ROM0 Label: db $01, $02 .end: SECTION "wram", WRAM0 WRAMLabel: ds Label.end - Label SECTION "UnRef", ROM0 UnRef: db UnRef
Not sure why that would be a valid test. There is no way to know where section "A" is located, you only know how big it is.
RGBASM is able to compute the difference between these two labels itself, and so it does: the ds line is processed as if it was ds 2, and RGBASM does not emit a reference to Label nor Label.end.
Yet, since section "wram" is kept, and it contains a source-code reference to Label, "A" should be kept by "smart linking". Shouldn't it?
At a detailed source level, yes, it contains a reference. But at a practical level higher overview, it does not contain a reference. It only contains a reference to the size (which is constant, and thus can be pre-computed before linking)
IMHO, it's one of those edge cases that does not require to work as there is no practical usage of this in any way. While fixing it is annoyingly needlessly complicated.
Before this is merged, I should probably say that specifying the starting sections in the command line isn't the best idea.
I'd expect this to be given as a section attribute in the source code itself: SECTION "Foobar", ROM0, NODISCARD (or whatever you want to call the "treat this section as always referenced" attribute).
Of course, this doesn't mean that the command line flag cannot be used, but I'd expect to see a way to specify this in code — which would also function as an escape hatch for the above "should this section be included?" problem.
IIRC we wanted the linker script to also support designating root sections.
Proposal: add an RPN_REFCONST expression type, which would be like a constant expression, but also tracking lists of the symbol and section names which went into computing its value. When an asm-time constant is emitted that depends on a name and/or on other REFCONST expressions, build up the list. Then RGBLINK's patch_FindReferencedSections would take those names into account for finding references.
(This would be a new expression type so the existing RPN_CONST would remain unchanged, and would not be bloated by 0 section and symbol counts for the majority of cases where there aren't any.)
@ISSOtm @daid What do you think?
Edit: Oh, y'all discussed the exemplifying test case already. I agree with ISSOtm that using ld a, BANK(Label) or dw Label - OtherLabel or so on is intuitively as much of a reference as ld hl, Label is; the asm-time constant evaluation is just an optimization by us.