Skip to content

AGU modulo addressing skips wrap-around for a buffer based at address 0 (unsigned underflow in updateAddressRegister) #5

Description

@crosswick

hi - my LLM says it's found and fixed a bug. in its own words:

Summary
AGU::updateAddressRegister in source/dsp56kEmu/agu.h computes the modulo wrap-around using an unsigned comparison against a lower bound. When the modulo buffer is based at address 0 (so lowerBound == 0) and an address-register update carries the pointer below the base, the pointer underflows to a large unsigned value (0xFFFFFxxx) instead of wrapping. The if (r < lowerBound) test can then never fire, so the wrap is skipped and the pointer escapes into high memory. On real DSP56300 hardware the pointer wraps correctly for any |Nn| <= M.

Affected code

n = signextend<int, 24>(n);

const auto lowerBound = r & ~moduloMask;
const auto upperBound = lowerBound + m;

if constexpr(add)
    r += n;
else
    r -= n;

modulo = n & moduloMask ? modulo : 0;

if(r < lowerBound)        // <-- r is unsigned; when lowerBound == 0 this is never true
    r += modulo;
if(r > upperBound)
    r -= modulo;

r is an unsigned TWord. After r -= n underflows below the base, r becomes a large unsigned value, so r < lowerBound (with lowerBound == 0) is unreachable and the corrective r += modulo never runs. The final r &= 0x00ffffff then yields a wrong (high) address rather than the correctly-wrapped one.

Interestingly, the file already contains a commented-out alternative implementation that tests the sign of the buffer-relative offset rather than the absolute pointer value. That form is correct for buffers at any base, including 0.

Reproduction
Any modulo-addressed access on a buffer whose base is address 0 and whose pointer update moves it below the base triggers this. In our case it showed up as a periodic audio dropout: firmware running on the emulator used a mod-0x2000 delay line based at Y:0x0000; every 64 blocks the read pointer's base landed at 0 and a negative offset underflowed, causing the emulator to read zeros (silence) instead of the wrapped delay-line samples. Real hardware produces continuous audio in the same scenario.

Fix
Switching to the sign-bit form (the file's own previously-commented-out alternative, with explicit int32_t casts so the sign test is well-defined) resolves it. It tests the sign of the buffer-relative offset instead of comparing the absolute unsigned pointer, so it wraps regardless of buffer base:

modulo = n & moduloMask ? modulo : 0;

if constexpr(add)
{
    const int32_t p  = static_cast<int32_t>(r & moduloMask) + static_cast<int32_t>(n);
    const int32_t mt = static_cast<int32_t>(m) - p;
    r += n;
    r += (p>>31)  & modulo;
    r -= (mt>>31) & modulo;
}
else
{
    const int32_t p  = static_cast<int32_t>(r & moduloMask) - static_cast<int32_t>(n);
    const int32_t mt = static_cast<int32_t>(m) - p;
    r -= n;
    r += (p>>31)  & modulo;
    r -= (mt>>31) & modulo;
}

We verified this is surgical — it produces identical results to the original code on every already-correct case, for both add and sub, across the whole |Nn| <= M domain (exhaustive test). It also eliminated the audio dropout end-to-end and matched hardware behavior over a 600-block run.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions