gbdev / rgbds

Rednex Game Boy Development System - An assembly toolchain for the Nintendo Game Boy and Game Boy Color
https://rgbds.gbdev.io
MIT License
1.33k stars 175 forks source link

Tweaks to rgblink .map file output #1099

Closed Rangi42 closed 1 year ago

Rangi42 commented 1 year ago
ISSOtm commented 1 year ago

The indent level of ; New union/; New fragment is because it's more of a section-level split (different fragment).

The purpose of that line is to explain why the rule that labels are sorted by address is exceptionally broken here. I'm not sure why one would want it removed.

Argeed about that third one regardless.

Rangi42 commented 1 year ago

Right, the ; New union/fragment comment explains something about labels, which is why I'd want it omitted with -M.

As for why to remove it completely: the labels are sorted by address. This asm:

SECTION UNION "Misc", WRAM0
wFoo1:: db
wFoo2:: dw
wFoo3:: db

SECTION UNION "Misc", WRAM0
wBar1:: db
wBar2:: db
wBar3:: db
wBar4:: db

SECTION UNION "Misc", WRAM0
wQux1:: dw
wQux2:: db
wQux3:: db

produces this map, which obeys the usual rule of sorting symbols by address:

WRAM0 bank #0:
    SECTION: $c000-$c003 ($0004 bytes) ["Misc"]
        ; New union
             $c000 = wQux1
             $c000 = wFoo1
             $c000 = wBar1
             $c001 = wFoo2
             $c001 = wBar2
             $c002 = wQux2
             $c002 = wBar3
             $c003 = wQux3
             $c003 = wFoo3
             $c003 = wBar4
    EMPTY: $c004-$cfff ($0ffc bytes)
    TOTAL EMPTY: $0ffc bytes

SUMMARY:
    WRAM0: 4 bytes used / 4092 free

I would see the usefulness of the comment if it were like this:

WRAM0 bank #0:
    SECTION: $c000-$c003 ($0004 bytes) ["Misc"]
        ; New union
             $c000 = wFoo1
             $c001 = wFoo2
             $c003 = wFoo3
        ; New union
             $c000 = wBar1
             $c001 = wBar2
             $c002 = wBar3
             $c003 = wBar4
        ; New union
             $c000 = wQux1
             $c002 = wQux2
             $c003 = wQux3
    EMPTY: $c004-$cfff ($0ffc bytes)
    TOTAL EMPTY: $0ffc bytes

SUMMARY:
    WRAM0: 4 bytes used / 4092 free
ISSOtm commented 1 year ago

Ah, yeah, that's caused by RGBASM merging the fragments itself, so RGBLINK sees them all as a single one. I'm not sure how to go about fixing this. Unique ID? Idk.

Rangi42 commented 1 year ago

How about printing comments between identically-named unions or fragments, instead of before each one? So:

WRAM0 bank #0:
    SECTION: $c000-$c003 ($0004 bytes) ["Misc"]
             $c000 = wFoo1
             $c001 = wFoo2
             $c003 = wFoo3
        ; Next union
             $c000 = wBar1
             $c001 = wBar2
             $c002 = wBar3
             $c003 = wBar4
        ; Next union
             $c000 = wQux1
             $c002 = wQux2
             $c003 = wQux3
    EMPTY: $c004-$cfff ($0ffc bytes)
    TOTAL EMPTY: $0ffc bytes
ISSOtm commented 1 year ago

Yes, that would be better, I think.

Also, I got confused with -M for a moment, so I agree with removing ; New union/fragment from it.

rlewicki commented 1 year ago

Hey, I think I found where is this weird indent on EMPTY coming from however I'm not exactly sure how would I go about testing it? It's inside output.c:485 file.

ISSOtm commented 1 year ago

Yeah, this should be using a tab instead:

https://github.com/gbdev/rgbds/blob/1a9fc964dfa3f157f35ea0c7e1952d51ec982887/src/link/output.c#L485

(You can paste the link you get from "Copy permalink" to get this embed)

Rangi42 commented 1 year ago

In that case, let's omit the EMPTY: $[the section size] bytes as redundant. So it's just:

ROMX bank #117:
    EMPTY