Keep the Gekko float pipeline in the runtime helpers - #11
Conversation
The C backend inlines single- and double-precision floating point as plain C:
ctx->fpr[d] = (f64)(f32)(ctx->fpr[a] * ctx->fpr[c]);
That computes a plausible value and drops four things the hardware does, none
of which are visible in the generated source:
* ppc_fmuls runs its C operand through force_25bit_c first. The Gekko
truncates a multiply's C operand to a 25-bit mantissa, so an inlined f64
multiply is numerically wrong on every instruction, not just edge cases.
* fp_write_single writes ps1 as well as fpr. Single-precision results occupy
both halves of a paired-single register; the inline form leaves ps1 holding
whatever was there before, and the ps_* instructions go on to read it.
* FPRF, FI and FR in FPSCR are never updated.
* NaN and invalid-operation gating is skipped. emit_fcompare also dropped the
ordered flag entirely, so fcmpo and fcmpu compiled to identical code even
though only fcmpo signals on a NaN operand.
dolrecomp_f32_to_bits and dolrecomp_f32_from_bits had likewise become a cast
plus a reinterpret. They are restored to the bit-exact conversions (the same
shape as Dolphin's ConvertToSingle / ConvertToDouble), which matters for
denormals and for values that are not already single-representable, and the
(f32)/(f64) casts at the call sites are removed so the conversions actually see
their inputs.
Route the affected instructions back through the helpers: fadds, fsubs, fmuls,
fdivs, fadd, fsub, fmul, fdiv, frsp, fcmpo, fcmpu, and the ps_* group.
Paired-single is the Gekko's SIMD unit, so this is the path vertex and matrix
maths take. The visible symptom on Skyward Sword (NTSC-U, SOUE01) was character
models losing limbs and bursting into stray triangles as soon as they animated;
the title renders correctly with a stale build and incorrectly with this one.
After the change the recompiled module renders byte-identical frames to that
known-good build, from a fixed savestate driven by scripted input.
No change to the CFG work: loop outlining, direct back-edges and the routed
local returns are all still emitted -- 819 outlined loops in this title before
and after. ctest is 14/14.
|
The new C emitter calls ppc_fcmp, but that helper uses: cpu->fpscr |= compare << 12;A less-than comparison followed by equal produces FPCC 0xA instead of replacing it with 0x2. Clear the |
ppc_fcmp OR-ed its result into FPSCR, so a less-than followed by an equal left FPCC at 0xA rather than 0x2. Clear the four FPCC bits first and keep bit 16, the C class bit, which a compare does not modify. test_fpscr covers consecutive compares of differing results and the preserved C bit; it reports FPCC 0xA without the fix.
|
Fixed in 2515808.
|
test_float_semantics counts divergence per dropped behaviour instead of checking the helpers against themselves, so the numbers can be read off rather than taken on trust. It also corrects a claim in this PR's description. The 25-bit truncation of a multiply's C operand is not wrong on every instruction: a C operand that came from lfs is already single-representable, its low 29 mantissa bits are zero, and truncation is a no-op. Over 100,000 fixed-seed pairs those agree exactly, 0/100000. Only a C operand carrying more than 25 mantissa bits diverges, and then in 12,545/100000 cases -- the final narrowing to single absorbs most of the difference.
|
Added That first line corrects something I claimed in the description above. I wrote that an inlined So the scope of that one is narrower than I stated:
|
|
Render evidence for the Skyward Sword (NTSC-U, Without the fix — arms gone, head detached and floating near the desk, torso separated from the legs:
With it — Link intact:
Everything else in the frame is identical: room, furniture, HUD, the shadow on the rug. The defect is confined to the skinned character mesh, and it moves between consecutive shots as the idle animation plays, which is what a stale Two caveats on what this does and does not show. The framerate says nothing. Both arms held ~30 FPS with The unfixed module is a cached build from an earlier tree, and the fixed one predates the last two commits on this branch (the FPCC change and the tests). So this covers the helper routing that the PR is built on, not those two. Images are on a |
|
The previous FPCC issue is fixed, but I found a few remaining gaps in the emitted float path. ps_cmp* still performs an inline comparison that only updates CR. It does not update FPSCR FPCC ordistinguish the ordered and unordered NaN exception behavior. The scalar helpers now used for fadd(s), fsub(s), fmul(s), and fdiv(s) still write their results The record forms of those operations, along with frsp., do not update CR1 when Rc=1. good luck! |
The counts were printed but only asserted non-zero, so truncation could have collapsed from 12,545 divergent products to 3 and still passed. The LCG is fixed and the arithmetic is IEEE-defined, so both counts are exact: 0/100000 for a C operand from lfs, 12545/100000 for a full mantissa. Pinned. The ps_* path had no coverage at all, which is awkward given it is the one that reaches the screen. The corruption is not in the instruction that drops ps1 -- it is in the paired-single instruction that reads ps1 afterwards, so the test now runs that chain: poison ps1, produce the register with fmuls, consume it from ps_add, and check both lanes. Run again with the inline form standing in for fmuls and only the ps1 lane goes wrong, ps0 stays correct. That asymmetry is why this shows up as geometry rather than as an obviously broken number.
2 participants
The C backend inlines single- and double-precision floating point as plain C:
That produces a plausible value and drops four things the hardware does, none of
them visible in the generated source.
What the inline form loses
The 25-bit C operand.
ppc_fmulsruns its C operand throughforce_25bit_cfirst — the Gekko truncates a multiply's C operand to a 25-bit mantissa. An
inlined
f64multiply is therefore numerically wrong on every instruction, notonly in edge cases.
ps1.fp_write_singlewritesps1as well asfpr. A single-precisionresult occupies both halves of a paired-single register. The inline form leaves
ps1holding whatever was there before, and theps_*instructions go on toread it.
FPSCR.
FPRF,FIandFRare never updated.NaN handling. Invalid-operation gating is skipped.
emit_fcomparealsodropped the
orderedflag entirely, sofcmpoandfcmpucompiled toidentical code even though only
fcmposignals on a NaN operand.dolrecomp_f32_to_bits/dolrecomp_f32_from_bitshad likewise become a castplus a reinterpret. They are restored to the bit-exact conversions (the same
shape as Dolphin's
ConvertToSingle/ConvertToDouble), which matters fordenormals and for values that are not already single-representable, and the
(f32)/(f64)casts at the call sites are removed so the conversions see theirreal inputs.
The change
Route the affected instructions back through the helpers:
fadds,fsubs,fmuls,fdivs,fadd,fsub,fmul,fdiv,frsp,fcmpo,fcmpu, andthe
ps_*group. One file,src/backend/emitter.c.Why it shows up as graphics corruption
Paired-single is the Gekko's SIMD unit, so this is the path vertex and matrix
maths take. Feeding it a stale
ps1corrupts skinning.On Skyward Sword (NTSC-U,
SOUE01) character models lost limbs and burst intostray triangles as soon as they animated. A GameCube title in the same tree
looked fine by eye, which is worth noting: the symptom depends on how much the
game leans on paired-single, so a clean-looking screenshot is not evidence the
arithmetic is right.
Verification
Measured by diffing the emitted C per guest instruction against a build from
before the regression, over 1,269,248 instructions:
ps_*and the comparesThe recompiled module then renders byte-identical frames to the known-good
build, from a fixed savestate with scripted input, compared by hash rather than
by eye.
ctestis 14/14, includingc_execute.Not affected
The CFG work is untouched — loop outlining, direct back-edges and routed local
returns are all still emitted, 819 outlined loops in this title before and
after. This is a correctness fix, not a performance trade.