refactor label optimizer again (closes #383)

Author: trillllian

src/asar/assembleblock.cpp

@@ -761,14 +761,17 @@ static void resolve_pinned_freespaces() {
 		fs.pin_target_id = get_freespace_pin_target(value.freespace_id);
 		fs.len = 0;
 	}
+	for(int i = 1; i < freespaces.count; i++) {
+		freespace_data& fs = freespaces[i];
+		// just in case the pin target changed again or something
+		fs.pin_target_id = get_freespace_pin_target(fs.pin_target_id);
+	}
 }
 
 static void allocate_freespaces() {
 	// compute real size of all pinned freespace blocks
 	for(int i = 1; i < freespaces.count; i++) {
 		freespace_data& fs = freespaces[i];
-		// just in case the pin target changed again or something
-		fs.pin_target_id = get_freespace_pin_target(fs.pin_target_id);
 		freespace_data& target = freespaces[fs.pin_target_id];
 		target.total_len += fs.len;
 		target.search_start = std::max(fs.search_start, target.search_start);

src/asar/main.cpp

@@ -357,31 +357,40 @@ virtual_file_error asar_get_last_io_error()
 
 int getlenforlabel(int labelpos, int label_fs_id, bool exists)
 {
-	unsigned int bank = labelpos>>16;
+	int bank = labelpos>>16;
 	unsigned int word = labelpos&0xFFFF;
-	bool lblfreespace = label_fs_id > 0;
+	bool lbl_is_freespace = label_fs_id > 0;
+	// bank number for mirroring considerations.
+	// this is the number of a bank which has the same "layout" as the current bank.
 	unsigned int relaxed_bank;
+	// nonnegative if we know that DBR is pointing at a certain bank
+	int cur_effective_bank = -1;
 	if(optimizeforbank >= 0) {
-		relaxed_bank = optimizeforbank;
+		cur_effective_bank = relaxed_bank = optimizeforbank;
 	} else {
 		if(freespaceid == 0) {
-			relaxed_bank = snespos >> 16;
+			cur_effective_bank = relaxed_bank = snespos >> 16;
 		} else {
 			int target_bank = freespaces[freespaceid].bank;
 			if(target_bank == -2) relaxed_bank = 0;
 			else if(target_bank == -1) relaxed_bank = 0x40;
-			else relaxed_bank = target_bank;
+			else cur_effective_bank = relaxed_bank = target_bank;
 		}
 	}
+
+	if(lbl_is_freespace) {
+		bank = freespaces[label_fs_id].bank;
+	}
+
 	if (!exists)
 	{
 		return 2;
 	}
-	else if((optimize_dp == optimize_dp_flag::RAM) && bank == 0x7E && (word-dp_base < 0x100) && !lblfreespace)
+	else if((optimize_dp == optimize_dp_flag::RAM) && bank == 0x7E && (word-dp_base < 0x100) && !lbl_is_freespace)
 	{
 		return 1;
 	}
-	else if(optimize_dp == optimize_dp_flag::ALWAYS && (bank == 0x7E || !(bank & 0x40)) && (word-dp_base < 0x100) && !lblfreespace)
+	else if(optimize_dp == optimize_dp_flag::ALWAYS && (bank == 0x7E || !(bank & 0x40)) && (word-dp_base < 0x100) && !lbl_is_freespace)
 	{
 		return 1;
 	}
@@ -391,7 +400,7 @@ int getlenforlabel(int labelpos, int label_fs_id, bool exists)
 		// and we're in a bank with ram mirrors... (optimizeforbank=0x7E is checked later)
 		&& !(relaxed_bank & 0x40)
 		// and the label is in low RAM
-		&& bank == 0x7E && word < 0x2000 && !lblfreespace)
+		&& bank == 0x7E && word < 0x2000 && !lbl_is_freespace)
 	{
 		return 2;
 	}
@@ -401,27 +410,33 @@ int getlenforlabel(int labelpos, int label_fs_id, bool exists)
 		// we're in a bank with ram mirrors...
 		&& !(relaxed_bank & 0x40)
 		// and the label is in a mirrored section
-		&& !(bank & 0x40) && word < 0x8000 && !lblfreespace)
+		&& !(bank & 0x40) && word < 0x8000 && !lbl_is_freespace)
 	{
 		return 2;
 	}
 	else if (optimizeforbank>=0)
 	{
 		// if optimizing for a specific bank:
-		// if the label is in freespace, never optimize
-		if (lblfreespace) return 3;
-		else if (bank==(unsigned int)optimizeforbank) return 2;
+		// if the label is in freespace, then bank is the freespace's bank, and
+		// if that's non-negative, then the freespace is forced to that bank.
+		// if the freespace isn't forced to a specific bank, then bank is
+		// negative, so this equality will never hold.
+		if (bank == optimizeforbank) return 2;
 		else return 3;
 	}
