Attention is currently required from: Angel Pons, Nico Huber.
Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78206?usp=email )
Change subject: nb/intel/sandybridge: Pre-render constants in MRC pei_data ......................................................................
Patch Set 4:
(1 comment)
File src/northbridge/intel/sandybridge/raminit_mrc.c:
https://review.coreboot.org/c/coreboot/+/78206/comment/de126dd3_f9d50a4a : PS4, Line 404: struct pei_data pei_data;
This much easier to maintain, and also allows initialising fields whose value is always the result o […]
This is a (pretty extreme) optimization attempt. With struct initialiser, there is still runtime code to populate the struct, although more optimized still, to the point it took me some time to understand what gcc did.
I did a code size comparison of baseline (before this patch), cut struct (my proposal), struct initialiser (yours).
raminit_mrc.o size: 45052/45724/44888 (bytes, respectively) perform_raminit() length: 1957/1862/1926 End of code offset before reset vector: 7feed0/7feef0/7feed0
bootblock and romstage (if separated) size are same at 57344 and 52848 bytes respectively.
I don't see any further savings including system_type and gbe_enable in the struct initialiser.
Finally, as struct initialiser also generated code to zero the struct for us, I'm comfortable dropping memset() and anything that set a field to 0, but I would need to add a comment saying compiler did it for us, lest someone thinks we have (another) uninitialized variable issue.