From df5162edcac2c6ac6f93e8f7206b68acc98fe391 Mon Sep 17 00:00:00 2001 From: Rangi42 Date: Mon, 27 Oct 2025 12:05:27 -0400 Subject: [PATCH] Use loops instead of tail calls and `musttail` gcc 15.2.1 20250813 complains "address of automatic variable can escape to `musttail` call" from `-Wmaybe-musttail-local-addr`, and guaranteeing tail-call optimization cross-platform is more trouble than it's worth. --- include/platform.hpp | 9 --- src/asm/lexer.cpp | 67 +++++++++--------- src/link/assign.cpp | 160 +++++++++++++++++++++---------------------- 3 files changed, 112 insertions(+), 124 deletions(-) diff --git a/include/platform.hpp b/include/platform.hpp index 3a0ccfb4..5b64a572 100644 --- a/include/platform.hpp +++ b/include/platform.hpp @@ -60,13 +60,4 @@ #define _POSIX_C_SOURCE 200809L #endif -// gcc and clang have their own `musttail` attributes for tail recursion -#if defined(__clang__) && __has_cpp_attribute(clang::musttail) - #define MUSTTAIL [[clang::musttail]] -#elif defined(__GNUC__) && __has_cpp_attribute(gnu::musttail) - #define MUSTTAIL [[gnu::musttail]] -#else - #define MUSTTAIL -#endif - #endif // RGBDS_PLATFORM_HPP diff --git a/src/asm/lexer.cpp b/src/asm/lexer.cpp index 530b0d00..4889cede 100644 --- a/src/asm/lexer.cpp +++ b/src/asm/lexer.cpp @@ -716,45 +716,44 @@ int LexerState::peekCharAhead() { static std::pair> readInterpolation(size_t depth); static int peek() { - int c = lexerState->peekChar(); + for (;;) { + int c = lexerState->peekChar(); - if (lexerState->expansionScanDistance > 0) { - return c; - } - - ++lexerState->expansionScanDistance; // Do not consider again - - if (lexerState->disableExpansions) { - return c; - } else if (c == '\\') { - // If character is a backslash, check for a macro arg - ++lexerState->expansionScanDistance; - if (!isMacroChar(lexerState->peekCharAhead())) { + if (lexerState->expansionScanDistance > 0) { return c; } - // If character is a macro arg char, do macro arg expansion - shiftChar(); - if (std::shared_ptr str = readMacroArg(); str) { - beginExpansion(str, std::nullopt); + ++lexerState->expansionScanDistance; // Do not consider again - // Mark the entire macro arg expansion as "painted blue" - // so that macro args can't be recursive - // https://en.wikipedia.org/wiki/Painted_blue - lexerState->expansionScanDistance += str->length(); + if (lexerState->disableExpansions) { + return c; + } else if (c == '\\') { + // If character is a backslash, check for a macro arg + ++lexerState->expansionScanDistance; + if (!isMacroChar(lexerState->peekCharAhead())) { + return c; + } + // If character is a macro arg char, do macro arg expansion + shiftChar(); + if (std::shared_ptr str = readMacroArg(); str) { + beginExpansion(str, std::nullopt); + + // Mark the entire macro arg expansion as "painted blue" + // so that macro args can't be recursive + // https://en.wikipedia.org/wiki/Painted_blue + lexerState->expansionScanDistance += str->length(); + } + // Continue in the next iteration + } else if (c == '{') { + // If character is an open brace, do symbol interpolation + shiftChar(); + if (auto interp = readInterpolation(0); interp.first && interp.second) { + beginExpansion(interp.second, interp.first->name); + } + // Continue in the next iteration + } else { + return c; } - - MUSTTAIL return peek(); - } else if (c == '{') { - // If character is an open brace, do symbol interpolation - shiftChar(); - if (auto interp = readInterpolation(0); interp.first && interp.second) { - beginExpansion(interp.second, interp.first->name); - } - - MUSTTAIL return peek(); - } else { - return c; } } @@ -1943,8 +1942,6 @@ static Token yylex_NORMAL() { if (Symbol const *sym = sym_FindExactSymbol(std::get(token.value)); sym && sym->type == SYM_EQUS) { beginExpansion(sym->getEqus(), sym->name); - // We cannot do `MUSTTAIL return yylex_NORMAL();` because tail call optimization - // requires the return value to be "trivially destructible", and `Token` is not. continue; // Restart, reading from the new buffer } } diff --git a/src/link/assign.cpp b/src/link/assign.cpp index 1b964e0f..b99be3b3 100644 --- a/src/link/assign.cpp +++ b/src/link/assign.cpp @@ -15,7 +15,6 @@ #include "helpers.hpp" #include "itertools.hpp" #include "linkdefs.hpp" -#include "platform.hpp" // MUSTTAIL #include "verbosity.hpp" #include "link/main.hpp" @@ -119,92 +118,93 @@ static MemoryLocation getStartLocation(Section const §ion) { static std::optional getPlacement(Section const §ion, MemoryLocation &location) { SectionTypeInfo const &typeInfo = sectionTypeInfo[section.type]; - // Switch to the beginning of the next bank - std::deque &bankMem = memory[section.type][location.bank - typeInfo.firstBank]; - size_t spaceIdx = 0; + for (;;) { + // Switch to the beginning of the next bank + std::deque &bankMem = memory[section.type][location.bank - typeInfo.firstBank]; + size_t spaceIdx = 0; - if (spaceIdx < bankMem.size()) { - location.address = bankMem[spaceIdx].address; - } - - // Process locations in that bank - while (spaceIdx < bankMem.size()) { - // If that location is OK, return it - if (isLocationSuitable(section, bankMem[spaceIdx], location)) { - return spaceIdx; - } - - // Go to the next *possible* location - if (section.isAddressFixed) { - // If the address is fixed, there can be only one candidate block per bank; - // if we already reached it, give up. - if (location.address < section.org) { - location.address = section.org; - } else { - break; // Try again in next bank - } - } else if (section.isAlignFixed) { - // Move to next aligned location - // Move back to alignment boundary - location.address -= section.alignOfs; - // Ensure we're there (e.g. on first check) - location.address &= ~section.alignMask; - // Go to next align boundary and add offset - location.address += section.alignMask + 1 + section.alignOfs; - } else if (++spaceIdx < bankMem.size()) { - // Any location is fine, so, next free block + if (spaceIdx < bankMem.size()) { location.address = bankMem[spaceIdx].address; } - // If that location is past the current block's end, - // go forwards until that is no longer the case. - while (spaceIdx < bankMem.size() - && location.address >= bankMem[spaceIdx].address + bankMem[spaceIdx].size) { - ++spaceIdx; + // Process locations in that bank + while (spaceIdx < bankMem.size()) { + // If that location is OK, return it + if (isLocationSuitable(section, bankMem[spaceIdx], location)) { + return spaceIdx; + } + + // Go to the next *possible* location + if (section.isAddressFixed) { + // If the address is fixed, there can be only one candidate block per bank; + // if we already reached it, give up and try again in the next bank. + if (location.address >= section.org) { + break; + } + location.address = section.org; + } else if (section.isAlignFixed) { + // Move to next aligned location + // Move back to alignment boundary + location.address -= section.alignOfs; + // Ensure we're there (e.g. on first check) + location.address &= ~section.alignMask; + // Go to next align boundary and add offset + location.address += section.alignMask + 1 + section.alignOfs; + } else if (++spaceIdx < bankMem.size()) { + // Any location is fine, so, next free block + location.address = bankMem[spaceIdx].address; + } + + // If that location is past the current block's end, + // go forwards until that is no longer the case. + while (spaceIdx < bankMem.size() + && location.address >= bankMem[spaceIdx].address + bankMem[spaceIdx].size) { + ++spaceIdx; + } + + // Try again with the new location/free space combo } - // 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 (options.scrambleROMX && section.type == SECTTYPE_ROMX + && location.bank <= options.scrambleROMX) { + if (location.bank > typeInfo.firstBank) { + --location.bank; + } else if (options.scrambleROMX < typeInfo.lastBank) { + location.bank = options.scrambleROMX + 1; + } else { + return std::nullopt; + } + } else if (options.scrambleWRAMX && section.type == SECTTYPE_WRAMX + && location.bank <= options.scrambleWRAMX) { + if (location.bank > typeInfo.firstBank) { + --location.bank; + } else if (options.scrambleWRAMX < typeInfo.lastBank) { + location.bank = options.scrambleWRAMX + 1; + } else { + return std::nullopt; + } + } else if (options.scrambleSRAM && section.type == SECTTYPE_SRAM + && location.bank <= options.scrambleSRAM) { + if (location.bank > typeInfo.firstBank) { + --location.bank; + } else if (options.scrambleSRAM < typeInfo.lastBank) { + location.bank = options.scrambleSRAM + 1; + } else { + return std::nullopt; + } + } else if (location.bank < typeInfo.lastBank) { + ++location.bank; + } else { + return std::nullopt; + } + + // Try again in the next iteration. } - - // 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 (options.scrambleROMX && section.type == SECTTYPE_ROMX - && location.bank <= options.scrambleROMX) { - if (location.bank > typeInfo.firstBank) { - --location.bank; - } else if (options.scrambleROMX < typeInfo.lastBank) { - location.bank = options.scrambleROMX + 1; - } else { - return std::nullopt; - } - } else if (options.scrambleWRAMX && section.type == SECTTYPE_WRAMX - && location.bank <= options.scrambleWRAMX) { - if (location.bank > typeInfo.firstBank) { - --location.bank; - } else if (options.scrambleWRAMX < typeInfo.lastBank) { - location.bank = options.scrambleWRAMX + 1; - } else { - return std::nullopt; - } - } else if (options.scrambleSRAM && section.type == SECTTYPE_SRAM - && location.bank <= options.scrambleSRAM) { - if (location.bank > typeInfo.firstBank) { - --location.bank; - } else if (options.scrambleSRAM < typeInfo.lastBank) { - location.bank = options.scrambleSRAM + 1; - } else { - return std::nullopt; - } - } else if (location.bank < typeInfo.lastBank) { - ++location.bank; - } else { - return std::nullopt; - } - - MUSTTAIL return getPlacement(section, location); } static std::string getSectionDescription(Section const §ion) {