Tanmatsu: drop the C23 workaround and put frequency scaling back - #5
Merged
Conversation
include/tic80.h declares the counter and freq callbacks as u64 (*)(),
while tic.c defines tic80_tick taking CounterCallback and FreqCallback,
which are u64 (*)(void*), and core.c calls them with an argument:
core->data->counter(core->data->data)
Under C17 and earlier an empty parameter list means "unspecified", so the
mismatch is hidden. C23 gives () the same meaning as (void), and the
declaration and definition become incompatible types, so building with a
C23 compiler fails:
error: conflicting types for 'tic80_tick'
The studio already implements these correctly, in run.c, taking void* and
receiving .data. The two standalone callers did not, and worked only
because the callee ignored the extra argument.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
studio->net is read where either BUILD_EDITORS or BUILD_SURF is set:
initConsole(studio->console, studio, studio->fs, studio->net, ...)
but the field is declared only under BUILD_SURF, so compiling the studio
with BUILD_EDITORS and without BUILD_SURF fails:
error: 'Studio' has no member named 'net'
CMakeLists.txt turns BUILD_SURF on whenever BUILD_EDITORS is set, so the
in-tree builds never produce that combination; it is out-of-tree builds
compiling these sources directly that run into it.
The declaration now carries the same condition as the use, matching the
Console field just above it. No configuration that builds today changes:
with BUILD_SURF defined the preprocessed result is identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two fixes cherry-picked from the upstream branches make the C23 pin unnecessary: tic80_tick is now declared with the arguments it is defined and called with, so an empty parameter list meaning "(void)" no longer breaks the build. Also cherry-picked the Studio.net guard, which is what let this port compile the editors at all and had been carried as a local workaround. Both are proposed upstream as nesbox#2981 and nesbox#2982; carrying them here means not waiting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It was turned off while chasing frame rate, on the theory that it would
clock down under exactly the load that mattered. Measured against the
same demo cart, scene for scene, that theory does not hold:
light scene 11.09 ms -> 11.38 ms
heaviest scene 56.6 ms -> 56.6 ms
present 4.59 ms -> 4.63 ms
All within the spread between runs, and the frame rates are the same. The
main loop is busy for about 15.7 ms of every 16.7, so there is very
little idle for scaling to take, and what there is comes when the loop is
already ahead of the codec.
So the saving is free, and this is a battery powered handheld that would
otherwise sit at full clock for as long as it is switched on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cleanup, four commits, no new features.
Carry the two upstream fixes here
Cherry-picked from the branches proposed as nesbox#2981 and nesbox#2982, so this fork does not have to wait on review:
tic80_tickis declared with the arguments it is defined and called with. Under C23 an empty parameter list means(void), which made the declaration and definition incompatible types.Studio'snetfield is declared under the same condition it is used, which is what lets this port build the editors at all.Drop the
-std=gnu17pinThat pin existed only to hide the first of those. With the fix in place the component builds at the toolchain's default C23, and the binary comes out marginally smaller.
Put dynamic frequency scaling back on
It was turned off while chasing frame rate, on the theory that it would clock down under exactly the load that mattered. Measured against the same demo cart, scene for scene, that theory does not hold:
All inside the spread seen between runs, with the same frame rates. The main loop is busy for about 15.7 ms of every 16.7, so there is little idle to scale into, and what there is happens when the loop is already ahead of the codec.
The saving is therefore free, and this is a battery powered handheld that would otherwise sit at full clock for as long as it is switched on.