The bios_* symbols in include/ngdevkit/bios-ram.h mix two categories:
read-once-per-frame values (e.g. bios_mvs_flag, bios_country_code,
bios_game_dip[]) and state mutated asynchronously by the BIOS IRQ
(e.g. bios_p1current, bios_p1change, bios_frame_counter,
bios_vblank_sync, bios_statchange). The header doesn't distinguish
the two, and I couldn't find docs on the intended polling contract.
A user writing what looks like the natural poll shape for the second
category:
#include <ngdevkit/bios-ram.h>
void wait_for_input(void) {
while (!bios_p1change) { } /* spin until any P1 button changes */
}
compiles cleanly at -O2 and hangs forever - the load is legitimately
hoisted out of the loop since the compiler sees no observable mutation
between iterations. The issue dissolves at -O0, or by reading through
a per-use volatile cast (*(volatile u8 *)&bios_p1change), or by
intermixing some other volatile access in the loop body. Standard C
semantics; the compiler is doing the right thing for what the declaration
says.
I'm filing this as a question rather than a patch because the right
resolution depends on what you'd like the contract to be. A few shapes
come to mind:
- Document in the header (or a wiki page) that users polling these
symbols must declare them volatile themselves at the call site.
Smallest change; preserves the current ABI/API exactly.
- Mark only the IRQ-mutated subset
volatile in the declarations.
Makes the safe thing the default but changes the declared type for
every consumer.
- Leave as-is - polling these is already an unusual pattern, and most
real projects wait on REG_VBLANK / read the controllers from
*REG_P1CNT directly.
Happy to send a patch in whichever direction you prefer, or this could
land as a docs-only change. The motivation for filing is just that the
current behaviour is a footgun for newcomers - the bug manifests only
under optimisation and only with certain poll shapes, exactly when it's
hardest to attribute to the right cause.
The
bios_*symbols ininclude/ngdevkit/bios-ram.hmix two categories:read-once-per-frame values (e.g.
bios_mvs_flag,bios_country_code,bios_game_dip[]) and state mutated asynchronously by the BIOS IRQ(e.g.
bios_p1current,bios_p1change,bios_frame_counter,bios_vblank_sync,bios_statchange). The header doesn't distinguishthe two, and I couldn't find docs on the intended polling contract.
A user writing what looks like the natural poll shape for the second
category:
compiles cleanly at
-O2and hangs forever - the load is legitimatelyhoisted out of the loop since the compiler sees no observable mutation
between iterations. The issue dissolves at
-O0, or by reading througha per-use volatile cast (
*(volatile u8 *)&bios_p1change), or byintermixing some other volatile access in the loop body. Standard C
semantics; the compiler is doing the right thing for what the declaration
says.
I'm filing this as a question rather than a patch because the right
resolution depends on what you'd like the contract to be. A few shapes
come to mind:
symbols must declare them
volatilethemselves at the call site.Smallest change; preserves the current ABI/API exactly.
volatilein the declarations.Makes the safe thing the default but changes the declared type for
every consumer.
real projects wait on
REG_VBLANK/ read the controllers from*REG_P1CNTdirectly.Happy to send a patch in whichever direction you prefer, or this could
land as a docs-only change. The motivation for filing is just that the
current behaviour is a footgun for newcomers - the bug manifests only
under optimisation and only with certain poll shapes, exactly when it's
hardest to attribute to the right cause.