diff --git a/src/link/assign.cpp b/src/link/assign.cpp index 683d64bf..dd45ae45 100644 --- a/src/link/assign.cpp +++ b/src/link/assign.cpp @@ -23,11 +23,6 @@ #include "link/symbol.hpp" #include "link/warning.hpp" -struct MemoryLocation { - uint16_t address; - uint32_t bank; -}; - struct FreeSpace { uint16_t address; uint16_t size; @@ -80,6 +75,71 @@ struct Scrambling { }; static Scrambling scrambling; +struct MemoryLocation { + uint16_t address; + uint32_t bank; + + static MemoryLocation initFor(Section const §ion) { + MemoryLocation location; + + if (section.isAddressFixed) { // This will never change. + location.address = section.org; + } + + if (section.isBankFixed) { + location.bank = section.bank; + } else { + location.bank = section.typeInfo().firstBank; + + if (auto info = scrambling.getInfoFor(section.type); + info.has_value() && info->maxOfs != 0) { // If scrambling is enabled... + // ...then we will begin the search from a different offset for each section. + // Go to the next offset (backwards), wrapping around. (Thus, no underflow!) + info->curOfs = (info->curOfs != 0 ? info->curOfs : info->maxOfs) - 1; + location.bank += info->curOfs; + } + } + + return location; + } + + // Try again in the next bank, if one is available. + [[nodiscard("This returns whether iteration can be continued")]] + bool goToNextApplicableBankFor(Section const §ion) { + assume(bank >= section.typeInfo().firstBank); + assume(bank <= section.typeInfo().lastBank); + + if (section.isBankFixed) { + // We have already tried the only possible bank. + return false; + } + + // Try scrambled banks in descending order until no bank in the scrambled range is + // available. + if (auto info = scrambling.getInfoFor(section.type); + info.has_value() && info->maxOfs != 0) { + // All floating sections within a scrambled region should be + // within the scrambled bank pool. + assume(bank < info->maxOfs + section.typeInfo().firstBank); + + uint16_t ofsWithinPool = bank - section.typeInfo().firstBank; + // Go to the next bank (backwards), wrapping around. (Thus, no overflow!) + ofsWithinPool = (ofsWithinPool != 0 ? ofsWithinPool : info->maxOfs) - 1; + + bank = ofsWithinPool + section.typeInfo().firstBank; + // Keep iterating unless we have wrapped back around to the start offset. + return ofsWithinPool != info->curOfs; + } + + // Otherwise, try in ascending order. + if (bank == section.typeInfo().lastBank) { + return false; + } + ++bank; + return true; + } +}; + // Checks whether a given location is suitable for placing a given section // This checks not only that the location has enough room for the section, but // also that the constraints (alignment...) are respected. @@ -101,31 +161,12 @@ static bool isLocationSuitable( return location.address + section.size <= freeSpace.address + freeSpace.size; } -static MemoryLocation getStartLocation(Section const §ion) { - MemoryLocation location; - - // Determine which bank we should start searching in - if (section.isBankFixed) { - location.bank = section.bank; - } else { - location.bank = section.typeInfo().firstBank; - - if (auto info = scrambling.getInfoFor(section.type); - info.has_value() && info->maxOfs != 0) { - info->curOfs = (info->curOfs != 0 ? info->curOfs : info->maxOfs) - 1; - location.bank += info->curOfs; - } - } - - return location; -} - // Returns a suitable free space index into `memory[section->type]` at which to place the given // section, or `std::nullopt` if none was found. static std::optional getPlacement(Section const §ion, MemoryLocation &location) { SectionTypeInfo const &typeInfo = section.typeInfo(); - for (;;) { + do { if (location.bank < typeInfo.firstBank || location.bank >= memory[section.type].size() + typeInfo.firstBank) { fatal( @@ -191,29 +232,9 @@ static std::optional getPlacement(Section const §ion, MemoryLocation // Try again with the new location/free space combo } - // Try again in the next bank, if one is available. - // Try scrambled banks in descending order until no bank in the scrambled range is - // available. Otherwise, try in ascending order. - if (section.isBankFixed) { - return std::nullopt; - } else if ( - auto info = scrambling.getInfoFor(section.type); info.has_value() && info->maxOfs != 0 - ) { - if (location.bank > typeInfo.firstBank) { - --location.bank; - } else if (info->maxOfs < typeInfo.lastBank) { - location.bank = info->maxOfs + 1; - } else { - return std::nullopt; - } - } else if (location.bank < typeInfo.lastBank) { - ++location.bank; - } else { - return std::nullopt; - } - // Try again in the next iteration. - } + } while (location.goToNextApplicableBankFor(section)); + return std::nullopt; } static std::string getSectionDescription(Section const §ion) { @@ -245,7 +266,15 @@ static std::string getSectionDescription(Section const §ion) { } else { description = description + "anywhere"; } + + if (auto info = scrambling.getInfoFor(section.type); + info.has_value() && info->maxOfs != 0) { // Only mention scrambling if it is enabled. + char size[6]; + snprintf(size, sizeof(size), "%" PRIu16, info->maxOfs); + description = description + " within the " + size + " scrambled banks"; + } } + return description; } @@ -263,16 +292,13 @@ static void assignSection(Section §ion, MemoryLocation const &location) { // Places a section in a suitable location, or error out if it fails to. // Due to the implemented algorithm, this should be called with sections of decreasing size! static void placeSection(Section §ion) { - SectionTypeInfo const &typeInfo = section.typeInfo(); + MemoryLocation location = MemoryLocation::initFor(section); // Specially handle 0-byte SECTIONs, as they can't overlap anything if (section.size == 0) { - // Unless the SECTION has a fixed address or non-zero alignment, the starting + // Unless the SECTION has a fixed address or non-zero alignment offset, the starting // address is fine for any alignment, as checked in `sect_DoSanityChecks`. - MemoryLocation location = { - .address = section.isAddressFixed ? section.org : typeInfo.startAddr, - .bank = section.isBankFixed ? section.bank : typeInfo.firstBank, - }; + location.address = section.isAddressFixed ? section.org : section.typeInfo().startAddr; if (section.isAlignFixed && !section.isAddressFixed) { if (uint16_t offset = (location.address - section.alignOfs) & section.alignMask; offset != 0) { @@ -285,9 +311,9 @@ static void placeSection(Section §ion) { // Place section using first-fit decreasing algorithm // https://en.wikipedia.org/wiki/Bin_packing_problem#First-fit_algorithm - MemoryLocation location = getStartLocation(section); if (std::optional spaceIdx = getPlacement(section, location); spaceIdx) { - std::deque &bankMem = memory[section.type][location.bank - typeInfo.firstBank]; + std::deque &bankMem = + memory[section.type][location.bank - section.typeInfo().firstBank]; FreeSpace &freeSpace = bankMem[*spaceIdx]; assignSection(section, location); @@ -324,9 +350,8 @@ static void placeSection(Section §ion) { if (!section.isBankFixed || !section.isAddressFixed) { // If a section failed to go to several places, nothing we can report fatal("Unable to place %s", getSectionDescription(section).c_str()); - } else if ( - uint16_t onePastEnd = typeInfo.endAddr() + 1; section.org + section.size > onePastEnd - ) { + } else if (uint16_t onePastEnd = section.typeInfo().endAddr() + 1; + section.org + section.size > onePastEnd) { // If the section just can't fit the bank, report that fatal( "Unable to place %s: section runs past end of region ($%04x > $%04x)", diff --git a/test/link/scramble-overflow.asm b/test/link/scramble-overflow.asm new file mode 100644 index 00000000..af066887 --- /dev/null +++ b/test/link/scramble-overflow.asm @@ -0,0 +1,15 @@ +DEF N equ 6 + +SECTION "fixed", ROMX, BANK[3] +ds $1000, $ff + +FOR i, N + SECTION "floating{d:i}", ROMX + xLabel{d:i}:: ds $2000, i + + SECTION "wram{d:i}", WRAMX + wLabel{d:i}:: dw + + SECTION "sram{d:i}", SRAM + sLabel{d:i}:: dw +ENDR diff --git a/test/link/scramble-overflow.flags b/test/link/scramble-overflow.flags new file mode 100644 index 00000000..8beff92c --- /dev/null +++ b/test/link/scramble-overflow.flags @@ -0,0 +1 @@ +-S romx=3,wramx=4,sram=4 diff --git a/test/link/scramble-overflow.out b/test/link/scramble-overflow.out new file mode 100644 index 00000000..8a7c6413 --- /dev/null +++ b/test/link/scramble-overflow.out @@ -0,0 +1,2 @@ +FATAL: Unable to place "floating0" (ROMX section) anywhere within the 3 scrambled banks +Linking aborted with 1 error diff --git a/test/link/scramble-specs.asm b/test/link/scramble-specs.asm new file mode 100644 index 00000000..412bcaab --- /dev/null +++ b/test/link/scramble-specs.asm @@ -0,0 +1,38 @@ +; XXX: This test is brittle, since it relies on more than just the scrambling algorithm. +; For example, if the order in which sections are processed changes, +; then they will also be scrambled differently, and this test will fail. +; As long as the actual values are coherent, feel free to change the assertions. + +DEF N equ 6 + +assert BANK(xLabel5) == 3 +assert BANK(xLabel4) == 2 +assert BANK(xLabel3) == 1 +assert BANK(xLabel2) == 3 +assert BANK(xLabel1) == 2 +assert BANK(xLabel0) == 1 + +assert BANK(sLabel5) == 3 +assert BANK(sLabel4) == 2 +assert BANK(sLabel3) == 1 +assert BANK(sLabel2) == 0 +assert BANK(sLabel1) == 3 +assert BANK(sLabel0) == 2 + +assert BANK(wLabel5) == 4 +assert BANK(wLabel4) == 3 +assert BANK(wLabel3) == 2 +assert BANK(wLabel2) == 1 +assert BANK(wLabel1) == 4 +assert BANK(wLabel0) == 3 + +FOR i, N + SECTION "floating{d:i}", ROMX + xLabel{d:i}:: ds $2000, i + + SECTION "sram{d:i}", SRAM + sLabel{d:i}:: dw + + SECTION "wram{d:i}", WRAMX + wLabel{d:i}:: dw +ENDR diff --git a/test/link/scramble-specs.flags b/test/link/scramble-specs.flags new file mode 100644 index 00000000..8beff92c --- /dev/null +++ b/test/link/scramble-specs.flags @@ -0,0 +1 @@ +-S romx=3,wramx=4,sram=4 diff --git a/test/link/scramble-specs/out.err b/test/link/scramble-specs.out similarity index 100% rename from test/link/scramble-specs/out.err rename to test/link/scramble-specs.out diff --git a/test/link/scramble-specs/a.asm b/test/link/scramble-specs/a.asm deleted file mode 100644 index 4fb38895..00000000 --- a/test/link/scramble-specs/a.asm +++ /dev/null @@ -1,25 +0,0 @@ -DEF N = 6 - -SECTION "fixed", ROMX, BANK[3] -; XXX: We rely on these landing at certain banks, which isn't *guaranteed*... -FOR i, 1, N + 1 - db BANK(xLabel{d:i}) -ENDR -FOR i, 1, N + 1 - db BANK(wLabel{d:i}) -ENDR -FOR i, 1, N + 1 - db BANK(sLabel{d:i}) -ENDR -ds $1000 - N * 3, $ff - -FOR i, 1, N + 1 - SECTION "floating{d:i}", ROMX - xLabel{d:i}:: ds $2000, i - - SECTION "wram{d:i}", WRAMX - wLabel{d:i}:: dw - - SECTION "sram{d:i}", SRAM - sLabel{d:i}:: dw -ENDR diff --git a/test/link/scramble-specs/out.gb b/test/link/scramble-specs/out.gb deleted file mode 100644 index ef9d9186..00000000 Binary files a/test/link/scramble-specs/out.gb and /dev/null differ diff --git a/test/link/test.sh b/test/link/test.sh index 63a0085b..50fd07b4 100755 --- a/test/link/test.sh +++ b/test/link/test.sh @@ -330,16 +330,6 @@ rgblinkQuiet -o "$gbtemp" -S "romx := 4" "$otemp" 2>"$outtemp" tryDiff "$test"/out.err "$outtemp" evaluateTest -test="scramble-specs" -startTest -"$RGBASM" -o "$otemp" "$test"/a.asm -continueTest -rgblinkQuiet -o "$gbtemp" -S "romx=3,wramx=4,sram=4" "$otemp" 2>"$outtemp" -tryDiff "$test"/out.err "$outtemp" -# This test does not trim its output with 'dd' because it needs to verify the correct output size -tryCmp "$test"/out.gb "$gbtemp" -evaluateTest - test="script-ds" startTest "$RGBASM" -o "$otemp" "$test"/a.asm