Rework main pool to use alloc only pool logic #859

aglab2 · 2025-01-30T14:10:01Z

This is refactored logic from PR #737 that implements new main pool logic.

arthurtilly · 2025-01-30T22:13:01Z

src/boot/memory.c

-    }
-    return addr;
+// takes the at least first 'size' bytes from 'region' and return pointer aligned on 'alignment'
+static void* main_pool_region_try_alloc_from_start_aligned(struct MainPoolRegion* region, u32 size, u32 alignment) {


all these functions seem quite messy and repetitive. there must be a way to consolidate some of these into fewer functions, perhaps with macros or whatnot. also where is main_pool_alloc defined now, i may be blind but i cant find it

main_pool_alloc is static inline in the header file. It is incredibly fast compiling to roughly 4 instructions.

Gotcha. Do you think the functions defined here could be cleaned up a little? The names and quantity of them are quite confusing

Will work on it

Yeah while I'd prefer long function names over poorly named ones, 9-word function names is a bit on the large side.

I added one "big" function that makes allocation with FORCEINLINE to enable const propagations. I hope it is good enough although there is still a bit of "7-word" functioning here. I'd rather be explicit with names because reading memory code tends to be complicated.

arthurtilly · 2025-01-30T22:14:19Z

src/boot/memory.c

@@ -352,10 +361,17 @@ void *load_segment(s32 segment, u8 *srcStart, u8 *srcEnd, u32 side, u8 *bssStart
 void *load_to_fixed_pool_addr(u8 *destAddr, u8 *srcStart, u8 *srcEnd) {
    void *dest = NULL;
    u32 srcSize = ALIGN16(srcEnd - srcStart);
-    u32 destSize = ALIGN16((u8 *) sPoolListHeadR - destAddr);
+    u32 destSize = ALIGN16((u8 *) RAM_END - destAddr);


is this assuming that main pool ends at end of ram? i think there might be a better variable to use here although not sure

load_to_fixed_pool_addr is loading at fixed address in the end of RAM.

src/engine/surface_load.c

src/boot/memory.c

src/game/memory.h

src/boot/memory.c

src/game/memory.h

gheskett · 2025-02-16T07:23:40Z

src/boot/memory.c

+    struct MainPoolRegion* region = &gMainPool.regions[region_idx];
+    char errmsg[70];
+    sprintf(errmsg, "%salloc failed for %d bytes in region %d\n%x-%x", freeable ? "freeable " : "", size, region_idx, region->start, region->end);
+    __n64Assert((char*) file, line, errmsg);


nit: ~~use the error macro to simplify this (and to make replacing these with assert_args or similar easier to find once 2.4 gets merged into 3.0)~~

Nevermind, I see what's going on here. Add a TODO then for replacing errmsg with gAssertionStr once 2.4 is merged.

gheskett · 2025-02-16T07:41:35Z

src/boot/memory.c

+void *main_pool_alloc_ex(int region, u32 size, u32 alignment) {
+    void* buf = region ? main_pool_region_alloc_from_start(region, size, alignment, MAIN_POOL_ALLOC_TRY) : NULL;
+    return buf ?: main_pool_region_alloc_from_start(0, size, alignment, MAIN_POOL_ALLOC_FORCE);


nit: I feel like this is needlessly difficult to read

src/boot/memory.c

gheskett · 2025-02-16T07:54:24Z

src/game/memory.h

+ Main pool initial state is a multiple regions of memory:
+ |-------|   |----|  |-------------------|
+ If alloc(sizeof(+++++)), first region is used and the state becomes:
+ |+++++--|   |----|  |-------------------|
+  ^
+  returned pointer, no extra memory overhead


The segmented portion was removed, but this comment still poses more questions than answers. Can I still allocate to the other memory regions? Are the other regions even supported in the first place?

Yes, it can if you explicitly call main_pool_region_alloc_from_start

I would add an example to this then to demonstrate that use case.

gheskett · 2025-02-16T08:08:43Z

src/game/memory.h

+    u8* end;
+};
+
+#define MAIN_POOL_REGIONS_COUNT 2


If regions are still specifiable and involve more than one use in HackerSM64, then we should still be using enums to specify region IDs.

aglab2 requested review from Reonu, gheskett and arthurtilly as code owners January 30, 2025 14:10

gheskett mentioned this pull request Jan 30, 2025

Rework main pool alloc to support fragmented buffer. Move zbuffer & fb to different memory banks #737

Closed

arthurtilly reviewed Jan 30, 2025

View reviewed changes

src/engine/surface_load.c Show resolved Hide resolved