From f5fd8daffb02bafff5869f181a13da48b0f3ae0e Mon Sep 17 00:00:00 2001 From: cadmic Date: Mon, 19 Aug 2024 09:16:04 -0700 Subject: [PATCH] Use incremental link for z_message/z_game_over data shenanigans (#2051) Co-authored-by: Tharo <17233964+Thar0@users.noreply.github.com> --- Makefile | 9 ++++- linker_scripts/audio_table_rodata.ld | 17 -------- linker_scripts/data_with_rodata.ld | 22 +++++++++++ spec | 10 ++--- tools/mkldscript.c | 59 +++++++++++----------------- tools/spec.c | 9 +---- tools/spec.h | 4 -- 7 files changed, 56 insertions(+), 74 deletions(-) delete mode 100644 linker_scripts/audio_table_rodata.ld create mode 100644 linker_scripts/data_with_rodata.ld diff --git a/Makefile b/Makefile index 772f45fb9a..72b1509f91 100644 --- a/Makefile +++ b/Makefile @@ -361,7 +361,8 @@ O_FILES := $(foreach f,$(S_FILES:.s=.o),$(BUILD_DIR)/$f) \ $(foreach f,$(SRC_C_FILES:.c=.o),$(BUILD_DIR)/$f) \ $(foreach f,$(ASSET_C_FILES_EXTRACTED:.c=.o),$(f:$(EXTRACTED_DIR)/%=$(BUILD_DIR)/%)) \ $(foreach f,$(ASSET_C_FILES_COMMITTED:.c=.o),$(BUILD_DIR)/$f) \ - $(foreach f,$(BASEROM_BIN_FILES),$(BUILD_DIR)/baserom/$(notdir $f).o) + $(foreach f,$(BASEROM_BIN_FILES),$(BUILD_DIR)/baserom/$(notdir $f).o) \ + $(BUILD_DIR)/src/code/z_message_z_game_over.o OVL_RELOC_FILES := $(shell $(CPP) $(CPPFLAGS) $(SPEC) | $(BUILD_DIR_REPLACE) | grep -o '[^"]*_reloc.o' ) @@ -634,6 +635,10 @@ $(BUILD_DIR)/assets/%.o: $(EXTRACTED_DIR)/assets/%.c $(BUILD_DIR)/src/%.o: src/%.s $(CPP) $(CPPFLAGS) -Iinclude $< | $(AS) $(ASFLAGS) -o $@ +# Incremental link to move z_message and z_game_over data into rodata +$(BUILD_DIR)/src/code/z_message_z_game_over.o: $(BUILD_DIR)/src/code/z_message.o $(BUILD_DIR)/src/code/z_game_over.o + $(LD) -r -T linker_scripts/data_with_rodata.ld -o $@ $^ + $(BUILD_DIR)/dmadata_table_spec.h $(BUILD_DIR)/compress_ranges.txt: $(BUILD_DIR)/$(SPEC) $(MKDMADATA) $< $(BUILD_DIR)/dmadata_table_spec.h $(BUILD_DIR)/compress_ranges.txt @@ -758,7 +763,7 @@ ifneq ($(RUN_CC_CHECK),0) $(CC_CHECK) $< endif $(CC) -c $(CFLAGS) $(MIPS_VERSION) $(OPTFLAGS) -o $(@:.o=.tmp) $< - $(LD) -r -T linker_scripts/audio_table_rodata.ld $(@:.o=.tmp) -o $@ + $(LD) -r -T linker_scripts/data_with_rodata.ld $(@:.o=.tmp) -o $@ @$(RM) $(@:.o=.tmp) -include $(DEP_FILES) diff --git a/linker_scripts/audio_table_rodata.ld b/linker_scripts/audio_table_rodata.ld deleted file mode 100644 index 27eab49fc4..0000000000 --- a/linker_scripts/audio_table_rodata.ld +++ /dev/null @@ -1,17 +0,0 @@ -OUTPUT_ARCH (mips) - -/* Audio Table Linker Script, maps data into rodata */ - -SECTIONS { - - .rodata : - { - *(.data*) - *(.rodata*) - } - - /DISCARD/ : - { - *(*); - } -} diff --git a/linker_scripts/data_with_rodata.ld b/linker_scripts/data_with_rodata.ld new file mode 100644 index 0000000000..8d508ee18b --- /dev/null +++ b/linker_scripts/data_with_rodata.ld @@ -0,0 +1,22 @@ +OUTPUT_ARCH (mips) + +/* Maps data into rodata, used for audio tables and z_message/z_game_over */ + +SECTIONS { + .rodata : + { + *(.data) + *(.rodata) + *(.rodata.str*) + *(.rodata.cst*) + } + + /DISCARD/ : + { + /* GNU ld assumes that the linker script always combines .gptab.data and + * .gptab.sdata into .gptab.sdata, and likewise for .gptab.bss and .gptab.sbss. + * To avoid dealing with this, we just discard all .gptab sections. + */ + *(.gptab.*) + } +} diff --git a/spec b/spec index 2edafc4fb2..9d2bc053af 100644 --- a/spec +++ b/spec @@ -641,12 +641,10 @@ beginseg include "$(BUILD_DIR)/src/code/fmodf.o" include "$(BUILD_DIR)/src/code/__osMemset.o" include "$(BUILD_DIR)/src/code/__osMemmove.o" - // For some reason, the data sections of these files are placed here near the - // rodata sections of the other files - include_data_only_within_rodata "$(BUILD_DIR)/src/code/z_message.o" - include_data_only_within_rodata "$(BUILD_DIR)/src/code/z_game_over.o" - include_no_data "$(BUILD_DIR)/src/code/z_message.o" - include_no_data "$(BUILD_DIR)/src/code/z_game_over.o" + // For some reason, the data sections of z_message and z_game_over are + // placed near the rodata sections of other files, so we first build this + // combined object before the final link. + include "$(BUILD_DIR)/src/code/z_message_z_game_over.o" include "$(BUILD_DIR)/src/code/z_construct.o" include "$(BUILD_DIR)/data/audio_tables.rodata.o" include "$(BUILD_DIR)/src/audio/tables/samplebank_table.o" diff --git a/tools/mkldscript.c b/tools/mkldscript.c index 199c77eb74..586a684b92 100644 --- a/tools/mkldscript.c +++ b/tools/mkldscript.c @@ -64,13 +64,10 @@ static void write_ld_script(FILE *fout) for (j = 0; j < seg->includesCount; j++) { - if (!seg->includes[j].dataOnlyWithinRodata) - { - fprintf(fout, " %s (.text)\n", seg->includes[j].fpath); - if (seg->includes[j].linkerPadding != 0) - fprintf(fout, " . += 0x%X;\n", seg->includes[j].linkerPadding); - fprintf(fout, " . = ALIGN(0x10);\n"); - } + fprintf(fout, " %s (.text)\n", seg->includes[j].fpath); + if (seg->includes[j].linkerPadding != 0) + fprintf(fout, " . += 0x%X;\n", seg->includes[j].linkerPadding); + fprintf(fout, " . = ALIGN(0x10);\n"); } fprintf(fout, " _%sSegmentTextEnd = .;\n", seg->name); @@ -81,9 +78,8 @@ static void write_ld_script(FILE *fout) for (j = 0; j < seg->includesCount; j++) { - if (!seg->includes[j].dataOnlyWithinRodata && !seg->includes[j].noData) - fprintf(fout, " %s (.data)\n" - " . = ALIGN(0x10);\n", seg->includes[j].fpath); + fprintf(fout, " %s (.data)\n" + " . = ALIGN(0x10);\n", seg->includes[j].fpath); } fprintf(fout, " _%sSegmentDataEnd = .;\n", seg->name); @@ -94,10 +90,6 @@ static void write_ld_script(FILE *fout) for (j = 0; j < seg->includesCount; j++) { - if (seg->includes[j].dataOnlyWithinRodata) - fprintf(fout, " %s (.data)\n" - " . = ALIGN(0x10);\n", seg->includes[j].fpath); - // Compilers other than IDO, such as GCC, produce different sections such as // the ones named directly below. These sections do not contain values that // need relocating, but we need to ensure that the base .rodata section @@ -106,12 +98,11 @@ static void write_ld_script(FILE *fout) // the beginning of the entire rodata area in order to remain consistent. // Inconsistencies will lead to various .rodata reloc crashes as a result of // either missing relocs or wrong relocs. - if (!seg->includes[j].dataOnlyWithinRodata) - fprintf(fout, " %s (.rodata)\n" - " %s (.rodata.str*)\n" - " %s (.rodata.cst*)\n" - " . = ALIGN(0x10);\n", - seg->includes[j].fpath, seg->includes[j].fpath, seg->includes[j].fpath); + fprintf(fout, " %s (.rodata)\n" + " %s (.rodata.str*)\n" + " %s (.rodata.cst*)\n" + " . = ALIGN(0x10);\n", + seg->includes[j].fpath, seg->includes[j].fpath, seg->includes[j].fpath); } fprintf(fout, " _%sSegmentRoDataEnd = .;\n", seg->name); @@ -121,17 +112,15 @@ static void write_ld_script(FILE *fout) fprintf(fout, " _%sSegmentSDataStart = .;\n", seg->name); for (j = 0; j < seg->includesCount; j++) - if (!seg->includes[j].dataOnlyWithinRodata) - fprintf(fout, " %s (.sdata)\n" - " . = ALIGN(0x10);\n", seg->includes[j].fpath); + fprintf(fout, " %s (.sdata)\n" + " . = ALIGN(0x10);\n", seg->includes[j].fpath); fprintf(fout, " _%sSegmentSDataEnd = .;\n", seg->name); fprintf(fout, " _%sSegmentOvlStart = .;\n", seg->name); for (j = 0; j < seg->includesCount; j++) - if (!seg->includes[j].dataOnlyWithinRodata) - fprintf(fout, " %s (.ovl)\n", seg->includes[j].fpath); + fprintf(fout, " %s (.ovl)\n", seg->includes[j].fpath); fprintf(fout, " _%sSegmentOvlEnd = .;\n", seg->name); @@ -159,24 +148,20 @@ static void write_ld_script(FILE *fout) seg->name, seg->name, seg->name, seg->name); for (j = 0; j < seg->includesCount; j++) - if (!seg->includes[j].dataOnlyWithinRodata) - fprintf(fout, " %s (.sbss)\n" - " . = ALIGN(0x10);\n", seg->includes[j].fpath); + fprintf(fout, " %s (.sbss)\n" + " . = ALIGN(0x10);\n", seg->includes[j].fpath); for (j = 0; j < seg->includesCount; j++) - if (!seg->includes[j].dataOnlyWithinRodata) - fprintf(fout, " %s (.scommon)\n" - " . = ALIGN(0x10);\n", seg->includes[j].fpath); + fprintf(fout, " %s (.scommon)\n" + " . = ALIGN(0x10);\n", seg->includes[j].fpath); for (j = 0; j < seg->includesCount; j++) - if (!seg->includes[j].dataOnlyWithinRodata) - fprintf(fout, " %s (.bss)\n" - " . = ALIGN(0x10);\n", seg->includes[j].fpath); + fprintf(fout, " %s (.bss)\n" + " . = ALIGN(0x10);\n", seg->includes[j].fpath); for (j = 0; j < seg->includesCount; j++) - if (!seg->includes[j].dataOnlyWithinRodata) - fprintf(fout, " %s (COMMON)\n" - " . = ALIGN(0x10);\n", seg->includes[j].fpath); + fprintf(fout, " %s (COMMON)\n" + " . = ALIGN(0x10);\n", seg->includes[j].fpath); fprintf(fout, " . = ALIGN(0x10);\n" " _%sSegmentBssEnd = .;\n" diff --git a/tools/spec.c b/tools/spec.c index b13455867c..962cf4bb25 100644 --- a/tools/spec.c +++ b/tools/spec.c @@ -136,8 +136,6 @@ static const char *const stmtNames[] = [STMT_entry] = "entry", [STMT_flags] = "flags", [STMT_include] = "include", - [STMT_include_data_only_within_rodata] = "include_data_only_within_rodata", - [STMT_include_no_data] = "include_no_data", [STMT_name] = "name", [STMT_number] = "number", [STMT_romalign] = "romalign", @@ -159,8 +157,7 @@ STMTId get_stmt_id_by_stmt_name(const char *stmtName, int lineNum) { bool parse_segment_statement(struct Segment *currSeg, STMTId stmt, char* args, int lineNum) { // ensure no duplicates (except for 'include' or 'pad_text') - if (stmt != STMT_include && stmt != STMT_include_data_only_within_rodata && - stmt != STMT_include_no_data && stmt != STMT_pad_text && + if (stmt != STMT_include && stmt != STMT_pad_text && (currSeg->fields & (1 << stmt))) util_fatal_error("line %i: duplicate '%s' statement", lineNum, stmtNames[stmt]); @@ -213,8 +210,6 @@ bool parse_segment_statement(struct Segment *currSeg, STMTId stmt, char* args, i util_fatal_error("line %i: alignment is not a power of two", lineNum); break; case STMT_include: - case STMT_include_data_only_within_rodata: - case STMT_include_no_data: currSeg->includesCount++; currSeg->includes = realloc(currSeg->includes, currSeg->includesCount * sizeof(*currSeg->includes)); @@ -222,8 +217,6 @@ bool parse_segment_statement(struct Segment *currSeg, STMTId stmt, char* args, i util_fatal_error("line %i: invalid filename", lineNum); currSeg->includes[currSeg->includesCount - 1].linkerPadding = 0; - currSeg->includes[currSeg->includesCount - 1].dataOnlyWithinRodata = (stmt == STMT_include_data_only_within_rodata); - currSeg->includes[currSeg->includesCount - 1].noData = (stmt == STMT_include_no_data); break; case STMT_increment: if (!parse_number(args, &currSeg->increment)) diff --git a/tools/spec.h b/tools/spec.h index 6b01b34755..734b1ba8c3 100644 --- a/tools/spec.h +++ b/tools/spec.h @@ -14,8 +14,6 @@ typedef enum { STMT_entry, STMT_flags, STMT_include, - STMT_include_data_only_within_rodata, - STMT_include_no_data, STMT_name, STMT_number, STMT_romalign, @@ -35,8 +33,6 @@ enum { struct Include { char* fpath; int linkerPadding; - bool dataOnlyWithinRodata; - bool noData; }; typedef struct Segment {