Skip to content

Commit df5162e

Browse files
Rangi42Rangi42
committed
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.
1 parent 2519d1e commit df5162e

File tree

3 files changed

+105
-117
lines changed

3 files changed

+105
-117
lines changed

‎include/platform.hpp‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,4 @@
6060
#define _POSIX_C_SOURCE 200809L
6161
#endif
6262

63-
// gcc and clang have their own `musttail` attributes for tail recursion
64-
#if defined(__clang__) && __has_cpp_attribute(clang::musttail)
65-
#define MUSTTAIL [[clang::musttail]]
66-
#elif defined(__GNUC__) && __has_cpp_attribute(gnu::musttail)
67-
#define MUSTTAIL [[gnu::musttail]]
68-
#else
69-
#define MUSTTAIL
70-
#endif
71-
7263
#endif // RGBDS_PLATFORM_HPP

‎src/asm/lexer.cpp‎

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -716,45 +716,44 @@ int LexerState::peekCharAhead() {
716716
static std::pair<Symbol const *, std::shared_ptr<std::string>> readInterpolation(size_t depth);
717717

718718
static int peek() {
719-
int c = lexerState->peekChar();
720-
721-
if (lexerState->expansionScanDistance > 0) {
722-
return c;
723-
}
724-
725-
++lexerState->expansionScanDistance; // Do not consider again
719+
for (;;) {
720+
int c = lexerState->peekChar();
726721

727-
if (lexerState->disableExpansions) {
728-
return c;
729-
} else if (c == '\\') {
730-
// If character is a backslash, check for a macro arg
731-
++lexerState->expansionScanDistance;
732-
if (!isMacroChar(lexerState->peekCharAhead())) {
722+
if (lexerState->expansionScanDistance > 0) {
733723
return c;
734724
}
735725

736-
// If character is a macro arg char, do macro arg expansion
737-
shiftChar();
738-
if (std::shared_ptr<std::string> str = readMacroArg(); str) {
739-
beginExpansion(str, std::nullopt);
726+
++lexerState->expansionScanDistance; // Do not consider again
740727

741-
// Mark the entire macro arg expansion as "painted blue"
742-
// so that macro args can't be recursive
743-
// https://en.wikipedia.org/wiki/Painted_blue
744-
lexerState->expansionScanDistance += str->length();
745-
}
728+
if (lexerState->disableExpansions) {
729+
return c;
730+
} else if (c == '\\') {
731+
// If character is a backslash, check for a macro arg
732+
++lexerState->expansionScanDistance;
733+
if (!isMacroChar(lexerState->peekCharAhead())) {
734+
return c;
735+
}
736+
// If character is a macro arg char, do macro arg expansion
737+
shiftChar();
738+
if (std::shared_ptr<std::string> str = readMacroArg(); str) {
739+
beginExpansion(str, std::nullopt);
746740

747-
MUSTTAIL return peek();
748-
} else if (c == '{') {
749-
// If character is an open brace, do symbol interpolation
750-
shiftChar();
751-
if (auto interp = readInterpolation(0); interp.first && interp.second) {
752-
beginExpansion(interp.second, interp.first->name);
741+
// Mark the entire macro arg expansion as "painted blue"
742+
// so that macro args can't be recursive
743+
// https://en.wikipedia.org/wiki/Painted_blue
744+
lexerState->expansionScanDistance += str->length();
745+
}
746+
// Continue in the next iteration
747+
} else if (c == '{') {
748+
// If character is an open brace, do symbol interpolation
749+
shiftChar();
750+
if (auto interp = readInterpolation(0); interp.first && interp.second) {
751+
beginExpansion(interp.second, interp.first->name);
752+
}
753+
// Continue in the next iteration
754+
} else {
755+
return c;
753756
}
754-
755-
MUSTTAIL return peek();
756-
} else {
757-
return c;
758757
}
759758
}
760759

@@ -1943,8 +1942,6 @@ static Token yylex_NORMAL() {
19431942
if (Symbol const *sym = sym_FindExactSymbol(std::get<std::string>(token.value));
19441943
sym && sym->type == SYM_EQUS) {
19451944
beginExpansion(sym->getEqus(), sym->name);
1946-
// We cannot do `MUSTTAIL return yylex_NORMAL();` because tail call optimization
1947-
// requires the return value to be "trivially destructible", and `Token` is not.
19481945
continue; // Restart, reading from the new buffer
19491946
}
19501947
}

‎src/link/assign.cpp‎

Lines changed: 74 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include "helpers.hpp"
1616
#include "itertools.hpp"
1717
#include "linkdefs.hpp"
18-
#include "platform.hpp" // MUSTTAIL
1918
#include "verbosity.hpp"
2019

2120
#include "link/main.hpp"
@@ -119,92 +118,93 @@ static MemoryLocation getStartLocation(Section const &section) {
119118
static std::optional<size_t> getPlacement(Section const &section, MemoryLocation &location) {
120119
SectionTypeInfo const &typeInfo = sectionTypeInfo[section.type];
121120

122-
// Switch to the beginning of the next bank
123-
std::deque<FreeSpace> &bankMem = memory[section.type][location.bank - typeInfo.firstBank];
124-
size_t spaceIdx = 0;
121+
for (;;) {
122+
// Switch to the beginning of the next bank
123+
std::deque<FreeSpace> &bankMem = memory[section.type][location.bank - typeInfo.firstBank];
124+
size_t spaceIdx = 0;
125125

126-
if (spaceIdx < bankMem.size()) {
127-
location.address = bankMem[spaceIdx].address;
128-
}
129-
130-
// Process locations in that bank
131-
while (spaceIdx < bankMem.size()) {
132-
// If that location is OK, return it
133-
if (isLocationSuitable(section, bankMem[spaceIdx], location)) {
134-
return spaceIdx;
126+
if (spaceIdx < bankMem.size()) {
127+
location.address = bankMem[spaceIdx].address;
135128
}
136129

137-
// Go to the next *possible* location
138-
if (section.isAddressFixed) {
139-
// If the address is fixed, there can be only one candidate block per bank;
140-
// if we already reached it, give up.
141-
if (location.address < section.org) {
142-
location.address = section.org;
143-
} else {
144-
break; // Try again in next bank
130+
// Process locations in that bank
131+
while (spaceIdx < bankMem.size()) {
132+
// If that location is OK, return it
133+
if (isLocationSuitable(section, bankMem[spaceIdx], location)) {
134+
return spaceIdx;
145135
}
146-
} else if (section.isAlignFixed) {
147-
// Move to next aligned location
148-
// Move back to alignment boundary
149-
location.address -= section.alignOfs;
150-
// Ensure we're there (e.g. on first check)
151-
location.address &= ~section.alignMask;
152-
// Go to next align boundary and add offset
153-
location.address += section.alignMask + 1 + section.alignOfs;
154-
} else if (++spaceIdx < bankMem.size()) {
155-
// Any location is fine, so, next free block
156-
location.address = bankMem[spaceIdx].address;
157-
}
158136

159-
// If that location is past the current block's end,
160-
// go forwards until that is no longer the case.
161-
while (spaceIdx < bankMem.size()
162-
&& location.address >= bankMem[spaceIdx].address + bankMem[spaceIdx].size) {
163-
++spaceIdx;
164-
}
137+
// Go to the next *possible* location
138+
if (section.isAddressFixed) {
139+
// If the address is fixed, there can be only one candidate block per bank;
140+
// if we already reached it, give up and try again in the next bank.
141+
if (location.address >= section.org) {
142+
break;
143+
}
144+
location.address = section.org;
145+
} else if (section.isAlignFixed) {
146+
// Move to next aligned location
147+
// Move back to alignment boundary
148+
location.address -= section.alignOfs;
149+
// Ensure we're there (e.g. on first check)
150+
location.address &= ~section.alignMask;
151+
// Go to next align boundary and add offset
152+
location.address += section.alignMask + 1 + section.alignOfs;
153+
} else if (++spaceIdx < bankMem.size()) {
154+
// Any location is fine, so, next free block
155+
location.address = bankMem[spaceIdx].address;
156+
}
165157

166-
// Try again with the new location/free space combo
167-
}
158+
// If that location is past the current block's end,
159+
// go forwards until that is no longer the case.
160+
while (spaceIdx < bankMem.size()
161+
&& location.address >= bankMem[spaceIdx].address + bankMem[spaceIdx].size) {
162+
++spaceIdx;
163+
}
168164

169-
// Try again in the next bank, if one is available.
170-
// Try scrambled banks in descending order until no bank in the scrambled range is
171-
// available. Otherwise, try in ascending order.
172-
if (section.isBankFixed) {
173-
return std::nullopt;
174-
} else if (options.scrambleROMX && section.type == SECTTYPE_ROMX
175-
&& location.bank <= options.scrambleROMX) {
176-
if (location.bank > typeInfo.firstBank) {
177-
--location.bank;
178-
} else if (options.scrambleROMX < typeInfo.lastBank) {
179-
location.bank = options.scrambleROMX + 1;
180-
} else {
181-
return std::nullopt;
165+
// Try again with the new location/free space combo
182166
}
183-
} else if (options.scrambleWRAMX && section.type == SECTTYPE_WRAMX
184-
&& location.bank <= options.scrambleWRAMX) {
185-
if (location.bank > typeInfo.firstBank) {
186-
--location.bank;
187-
} else if (options.scrambleWRAMX < typeInfo.lastBank) {
188-
location.bank = options.scrambleWRAMX + 1;
189-
} else {
167+
168+
// Try again in the next bank, if one is available.
169+
// Try scrambled banks in descending order until no bank in the scrambled range is
170+
// available. Otherwise, try in ascending order.
171+
if (section.isBankFixed) {
190172
return std::nullopt;
191-
}
192-
} else if (options.scrambleSRAM && section.type == SECTTYPE_SRAM
193-
&& location.bank <= options.scrambleSRAM) {
194-
if (location.bank > typeInfo.firstBank) {
195-
--location.bank;
196-
} else if (options.scrambleSRAM < typeInfo.lastBank) {
197-
location.bank = options.scrambleSRAM + 1;
173+
} else if (options.scrambleROMX && section.type == SECTTYPE_ROMX
174+
&& location.bank <= options.scrambleROMX) {
175+
if (location.bank > typeInfo.firstBank) {
176+
--location.bank;
177+
} else if (options.scrambleROMX < typeInfo.lastBank) {
178+
location.bank = options.scrambleROMX + 1;
179+
} else {
180+
return std::nullopt;
181+
}
182+
} else if (options.scrambleWRAMX && section.type == SECTTYPE_WRAMX
183+
&& location.bank <= options.scrambleWRAMX) {
184+
if (location.bank > typeInfo.firstBank) {
185+
--location.bank;
186+
} else if (options.scrambleWRAMX < typeInfo.lastBank) {
187+
location.bank = options.scrambleWRAMX + 1;
188+
} else {
189+
return std::nullopt;
190+
}
191+
} else if (options.scrambleSRAM && section.type == SECTTYPE_SRAM
192+
&& location.bank <= options.scrambleSRAM) {
193+
if (location.bank > typeInfo.firstBank) {
194+
--location.bank;
195+
} else if (options.scrambleSRAM < typeInfo.lastBank) {
196+
location.bank = options.scrambleSRAM + 1;
197+
} else {
198+
return std::nullopt;
199+
}
200+
} else if (location.bank < typeInfo.lastBank) {
201+
++location.bank;
198202
} else {
199203
return std::nullopt;
200204
}
201-
} else if (location.bank < typeInfo.lastBank) {
202-
++location.bank;
203-
} else {
204-
return std::nullopt;
205-
}
206205

207-
MUSTTAIL return getPlacement(section, location);
206+
// Try again in the next iteration.
207+
}
208208
}
209209

210210
static std::string getSectionDescription(Section const &section) {

0 commit comments

Comments
 (0)