-	else if (lblfreespace || freespaceid > 0)
-	{
-		// optimize only if the label is in the same freespace
-		// TODO: check whether they're pinned to the same bank
-		if (label_fs_id != freespaceid) return 3;
-		else return 2;
+
+	// check if the label is pinned to the current freespace.
+	// can only be checked after pass 0, as freespace pin targets aren't
+	// computed before then.
+	// This codepath also handles the case of labels that are in the current freespace.
+	if(pass > 0 && lbl_is_freespace && freespaceid > 0) {
+		int fs_pin_1 = freespaces[label_fs_id].pin_target_id;
+		int fs_pin_2 = freespaces[freespaceid].pin_target_id;
+		if(fs_pin_1 == fs_pin_2) return 2;
 	}
-	else if ((int)bank != snespos >> 16){ return 3; }
-	else { return 2;}
+
+	if (bank >= 0 && bank == cur_effective_bank) return 2;
+	else return 3;
 }
 int getlenforlabel(snes_label thislabel, bool exists) {
 	return getlenforlabel(thislabel.pos, thislabel.freespace_id, exists);

tests/optimizer.asm

@@ -1,27 +1,4 @@
 ;`+
-;`AD 00 00 AF 00 00 7E
-;`AD 00 00 A5 00
-;`A5 00
-;`AD 00 00 A5 00
-;`AF 00 10 01 AD 00 10
-;`AD 00 10
-;`AF 1D 80 00
-;`AF 1D 80 00
-;`AD 1D 80
-;`AD 00 80 AF 00 80 01
-;`
-;`80000 53 54 41 52 26 00 D9 FF
-;`  AD 08 80  AF 00 00 00  AF 00 60 02
-;`  AD 00 00  AF 00 10 01  AF 00 60 02
-;`  AD 00 00  AD 00 10  AD 00 60  AF 00 80 00
-;`  AF 37 80 90
-;`53 54 41 52 16 00 E9 FF
-;`  AF 08 80 90  AD 37 80  AF 00 00 00
-;`  AF 00 00 7E
-;`  AF 00 00 7E  AF 00 60 02
-;`FFFFF 00
-
-;1 line above = 1 block of code below
 ;namespace optimize_dp_flag {
 ;	enum : int {
 ;		NONE,	//don't optimize
@@ -78,75 +55,131 @@ base off
 optimize address default
 optimize dp none
 ;test default asar optimization (bank only)
+;`AD 00 00 AF 00 00 7E
 lda test_00.test
 lda test_7E.test
 
 ;test optimizations of dp on with dp base as default 0000 in bank 7E only
 ;optimize dp ram
 optimize dp ram
 dpbase $0000
+;`AD 00 00 A5 00
 lda test_00.test
 lda test_7E.test
 
 optimize dp always
+;`A5 00
 lda test_00.test
 
 dpbase $0100
+;`AD 00 00 A5 00
 lda test_00.test
 lda test_00.test_0100
 
 optimize dp none
 optimize address ram
+;`AF 00 10 01 AD 00 10
 lda test_word.up
 lda test_7E.word
 
 optimize address mirrors
+;`AD 00 10
 lda test_word.up
 
 bank noassume
 asdf:
+;`AF 1D 80 00
 lda asdf
 bank $10
+;`AF 1D 80 00
 lda asdf
 bank auto
+;`AD 1D 80
 lda asdf
 
+;`AD 00 80 AF 00 80 01
 lda nonmirror
 lda nonmirror01
 
 assert pc() <= $009000
 
+bank80label = $808080
+segment bank=$80
+;`AD 80 80
+lda bank80label
+banksegmentlabel:
+
+segment bank=$80
+;`AD 32 80
+lda banksegmentlabel
+;`AF 00 80 81
+lda bank1segmentlabel
+
+segment bank=$81
+bank1segmentlabel:
+;`08000 EA
+nop
+
+;`80000 53 54 41 52 26 00 D9 FF
 freecode cleaned
 optimize dp none
 optimize address default
 freec:
+;`  AD 08 80  AF 00 00 00  AF 00 60 02
 lda freec
 lda test_00.test
 lda mirror
 
 optimize address ram
+;`  AD 00 00  AF 00 10 01  AF 00 60 02
 lda test_7E.test
 lda test_word.up
 lda mirror
 
 optimize address mirrors
+;`  AD 00 00  AD 00 10  AD 00 60  AF 00 80 00
 lda test_7E.test
 lda test_word.up
 lda mirror
 lda nonmirror
 
+;`  AF 37 80 90
 lda freed
 
+;`53 54 41 52 16 00 E9 FF
 freedata cleaned
 freed:
 optimize address default
+;`  AF 08 80 90  AD 37 80  AF 00 00 00
 lda freec
 lda freed
 lda test_00.test
 
 optimize address ram
+;`  AF 00 00 7E
 lda test_7E.test
 
 optimize address mirrors
+;`  AF 00 00 7E  AF 00 60 02
 lda test_7E.test
 lda mirror
+
+segment
+pinnedseg:
+;`EA
+nop
+
+segment pin=pinnedseg
+;`AD 4E 80
+lda pinnedseg
+;`AF 5A 80 90
+lda unpinnedseg
+;`AF 00 80 81
+lda bank1segmentlabel
+
+segment
+unpinnedseg:
+;`EA
+nop
+
+;`FFFFF 00