Remove gSystemHeap and use _buffersSegmentEnd instead, make buffer alignments explicit for gcc (#1901)

* Remove gSystemHeap and use _buffersSegmentEnd instead, make buffer alignments explicit for gcc

* ALIGNEDn -> ALIGNED(n), reposition alignment attribute for PreNmiBuff

* Correct positioning of ALIGNED for filter data in audio/lib/data.c

* Add ALIGNED to TypenameMacros

* ALIGNED(4) on same line for PreNmiBuff

* Revert audio load.c change, to be submitted separately

.clang-format

@@ -23,4 +23,4 @@ AllowShortEnumsOnASingleLine: false
 AlignEscapedNewlines: Left
 AlignTrailingComments: true
 SortIncludes: false
-TypenameMacros: ['BAD_RETURN']
+TypenameMacros: ['BAD_RETURN', 'ALIGNED']

include/alignment.h

@@ -8,9 +8,9 @@
 #define ALIGN256(val) (((val) + 0xFF) & ~0xFF)
 
 #ifdef __GNUC__
-#define ALIGNED8 __attribute__ ((aligned (8)))
+#define ALIGNED(n)  __attribute__ ((aligned (n)))
 #else
-#define ALIGNED8
+#define ALIGNED(n)
 #endif
 
 #ifdef __sgi /* IDO compiler */

include/ultra64/controller.h

@@ -119,6 +119,12 @@
 #define BTN_B           0x4000
 #define BTN_A           0x8000
 
+#ifdef __GNUC__
+// Ensure data cache coherency for OSPifRam structures by aligning to the data cache line size.
+// On older compilers such as IDO this was done by placing each OSPifRam at the top of the file it is declared in,
+// however file alignment should not be relied on in general.
+__attribute__((aligned(0x10)))
+#endif
 typedef union {
     struct {
     /* 0x00 */ u32 ram[15];

include/variables.h

@@ -235,6 +235,5 @@ extern u64 gGfxSPTaskYieldBuffer[OS_YIELD_DATA_SIZE / sizeof(u64)]; // 0xC00 byt
 extern u64 gGfxSPTaskStack[SP_DRAM_STACK_SIZE64]; // 0x400 bytes
 extern GfxPool gGfxPools[2]; // 0x24820 bytes
 extern u8 gAudioHeap[0x38000]; // 0x38000 bytes
-extern u8 gSystemHeap[];
 
 #endif

include/z64.h

@@ -755,7 +755,12 @@ typedef struct OverlayRelocationSection {
     /* 0x14 */ u32 relocations[1]; // size is nRelocations
 } OverlayRelocationSection; // size >= 0x18
 
-typedef struct {
+// This struct is used at osAppNMIBuffer which is not at an 8-byte aligned address. This causes an unaligned access
+// crash if the OSTime variables use 64-bit load/store instructions, which is the case in any MIPS ABI other than O32
+// where 64-bit load/store instructions are emulated with 2x 32-bit load/store instructions. The alignment attribute
+// conveys that this structure will not always be 8-bytes aligned, allowing a modern compiler to generate non-crashing
+// code for accessing these. This is not an issue in the original compiler as it only output O32 ABI code.
+ALIGNED(4) typedef struct {
     /* 0x00 */ u32 resetting;
     /* 0x04 */ u32 resetCount;
     /* 0x08 */ OSTime duration;

spec

@@ -604,7 +604,7 @@ beginseg
     align 0x40
     include "$(BUILD_DIR)/src/buffers/zbuffer.o"
     include "$(BUILD_DIR)/src/buffers/gfxbuffers.o"
-    include "$(BUILD_DIR)/src/buffers/heaps.o"
+    include "$(BUILD_DIR)/src/buffers/audio_heap.o"
 endseg
 
 beginseg

src/audio/lib/data.c

@@ -1,7 +1,7 @@
 #include "global.h"
 
 // clang-format off
-s16 gSawtoothWaveSample[] = {
+ALIGNED(16) s16 gSawtoothWaveSample[] = {
     // Frequency of 1
     0,      1023,   2047,   3071,   4095,   5119,   6143,   7167,
     8191,   9215,   10239,  11263,  12287,  13311,  14335,  15359,
@@ -43,7 +43,7 @@ s16 gSawtoothWaveSample[] = {
     0,      8191,   16383,  24575,  -32767, -24575, -16383, -8191,
 };
 
-s16 gTriangleWaveSample[] = {
+ALIGNED(16) s16 gTriangleWaveSample[] = {
     // Frequency of 1
     0,      2047,   4095,   6143,   8191,   10239,  12287,  14335,
     16383,  18431,  20479,  22527,  24575,  26623,  28671,  30719,
@@ -85,7 +85,7 @@ s16 gTriangleWaveSample[] = {
     0,      16383,  32767,  16383,  0,      -16383, -32767, -16383,
 };
 
-s16 gSineWaveSample[] = {
+ALIGNED(16) s16 gSineWaveSample[] = {
     // Frequency of 1
     0,      3211,   6392,   9511,   12539,  15446,  18204,  20787,
     23169,  25329,  27244,  28897,  30272,  31356,  32137,  32609,
@@ -127,7 +127,7 @@ s16 gSineWaveSample[] = {
     0,      23169,  32767,  23169,  0,      -23169, -32767, -23169,
 };
 
-s16 gSquareWaveSample[] = {
+ALIGNED(16) s16 gSquareWaveSample[] = {
     // Frequency of 1
     0,      0,      0,      0,      0,      0,      0,      0,
     0,      0,      0,      0,      0,      0,      0,      0,
@@ -169,7 +169,7 @@ s16 gSquareWaveSample[] = {
     0,      0,      32767,  32767,  0,      0,      -32767, -32767,
 };
 
-s16 gWhiteNoiseSample[] = {
+ALIGNED(16) s16 gWhiteNoiseSample[] = {
     // Frequency of 1
     0,      -25689, -25791, 27803,  -27568, -21030, 22174,  6298,
     27071,  -18531, 28649,  2284,   3380,   6890,   -12682, -21114,
@@ -212,7 +212,7 @@ s16 gWhiteNoiseSample[] = {
 };
 
 // Sine White Noise?
-s16 D_8012EA90[] = {
+ALIGNED(16) s16 D_8012EA90[] = {
     // Frequency of 1
     0,      16316,  20148,  20257,  27209,  -32657, 29264,  27259,
     -29394, -21494, -26410, 30770,  30033,  29130,  20206,  14129,
@@ -255,7 +255,7 @@ s16 D_8012EA90[] = {
 };
 
 // Pulse Wave (duty cycle = 12.5%)
-s16 gEighthPulseWaveSample[] = {
+ALIGNED(16) s16 gEighthPulseWaveSample[] = {
     // Frequency of 1
     0,      0,      0,      0,      0,      0, 0,      0,
     0,      0,      0,      0,      0,      0, 0,      0,
@@ -298,7 +298,7 @@ s16 gEighthPulseWaveSample[] = {
 };
 
 // Pulse Wave (duty cycle = 25%)
-s16 gQuarterPulseWaveSample[] = {
+ALIGNED(16) s16 gQuarterPulseWaveSample[] = {
     // Frequency of 1
     0,      0,      0,      0,      0,      0,      0,      0,
     0,      0,      0,      0,      0,      0,      0,      0,
@@ -686,7 +686,7 @@ f32 gDefaultPanVolume[] = {
     0.086471f, 0.074143f, 0.061803f, 0.049454f, 0.037097f, 0.024734f, 0.012368f, 0.0f,
 };
 
-s16 gLowPassFilterData[16 * 8] = {
+ALIGNED(16) s16 gLowPassFilterData[16 * 8] = {
     /* 0x0 */ 0,     0,     0,    32767, 0,    0,     0,     0, // Identity filter (delta function)
     /* 0x1 */ 3854,  4188,  4398, 4469,  4398, 4188,  3854,  3416,
     /* 0x2 */ 3415,  4314,  4915, 5126,  4915, 4314,  3415,  2351,
@@ -705,7 +705,7 @@ s16 gLowPassFilterData[16 * 8] = {
     /* 0xF */ 841,   -853,  863,  26829, 863,  -853,  841,   -820,
 };
 
-s16 gHighPassFilterData[15 * 8] = {
+ALIGNED(16) s16 gHighPassFilterData[15 * 8] = {
     /* 0x0 */ -289,  -291,  -289,  30736, -289,  -291,  -289,  -290,
     /* 0x1 */ -464,  -467,  -467,  29506, -467,  -467,  -464,  -463,
     /* 0x2 */ -662,  -670,  -672,  28101, -672,  -670,  -662,  -656,
@@ -723,7 +723,7 @@ s16 gHighPassFilterData[15 * 8] = {
     /* 0xE */ -772,  -3,    -6985, 17240, -6985, -3,    -772,  -3,
 };
 
-s16 D_80130418[8 * 8] = {
+ALIGNED(16) s16 D_80130418[8 * 8] = {
     /* 0x0 */ 0,      6392,   12539,  18204,  23169,  27244,  30272,  32137,
     /* 0x1 */ 32767,  32137,  30272,  27244,  23169,  18204,  12539,  6392,
     /* 0x2 */ 0,      -3211,  -6392,  -9511,  -12539, -15446, -18204, -20787,

src/boot/idle.c

@@ -46,7 +46,8 @@ void Idle_ThreadEntry(void* arg) {
     PRINTF("ダイナミックバッファのサイズは %d キロバイトです\n", 0x92);
     PRINTF("ＦＩＦＯバッファのサイズは %d キロバイトです\n", 0x60);
     PRINTF("ＹＩＥＬＤバッファのサイズは %d キロバイトです\n", 3);
-    PRINTF("オーディオヒープのサイズは %d キロバイトです\n", ((intptr_t)gSystemHeap - (intptr_t)gAudioHeap) / 1024);
+    PRINTF("オーディオヒープのサイズは %d キロバイトです\n",
+           ((intptr_t)&gAudioHeap[ARRAY_COUNT(gAudioHeap)] - (intptr_t)gAudioHeap) / 1024);
     PRINTF(VT_RST);
 
     osCreateViManager(OS_PRIORITY_VIMGR);

src/boot/yaz0.c

@@ -1,6 +1,6 @@
 #include "global.h"
 
-u8 sYaz0DataBuffer[0x400];
+ALIGNED(16) u8 sYaz0DataBuffer[0x400];
 u8* sYaz0DataBufferEnd;
 uintptr_t sYaz0CurRomStart;
 size_t sYaz0CurSize;

src/buffers/audio_heap.c

@@ -0,0 +1,3 @@
+#include "z64.h"
+
+ALIGNED(16) u8 gAudioHeap[0x38000];

src/buffers/gfxbuffers.c

@@ -1,13 +1,9 @@
 #include "z64.h"
 
-// 0x18000 bytes
+ALIGNED(16) u64 gGfxSPTaskOutputBuffer[0x3000];
-u64 gGfxSPTaskOutputBuffer[0x3000];
 
-// 0xC00 bytes
+ALIGNED(16) u64 gGfxSPTaskYieldBuffer[OS_YIELD_DATA_SIZE / sizeof(u64)];
-u64 gGfxSPTaskYieldBuffer[OS_YIELD_DATA_SIZE / sizeof(u64)];
 
-// 0x400 bytes
+ALIGNED(16) u64 gGfxSPTaskStack[SP_DRAM_STACK_SIZE64];
-u64 gGfxSPTaskStack[SP_DRAM_STACK_SIZE64];
 
-// 0x12410 bytes each; 0x24820 bytes total
+ALIGNED(16) GfxPool gGfxPools[2];
-GfxPool gGfxPools[2];

src/buffers/heaps.c

@@ -1,6 +0,0 @@
-#include "z64.h"
-
-// 0x38000 bytes
-u8 gAudioHeap[0x38000];
-
-u8 gSystemHeap[UNK_SIZE];

src/buffers/zbuffer.c

@@ -1,4 +1,3 @@
 #include "z64.h"
 
-// 0x25800 bytes
+ALIGNED(64) u16 gZBuffer[SCREEN_HEIGHT][SCREEN_WIDTH];
-u16 gZBuffer[SCREEN_HEIGHT][SCREEN_WIDTH];

src/code/main.c

@@ -1,6 +1,8 @@
 #include "global.h"
 #include "terminal.h"
 
+extern u8 _buffersSegmentEnd[];
+
 s32 gScreenWidth = SCREEN_WIDTH;
 s32 gScreenHeight = SCREEN_HEIGHT;
 u32 gSystemHeapSize = 0;
@@ -29,7 +31,8 @@ OSMesg sSerialMsgBuf[1];
 void Main_LogSystemHeap(void) {
     PRINTF(VT_FGCOL(GREEN));
     // "System heap size% 08x (% dKB) Start address% 08x"
-    PRINTF("システムヒープサイズ %08x(%dKB) 開始アドレス %08x\n", gSystemHeapSize, gSystemHeapSize / 1024, gSystemHeap);
+    PRINTF("システムヒープサイズ %08x(%dKB) 開始アドレス %08x\n", gSystemHeapSize, gSystemHeapSize / 1024,
+           _buffersSegmentEnd);
     PRINTF(VT_RST);
 }
 #endif
@@ -48,7 +51,7 @@ void Main(void* arg) {
     PreNmiBuff_Init(gAppNmiBufferPtr);
     Fault_Init();
     SysCfb_Init(0);
-    systemHeapStart = (uintptr_t)gSystemHeap;
+    systemHeapStart = (uintptr_t)_buffersSegmentEnd;
     fb = (uintptr_t)SysCfb_GetFbPtr(0);
     gSystemHeapSize = fb - systemHeapStart;
     // "System heap initalization"

src/code/z_common_data.c

@@ -1,7 +1,6 @@
 #include "global.h"
 
-// The use of ALIGNED8 here is just a temporary solution until the SaveContext is re-structured
+ALIGNED(16) SaveContext gSaveContext;
-ALIGNED8 SaveContext gSaveContext;
 u32 D_8015FA88;
 u32 D_8015FA8C;
 

tools/disasm/gc-eu-mq/variables.txt

@@ -722,7 +722,7 @@ gGfxSPTaskYieldBuffer = 0x80168000; // size:0xC00
 gGfxSPTaskStack = 0x80168C00; // size:0x400
 gGfxPools = 0x80169000; // size:0x24820
 gAudioHeap = 0x8018D820; // size:0x38000
-gSystemHeap = 0x801C5820; //
+_buffersSegmentEnd = 0x801C5820; //
 D_80811BB0 = 0x80810D70; // size:0x180
 D_80811D30 = 0x80810EF0; // size:0x100
 D_80811E30 = 0x80810FF0; // size:0x100