Sram hunting

[vDorst]

I am hunting for SRAM bytes.

I am looking in every generated output/*.asm to see what is using SRAM.
Looking for section

;--------------------------------------------------------
; internal ram data
;--------------------------------------------------------

1. Some are easy fixes. Like static pointer that is not declared to put in XDATA.
2. But other are by the compiler that allocates SRAM forever, for function that are only called once at boottime.
3. Deadstore, the compiler stores a bool flag, but never reads it back because that part is already optimized out. (found at-least 3 placed and bytes).
4. a fix SRAM byte is used for only one function to pass a argument. Easy fix is just add __xdata in the front. Although that is not ideal, I am not sure why SDCC is doing that.
5. added multiple 32-bit values needed more sram.

Still work in progress but I want it to share it already.

Current progress: 31 bytes of SRAM freed.
Baseline code: c86d4b3 vs this PR.

< 0x10:|b|b|b|b|b|b|b|b|b|b|b|c|c|c|c|k|
< 0x20:|B|B|T|d|d|e|e|e|e|e|e|e|e|e|e|e|
< 0x30:|e|e|e|e|e|e|e|e|e|e|e|e|e|e|e|e|
< 0x40:|e|e|f|f|f|f|f|g|g|g|g|g|g|g|g|g|
< 0x50:|g|g|g|g|g|g|h|h|h|h|h|h|h|h|h|h|
< 0x60:|i|i|i|i|i|i|i|i|i|j|j|j|j|j|j|j|
< 0x70:|j|j|j|j|j|Q|Q|Q|Q|Q|Q|Q|Q|S|S|S|
---
> 0x10:|b|b|b|b|b|b|b|b|b|b|b|c|c|h| | |
> 0x20:|B|B|T|d|d|d|d|d|d|d|d|d|d|d|d|d|
> 0x30:|d|d|d|d|d|d|d|d|d|d|d|e|e|e|e|e|
> 0x40:|e|e|e|e|f|f|f|f|f|f|f|f|g|g|g|g|
> 0x50:|g|g|g|g|g|g|Q|Q|Q|Q|Q|Q|Q|Q|S|S|
> 0x60:|S|S|S|S|S|S|S|S|S|S|S|S|S|S|S|S|
> 0x70:|S|S|S|S|S|S|S|S|S|S|S|S|S|S|S|S|
21,22c21,22
< 16 bit mode initial stack starts at: 0x7d (sp set to 0x7c) with 131 bytes available.
< No spare internal RAM space left.
---
> 16 bit mode initial stack starts at: 0x5e (sp set to 0x5d) with 162 bytes available.
> The largest spare internal RAM space starts at 0x1e with 2 bytes available.
28,29c28,29
<    EXTERNAL RAM     0x0001   0x27c7   10183    16777216
<    ROM/EPROM/FLASH  0x0000   0x2d2fc  94666    16777216
---
>    EXTERNAL RAM     0x0001   0x27ea   10218    16777216
>    ROM/EPROM/FLASH  0x0000   0x2d32a  95117    16777216

[DrDoof]

Nice direction — the double-__xdata on pointer variables is the classic silent SRAM sink in small-model SDCC builds, and it's worth hunting. I built the branch against its base (c86d4b3) to get numbers, so a few notes while you're still iterating.

Caveat up front: this is build-time analysis only. I have not flashed or run any of these images on hardware, so nothing below says the SFP path still works — only that the code still compiles, links, and emits the same bytes.

Measured deltas

Full builds, MACHINE chosen to cover the SFP dimension since that's what the send_status() refactor touches:

MACHINE	n_sfp	free SRAM (base → PR)	XDATA	ROM
ZX310S_4T2XT	0	135 → 145 B	+18 B	+218 B
SWTGW218AS	1	135 → 145 B	+18 B	+218 B
STEAMEMO_IG204_V1	2	135 → 145 B	+18 B	+218 B

So −10 bytes of internal RAM, at a cost of +18 B XDATA and +218 B ROM. Consistent across all three. Good trade here given the linker already says No spare internal RAM space left, but the ROM cost isn't mentioned in the description and it's the larger number.

I couldn't reproduce the 14 B figure. Also — and this is the main reason I'm writing — the patch block in the description reads backwards: the < lines are the patched build and > is the base. Your > values (10183 XDATA, 94666 ROM) match my base build exactly, and < (0x6f/145 B, 10201) match my patched build exactly. As written it looks like the change costs 14 B of stack and saves ROM, which is the opposite of what it does. The stack figures are the only pair that don't line up with mine, so the two .mem files may have come from different machines.

Static checks — no runtime testing

To be clear, none of this was verified on a switch. What I did check, at build time:

For the send_status() rewrite I diffed the string literals extracted from both linked images — the only difference in the whole JSON/HTTP literal set is

- ,"sfp_options":"0x
+ 1,"sfp_options":"0x

i.e. exactly the bool_to_html(1) fold. Nothing else in the wire format moved. make machine_check also passes on all 23 machines. That's evidence the JSON shape is preserved; it is not evidence that the I2C reads still return the right values, and someone with an SFP module in hand should confirm that before this lands.

Dropping the = 0 on content_type/session is safe twice over, FWIW: __mcs51_genXRAMCLEAR (GSINIT4) zeroes all of XSEG at startup, and scan_header() resets both at entry before any read anyway.

Small things

1. &sfp_module_vendor[sfp] has type __xdata char (*)[17], not __xdata char *. Same address and sdcc doesn't warn, but a stricter compiler would — plain sfp_module_vendor[sfp] is the intended spelling. Same for _model / _serial.
2. sfp and sfp_len aren't static, so they're exported to the linker under very generic names. static costs nothing and avoids a future collision. Note there's already a parameter named sfp in send_sfp_info() just above, which shadows the new global — harmless today, but it means anyone later calling sfp_send_data() from inside that function silently gets the wrong slot.
3. xstrtox() writes a terminating '\0' that the loops it replaces didn't. It's always overwritten by the next strtox() and outbuf is 2500 B so there's no overflow, but it is a quiet semantic change in a function named like strtox.

[vDorst]

Thanks for the quick review. Tomorrow I will look into it.

[DrDoof]

Re-measured against e79d849. First, a correction to my earlier comment: the numbers I posted there were for commit 5 and I never said so, which is misleading now that the branch has moved. Here's the whole picture, all against base c86d4b3 and identical on the 0-, 1- and 2-SFP machines (ZX310S_4T2XT, SWTGW218AS, STEAMEMO_IG204_V1):

	SRAM	XDATA	ROM
commit 5	−10 B	+18	+218
commit 9	−27 B	+35	+455

Two things I noticed reading it, neither of which breaks anything today.

memcmp() returns true when the buffers are equal. That's inverted relative to C's memcmp, and relative to strcmp() sitting a few lines away in the same file. There's exactly one caller so nothing is wrong in practice, but the next person to use it will get it backwards. mem_equal() would say what it does and would sit nicely next to is_mem_zero().

The other is in uip_arp_update(): the "is this entry in use" test changes from AND to OR. That actually matches uip_arp_timer(), which was already using (ipaddr[0]|ipaddr[1]) != 0, so the two are consistent now — but the commit message doesn't mention it, and a behaviour change hidden in a size-reduction commit is easy to lose. The trigger is narrow: the two versions only disagree for an address with exactly one zero 16-bit half, i.e. x.y.0.0. The 0.0.0.0 you get from DHCP is zero in both halves and behaves the same either way.

Things I checked at commit 9 and found fine: the JSON and HTTP literals are unchanged apart from folding bool_to_html(1) into "1"; make machine_check passes 23/23; memcmp/is_mem_zero land in HOME (0x0339 / 0x0370) and are called from BANK1 in uip_arp.c, which is the same arrangement as the existing memcpy at 0x02C1, so banking is fine; uip_arp.c does include ../rtl837x_common.h, so no implicit declarations; and scan_header/is_word_x are local to httpd.c with no header declaration, so changing a parameter's storage class can't drift against another translation unit.

All of the above is build-time analysis — I haven't run this on hardware.

[vDorst]

Still working on this I my own repo/branch https://github.com/vDorst/RTLPlayground/tree/SRAM-HUNTING_move_sfr_data.
I already freed-up more space and even moved sfr_data to SRAM.
But I am going to redo some parts and create new PR so changes are applied in batches instead of a large PR.